fix(kimi): adapt to newer Kimi Code CLI (0.28.1) flag and stream changes - #1586
fix(kimi): adapt to newer Kimi Code CLI (0.28.1) flag and stream changes#1586OctopusWen wants to merge 1 commit into
Conversation
The newer Kimi Code CLI removed the standalone --work-dir, --quiet, --resume and --print flags, and changed the stream-json format: - Probe kimi --help for --work-dir/--quiet/--resume in addition to --print, and only emit flags the installed binary advertises. Passing --work-dir to 0.28.1 errors with 'unknown option'. - Fall back to cmd.Dir (already set to the session workDir) when --work-dir is unsupported; degrade --quiet to a debug log; use the hidden -r alias when --resume is gone. - Accept assistant/tool content as a plain string, not just block arrays, so text is no longer silently dropped. - Handle the meta session.resume_hint event the new CLI emits instead of the legacy plain-text resume line.
chenhg5
left a comment
There was a problem hiding this comment.
Conclusion: Request changes
Overall assessment:
- The code is technically solid and well-scoped: probe-driven flag gating for
--quiet/--resume/--work-dir, ametarole case forsession.resume_hint, format-tolerant content parsers in bothhandleAssistantandhandleTool, and six new black-box tests viabuildArgs/handleEvent/flushPendingAsText. CI is 5/5 green,go test -racepasses,go vetclean. However, this PR is functionally subsumed by #1564 — every change here is also in #1564 (with a CHANGELOG entry, alive_e2e_test.go, dual session-storage roots,state.jsonshape tolerance, and/list//deleteparity that this PR doesn't touch). Merging #1586 before #1564 would create duplicate code paths on thekimiFlagSupportstruct and the-rfallback branch, requiring future reconciliation.
Review scope:
- Reviewed
agent/kimi/probe.go,agent/kimi/probe_test.go,agent/kimi/session.go,agent/kimi/session_test.go(+272/-58 across 4 files). - Focused on correctness, probe-based design consistency, and coverage vs the 4 concurrent PRs.
✅ What looks good:
- Pattern consistency with #1461 / #1483. All four gates (
Print,WorkDir,Resume,Quiet) follow the same probe-driven shape. Reviewers who already approved those can verify in 30 seconds. -rfallback matches the kimi team's own resume_hint. The CLI advertiseskimi -r <id>in the modern meta line; this PR picks-rover the more verbose--session(which #1566 picked instead — #1566 is the outlier here).- Two-realistic-surfaces test fixtures (
LegacyCLISurface+ModernCLISurface) pin the all-true and all-false probe surfaces symmetrically. ThedrainEventshelper at the bottom is clean. - Meta resume-hint handler strictly checks
type == "session.resume_hint";TestHandleEvent_MetaResumeHintexplicitly asserts unrelated meta doesn't clobber the session ID — exactly the right regression test. - Surgical diff. No scope creep into other agents, no platform changes, no core changes.
🚨/🔴 Must fix:
- P1 —
WorkDirhalf of this diff is a duplicate of #1483 (already merged). The struct field, the probe wiring, and the gate are all identical to what #1483 just shipped (merge commit6c860798). The only delta is the expanded doc-comment inprobe.go. Merging this half of #1586 would re-introduce the same struct field, which is harmless but pollutes git history.
Impact: code duplication; future maintainers must reconcile whichkimiFlagSupportshape is canonical.
Suggested fix: if the maintainer chooses Strategy A (merge #1564, close #1586), no fix needed — close and credit the author. If Strategy B (merge #1586 as minimal fix ahead of #1564), coordinate with #1483 / #1601 / #1564 so the struct shape is consistent.
Verification:gh pr diff 1586 -- agent/kimi/probe.goshows only the doc-comment delta vs main's #1483 result.
🟠 Should improve:
- P2 — CHANGELOG entry missing. Per cc-connect convention (#1461, #1483, #1564 all add a
### Fixedbullet). Flagging, not blocking — maintainer may prefer to consolidate into the eventual #1564 entry. - P2 — No
live_e2e_test.go. #1564 ships aKIMI_LIVE_E2E=1-gated live test that exercises the multi-turn resume path this PR is meant to fix. PR description doesn't claim live validation either. Without it, the CI-evidenced surface for the meta-hint path is only the unit test. - P2 — Coordinate with
highland0971(#1601) onkimiFlagSupport.Quiet. Both this PR and #1601 add fields tokimiFlagSupport; the struct shapes must match for either to merge cleanly with #1564's broader struct. If both land independently, #1564's rebase will see conflicts.
🔵 Optional:
- P3 —
drainEventshelper (last 10 lines of the diff) is added tosession_test.go. If it's not reused elsewhere, consider moving to atesthelper_test.gopackage-private file to keepsession_test.gofocused. - P3 — Doc comment in
probe.goduplicates what #1483 already added. Most of the wording mirrors #1483's commit; cleanup if #1586 lands standalone.
❓ Questions:
- Q1 — Is the author aware of #1564 / #1566 / #1601? This is a first-time-contributor PR; the PR description doesn't acknowledge the overlap. A maintainer comment pointing at #1564 as the canonical fix (once rebased) would save the author follow-up effort and credit their work.
- Q2 — If the maintainer closes this in favor of #1564, can the author contribute the missing pieces (e.g. a
live_e2e_test.go, or the--quietgate in #1564's stack)? That's a way to credit the work without leaving duplicate code inmain.
Testing / Risk:
- Verified locally:
go test -race ./agent/kimi/... -count=1 -timeout 60s→ PASS.go vet ./agent/kimi/...→ clean. CI 5/5 SUCCESS atb635b8a8. - Backward compat: legacy kimi-cli path unchanged when probe advertises all flags. The
-rfallback is safe because legacy kimi-cli also accepts-rper thelegacyKimiHelpfixture (which the diff extends). - Remaining risk if merged as-is: code duplication with #1564 on the struct shape and the
-rfallback.
Next step:
- Maintainer: decide between Strategy A (merge #1564 once rebased, close #1586 + #1566 + #1601) or Strategy B (merge #1586 / #1601 as minimal fix, then trimmed #1564). I'd recommend Strategy A — #1564's live e2e test is the strongest signal that the full #1561 surface is covered end-to-end.
- Author: if Strategy A, I'll close #1586 with a thank-you pointing at #1564. If Strategy B, please address the CHANGELOG + live e2e gaps and coordinate with
highland0971onkimiFlagSupport.Quiet.
|
@OctopusWen 谢谢 PR! 整体方向、probe-gating 模式和测试都做对了 — 跟 #1461 / #1483 风格一致, 也是用 不过有两个 maintenance 层面的考量想跟你说明: 核心问题: 这个 PR 的所有改动已经被 #1564 完整覆盖了 — #1564 多带了 4 件事: dual session-storage roots、 #1586 跟 #1483 的重叠: 你新加的 两个方向供你选:
倾向选哪个? 你说一声。 — cc-connect/qa-claudecode |
…enhg5#1561) Ported from chenhg5#1586: an assistant event carrying plain-string content AND tool_calls must surface the text as a thinking event (via flushPendingAsThinking) before the tool-use event, not drop it silently.
Closing in favor of #1564 — credit preserved@OctopusWen 谢谢你的 PR! 整体方向、probe-gating 模式、测试都做对了 — 跟 #1461 / #1483 风格一致。 8-15 你主动选了方案 2:在 #1564 上加了 结果 #1564 (现在 +795/-120, 14 commits, CI 5/5 ✓) 已经吸收了你的核心贡献:
这个 PR 作为独立 commit 历史保留在这里 — git log 里 #1564 已经过 maintainer fresh review 通过,可以合并。后续 issue / 新适配如果有 Kimi Code CLI 0.28+ 兼容需求,欢迎继续 push。 感谢对 cc-connect 的贡献! 🙏 — cc-connect/qa-claudecode |
* fix(kimi): gate --work-dir flag on probe, like --print (#1476) Same pattern as the #1456 / PR #1461 --print fix. Newer Kimi Code CLI builds no longer accept --work-dir, exiting with `error: unknown option --work-dir` whenever the user's config sets a non-default workspace directory. - agent/kimi/probe.go: kimiFlagSupport gains a WorkDir bool; the probe fills it from `kimi --help` (parseKimiHelpFlags already scans the --work-dir token, no parser changes needed). - agent/kimi/session.go buildArgs: --work-dir is now emitted only when flagSupport.WorkDir is true. The agent still runs in the correct directory via exec.Command.Dir (set separately), so omitting the flag on modern CLIs is functionally equivalent for users on default dirs and graceful for users on non-default dirs whose CLI just ignores it. - agent/kimi/probe_test.go: extend the legacy test to assert --work-dir is detected, add modernKimiHelpWithoutWorkDir constant that mirrors the build the reporter hits, plus full TestKimiFlagSupport_LegacyHelpSetsWorkDir / TestKimiFlagSupport_ModernWithoutWorkDir coverage of the probe mapping. - agent/kimi/session_test.go: TestBuildArgs_WorkDirFlagGated (no flag on modern CLI) + TestBuildArgs_WorkDirFlagEmitted (legacy CLI keeps it). Also asserts the work-dir value doesn't leak into args when the gate is closed, catching a partial-gate future bug. go test ./agent/kimi/... — all pass. * feat(kimi): native Kimi Code CLI dialect support Fixes #1561 The kimi agent targeted the legacy Python kimi-cli dialect; the newer Node.js Kimi Code CLI (kimi-code) speaks a different one. Extend the #1461/#1483 probe-gating approach to cover the remaining differences: - probe: also detect --quiet; add isModernFlavor() using --print absence as the family discriminator established in #1456 - buildArgs: resume with -r instead of --resume on the modern dialect (--resume is rejected; -r matches the CLI's own resume hint); gate --quiet and emulate quiet mode via local event suppression when the binary dropped it; never pass --yolo/--auto (bare --prompt already auto-approves, and Kimi Code rejects combining them) - stream-json: accept plain-string content for assistant/tool messages alongside the legacy block-array shape (format-tolerant, no gating) - session continuity: capture the session id from Kimi Code's stdout meta line {"role":"meta","type":"session.resume_hint"} instead of the legacy plain-text/stderr hint - session listing: scan both ~/.kimi/sessions and ~/.kimi-code/sessions, understand the Kimi Code state.json schema, honor its workDir field Tests: real v0.26.0 --help fixture, per-dialect arg tests, content-shape and meta-hint regression tests, dual-flavor session listing test, and an env-guarded live e2e (KIMI_LIVE_E2E=1) that verifies anchor recall across a resumed turn against the production binary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(kimi): don't report 0 messages for Kimi Code sessions in /list Kimi Code CLI sessions have no context.jsonl; their transcript lives at agents/main/wire.jsonl. Without a fallback, /list showed 0 messages for every modern session. Count user turns from wire.jsonl (and use the first user turn as the summary), matching legacy kimi-cli behavior. Addresses review feedback on #1564. * fix(kimi): adapt live e2e to main's Send(messageID, ...) signature CI lint failed on the PR merge ref: main added a messageID string param to AgentSession.Send (core/interfaces.go), but the PR still called the old 3-arg form in live_e2e_test.go, so the merged code failed to compile ('not enough arguments in call to s.Send'). origin/main was merged into kimi-code-flavor (no conflicts); kimi's Send now carries the 4-arg signature matching main. This updates the remaining 3-arg call site to the new signature. agent/kimi and core compile, vet and tests green. * fix(kimi): check f.Close error in transcript helpers (errcheck) golangci-lint flagged the deferred file Close calls in the new parseKimiTranscript/count* helpers as unchecked errors. Use an explicit blank-assignment close to satisfy errcheck. agent/kimi still compiles, vets and tests green. * feat(kimi): native Kimi Code CLI dialect support Fixes #1561 The kimi agent targeted the legacy Python kimi-cli dialect; the newer Node.js Kimi Code CLI (kimi-code) speaks a different one. Extend the #1461/#1483 probe-gating approach to cover the remaining differences: - probe: also detect --quiet; add isModernFlavor() using --print absence as the family discriminator established in #1456 - buildArgs: resume with -r instead of --resume on the modern dialect (--resume is rejected; -r matches the CLI's own resume hint); gate --quiet and emulate quiet mode via local event suppression when the binary dropped it; never pass --yolo/--auto (bare --prompt already auto-approves, and Kimi Code rejects combining them) - stream-json: accept plain-string content for assistant/tool messages alongside the legacy block-array shape (format-tolerant, no gating) - session continuity: capture the session id from Kimi Code's stdout meta line {"role":"meta","type":"session.resume_hint"} instead of the legacy plain-text/stderr hint - session listing: scan both ~/.kimi/sessions and ~/.kimi-code/sessions, understand the Kimi Code state.json schema, honor its workDir field Tests: real v0.26.0 --help fixture, per-dialect arg tests, content-shape and meta-hint regression tests, dual-flavor session listing test, and an env-guarded live e2e (KIMI_LIVE_E2E=1) that verifies anchor recall across a resumed turn against the production binary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(kimi): don't report 0 messages for Kimi Code sessions in /list Kimi Code CLI sessions have no context.jsonl; their transcript lives at agents/main/wire.jsonl. Without a fallback, /list showed 0 messages for every modern session. Count user turns from wire.jsonl (and use the first user turn as the summary), matching legacy kimi-cli behavior. Addresses review feedback on #1564. * fix(kimi): adapt live e2e to main's Send(messageID, ...) signature CI lint failed on the PR merge ref: main added a messageID string param to AgentSession.Send (core/interfaces.go), but the PR still called the old 3-arg form in live_e2e_test.go, so the merged code failed to compile ('not enough arguments in call to s.Send'). origin/main was merged into kimi-code-flavor (no conflicts); kimi's Send now carries the 4-arg signature matching main. This updates the remaining 3-arg call site to the new signature. agent/kimi and core compile, vet and tests green. * fix(kimi): check f.Close error in transcript helpers (errcheck) golangci-lint flagged the deferred file Close calls in the new parseKimiTranscript/count* helpers as unchecked errors. Use an explicit blank-assignment close to satisfy errcheck. agent/kimi still compiles, vets and tests green. * test(kimi): pin assistant string-content + tool_calls flush order (#1561) Ported from #1586: an assistant event carrying plain-string content AND tool_calls must surface the text as a thinking event (via flushPendingAsThinking) before the tool-use event, not drop it silently. * docs(changelog): add #1561 kimi-code dialect entry --------- Co-authored-by: dev-claudecode <dev-claudecode@cc-connect.local> Co-authored-by: whyihaveyou <whyihaveyou@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: QZP <qzp@QZPdeMacBook-Air.local> Co-authored-by: octopus <2841776039@qq.com>
The newer Kimi Code CLI removed the standalone --work-dir, --quiet, --resume and --print flags, and changed the stream-json format:
Summary
Type of change
Testing
Automated tests added in this PR
TestX_Y_Zinpath/to/file_test.go— what it assertsFor bug fixes only — regression test
Test...Critical User Journeys (CUJ) impact
/new/switch/list/historyetc.)/mode/cancel/stoppermissions)allow_fromadmin_frombanned_wordsrate limits)/cron/timer)/lang/provider/modelreload)If any CUJ group is touched, confirm:
go test ./core/ -run TestCUJpasses locally.CUJ test was updated (or a new CUJ added) to cover the new behavior.
Manual / user-visible behavior change
Checklist (reviewer will verify)
go build ./...passesgo test ./...passes (with-raceif touching concurrency)core/Related