Skip to content

feat(agents): add Kimi Code CLI support - #248

Open
LarryHu0217 wants to merge 10 commits into
johannesjo:mainfrom
LarryHu0217:codex/kimi-cli-109
Open

feat(agents): add Kimi Code CLI support#248
LarryHu0217 wants to merge 10 commits into
johannesjo:mainfrom
LarryHu0217:codex/kimi-cli-109

Conversation

@LarryHu0217

Copy link
Copy Markdown
Contributor

Summary

  • register Kimi Code as a built-in kimi agent with current resume and permission-bypass flags
  • install Kimi Code in the bundled Docker image and share its .kimi-code auth/config directory
  • avoid the unsupported generic --mcp-config argument and cover native, Docker, and coordinator launch behavior

Fixes #109.

Validation

  • npx vitest run electron/ipc/agents.test.ts electron/mcp/agent-args.test.ts electron/ipc/pty.test.ts src/lib/agent-args.test.ts src/store/tasks.test.ts
  • npm run check
  • npm run check:static
  • npm test
  • npm run test:security-rules
  • npm run build:frontend
  • npm run build:remote
  • npm run build:mcp

@johannesjo johannesjo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for one merge-blocking coordinator-path issue.

electron/mcp/agent-args.ts:62 — Coordinator-created Kimi children never load their per-task MCP config.

Coordinator.createTask writes each child config to /tmp/parallel-code-subtask-.json natively or the coordinator .parallel-code/subtask-.json in Docker, then relies on buildMcpLaunchArgs to pass that path. This new Kimi branch returns an empty argument list. Kimi Code 0.31.1 discovers only its user mcp.json, project-root .mcp.json, and cwd/.kimi-code/mcp.json (published loader source), so neither generated child path is read. The child therefore starts without the parallel-code MCP server and cannot call land_self or signal_done, breaking the default Kimi coordinator-child flow in both native and Docker modes.

Please write or merge the unique child config into an auto-discovered child-worktree location while preserving token isolation and cleanup/restoration, and add coordinator child-creation coverage for Kimi. The new store test exercises top-level coordinator restoration only; it does not cover Coordinator.createTask.

@LarryHu0217

Copy link
Copy Markdown
Contributor Author

Addressed the coordinator-child blocker in 69a9fad.

  • Kimi children now merge their per-task parallel-code MCP entry into the child worktree's auto-discovered .mcp.json before spawn, in both native and Docker flows.
  • Per-child task IDs and done tokens remain isolated.
  • Existing MCP servers and any previous parallel-code entry are preserved and restored during teardown; concurrent edits are not overwritten.
  • Hydration/server-info refreshes also update the auto-discovered config.
  • Added coordinator creation, isolation, restoration, and concurrent-edit coverage.

Validation:

  • npm test (1637 passed, 23 skipped)
  • npm run check
  • npm run check:static
  • npm run build:frontend && npm run build:mcp
  • targeted Semgrep scan of the changed production files (0 findings)

@johannesjo johannesjo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed current head 4d3d594. The original coordinator-child blocker is addressed, the Kimi flags and discovery paths match the published 0.32.0 implementation, and CI passes. Two merge-blocking lifecycle issues remain:

  1. electron/mcp/coordinator.ts:1554 — A tracked .mcp.json breaks or contaminates self-landing. The child config is merged directly into the task worktree's project-root .mcp.json. Adding that path to .git/info/exclude does not hide modifications to an already tracked file. Such a task therefore either fails prepareCleanSelfLandingWorktree() because .mcp.json is dirty, or can commit and merge the generated parallel-code entry containing the ephemeral token. Cleanup/restoration currently happens only after the merge/delete path. Please ensure the generated entry is restored or removed before landing validation and merge (or use an auto-discovered path guaranteed not to be tracked), and add a tracked-.mcp.json self-landing test.

  2. electron/mcp/coordinator.ts:1571 — Restart hydration loses the original restoration snapshot. previousParallelCode lives only in task.autoDiscoveredMcpConfig; that state is neither persisted nor supplied to hydrateTask. After an application restart, hydration reads the previously generated entry as though it were the original user entry, writes fresh credentials, and teardown later restores the stale generated entry instead of the user's value or deleting the file. Persist/hydrate the original snapshot (or durably identify generated state), and add a create → process restart/hydrate → deregister restoration test.

Requesting changes until these two paths preserve Git state and restoration semantics.

@LarryHu0217

Copy link
Copy Markdown
Contributor Author

Addressed both lifecycle blockers in d4ae1a6.

  • The original .mcp.json snapshot and generated-entry fingerprint now persist through autosave, IPC synchronization, and restart hydration.
  • Self-landing and merge restore a managed generated entry before Git validation, so tracked files cannot carry the temporary token into a commit.
  • A failed landing re-injects the temporary entry only when restoration remains safe; concurrent user edits remain untouched.
  • Added restart round-trip, tracked-.mcp.json self-landing, persistence, hydration, and sync coverage.

Validation: 412 focused tests; full suite 1,768 passed/23 skipped plus 9 client tests; type checks, static checks, and frontend build pass.

@LarryHu0217
LarryHu0217 requested a review from johannesjo August 4, 2026 14:29

@johannesjo johannesjo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed current head d4ae1a6, including two independent verification passes. The original coordinator-child MCP wiring and restart-persistence issues are addressed. CI is green, exact-head TypeScript checking passes, and all 262 coordinator tests pass. Three merge blockers remain:

  1. Critical — Kimi 0.33 stalls new coordinator children at the workspace-trust gate. docker/Dockerfile:53 installs Kimi without a version pin, so new images now receive @moonshot-ai/kimi-code@0.33.0. That release enables agent-core-v2 by default and asks whether to trust each new folder before creating a session; project .mcp.json/.kimi-code/mcp.json servers stay disabled until accepted (0.33 release source, trust gate). --yolo controls tool permissions and does not bypass this pre-session prompt. Every child starts in a fresh background worktree, so it waits at the dialog before loading the generated MCP server or receiving its assignment. Please either pin a known-compatible release or explicitly support the new trust flow without silently weakening its security boundary, and add a real-PTY fresh-worktree startup test.

  2. Critical — failed restoration still allows token-bearing config to enter Git history. restoreTaskAutoDiscoveredMcpConfig() clears the recovery snapshot before success is known and returns false for fingerprint mismatches and read/write/unlink failures. Both landSelf and mergeTask continue anyway; mergeTask then runs git add -A and auto-commits. A mismatched generated entry can therefore be committed with both ephemeral MCP tokens. If that entry was committed earlier, even successful restoration cannot remove the earlier secret-bearing commit from a default non-squash merge. Managed restoration must fail closed, retain its snapshot for retry, and secret-bearing generated config must be kept out of tracked Git history. Add mismatch/write-failure and prior-generated-commit tests asserting no stage, commit, or merge occurs.

  3. Important — semantic restoration does not restore a tracked .mcp.json cleanly. Both the initial write and restoration serialize the whole document with two-space indentation and no preserved trailing newline (restore write). A tracked file with compact JSON, different indentation, CRLF, or a final newline remains byte-dirty, so land_self rejects it. The new test declares Git clean from parsed-object equality and cannot catch this. Preserve/restore exact original bytes or use an auto-discovered path verified to be untracked, then cover it with a real temporary Git repository.

Verdict: needs changes.

@johannesjo

Copy link
Copy Markdown
Owner

Re-reviewed current head 0cb1285 from scratch. The earlier Docker-version, fail-closed restoration, and byte-for-byte restoration concerns are addressed. I also checked the native Kimi 0.33 trust flow against the existing question handoff/forced coordinator auto-trust behavior and am not carrying that concern forward.

CI is green. On the exact head I additionally ran npm run typecheck, npm run compile, the full unit suite (1,771 passed, 25 skipped), and the client suite (9 passed); all passed. Three merge blockers remain:

  1. Security: a tracked .mcp.json can still put the ephemeral MCP tokens into merged Git history. writeKimiAutoDiscoveredMcpConfig writes the token-bearing config into the project root and only adds it to .git/info/exclude, which does not protect a file that was already tracked. If the child commits that modification, restoring the final worktree does not remove the secret-bearing commit. Both the default merge path (squash ?? false) and self-landing (explicitly non-squash) preserve it. I reproduced this with a tracked .mcp.json: after the child commit, restoration, and non-squash merge, git log -S ephemeral-token -- .mcp.json still found the token-bearing commit on the target branch. Please avoid writing managed credentials to a tracked path (or fail task creation clearly), and reject landing if an earlier task commit contains the managed token; restoring only the tip is insufficient.

  2. Data loss: restoration overwrites concurrent edits outside mcpServers["parallel-code"]. The guard fingerprints only the owned entry (lines 1645–1648), but when that entry matches, the code replaces the entire current file with the launch-time raw snapshot (lines 1650–1654). I reproduced a user changing another MCP server plus a top-level setting while the task ran: the fingerprint still matched and both edits were reverted. Either fingerprint the complete generated document and fail closed on any change before restoring the raw bytes, or restore only the owned entry while preserving all other current content.

  3. Security: the raw pre-existing MCP config is copied into renderer state and ordinary app-state backups. previousContent captures the whole file and is sent to the renderer, then toPersistedTask includes it in the saved payload. A normal MCP config may contain API keys in server env fields, so this duplicates those credentials into state.json; the persistence layer writes with default process permissions and copies the prior state to state.json.bak before replacement (lines 27–42), allowing the secret-bearing snapshot to linger after restoration. Keep the raw backup out of renderer/general app state—e.g. a main-process-only mode-0600 backup with only non-secret metadata persisted—and remove it after restoration.

Verdict: needs changes because the current lifecycle can retain credentials in Git/app-state history and can discard unrelated user configuration edits.

@LarryHu0217

Copy link
Copy Markdown
Contributor Author

Addressed the remaining Kimi MCP lifecycle blockers in 3f91f45.\n\n- Writes managed Kimi child MCP credentials to Kimi's auto-discovered .kimi-code/mcp.json instead of project-root .mcp.json, and excludes that generated path.\n- Removes raw pre-existing MCP config content from renderer/persisted task state.\n- Restores only the owned parallel-code entry so concurrent edits to other MCP servers/settings are preserved.\n- Fails closed before self-landing/merge if the managed token is already present in task Git history.\n\nValidation: npx vitest run electron/mcp/coordinator.test.ts src/store/persistence.test.ts src/store/tasks.test.ts, npm run check, npm run check:static, npm run build:frontend && npm run build:mcp, git diff --check, and the push hook full unit/client tests passed. Hosted CI is running on the pushed head.

@LarryHu0217

Copy link
Copy Markdown
Contributor Author

Follow-up hardening is in 1508018 after an additional audit of the credential lifecycle:

  • Selects only an untracked Kimi discovery path, falls back to the other supported path, and fails task creation if both are tracked.
  • Refuses a pre-existing parallel-code entry instead of copying it into persisted state; persisted restoration metadata is now path plus fingerprint only.
  • Preserves concurrent edits while removing only the managed entry.
  • Recovers the managed tokens from the mode-0600 per-task config when the discovery file was deleted, so the Git-history guard still runs.
  • Scans Git history without putting either token in process arguments.

Validation: full unit suite (1,775 passed, 23 skipped), client suite (9 passed), npm run check, npm run check:static, targeted MCP/store tests (418 passed), frontend and MCP production builds, and git diff --check.

@LarryHu0217

LarryHu0217 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

The latest quality job was cancelled after waiting 15 minutes in the runner queue and executed zero steps. GitHub Status now confirms an active major Actions outage causing hosted jobs to remain queued or time out, so this is not a code failure. Once Actions recovers, a maintainer rerun will be needed because GitHub returns 403: Must have admin rights to Repository when a PR author tries to rerun an upstream workflow. The local push gate completed successfully on head 1508018: 1,775 unit tests passed (23 skipped), 9 client tests passed, and compile, typecheck, lint, and format checks passed.

@johannesjo

Copy link
Copy Markdown
Owner

Re-reviewed head 1508018. The credential-lifecycle work from rounds 2–4 all landed, and the tests track it closely. Rather than open a fifth round on the same axis, I want to put two questions on the table that change the shape of this PR — plus one functional gap that is a different class of problem entirely.

1. Should this reuse the coordinator's existing mechanism instead of building a parallel one?

deregisterCoordinator already does this exact job for the coordinator's own auto-discovered config: merge only the parallel-code key, remember what we wrote, restore-or-delete on teardown, unlink the file if no servers and no other top-level keys remain. Compare restoreTaskAutoDiscoveredMcpConfig against the existing block — same safeToRestore comparison against what we wrote, same hasServers / hasOtherKeys / unlink tail. This is now a second implementation with its own semantics rather than a generalization of the first.

I'd like to see one shared helper covering both the coordinator's .mcp.json and the child's .kimi-code/mcp.json. That is likely a meaningful reduction of this diff and removes the risk of the two copies drifting.

2. Is the child guard stricter than my own coordinator path?

This is the one I got wrong across rounds 2–4, and I want to correct it before asking for anything further.

The coordinator writes the same class of ephemeral token into .mcp.json in its own worktree (register.ts), protected by a .git/info/exclude entry and nothing else — no fingerprint persisted across restart, no fail-closed restoration, no git-history token scan. The child path now has all three, because I asked for them one round at a time.

So either the coordinator path carries the same exposure I called merge-blocking for children, or the bar I applied to children was higher than the project's own. I think it's the latter. .git/info/exclude plus restore-on-teardown is the standard this codebase already ships, and the child path clears it comfortably.

Concretely: I'm withdrawing the escalations from rounds 2–4 as merge-blocking. If you'd rather simplify toward the existing convention — drop assertManagedMcpTokensAbsentFromGitHistory and the fail-closed land/merge coupling, keep the untracked-path selection, the exclude entry, and the restore-only-our-own-entry logic — I'd take that PR. Keeping the current hardening is also fine. What I don't want is for this to keep growing to satisfy a standard I don't hold my own code to.

3. Blocker: Kimi children never receive the sub-task preamble

Different class of issue, and it's why I can't merge as-is: the MCP wiring all works, but the child is never told to use it.

injectSubTaskPreamble dispatches on substrings of the lowercased command — codex/opencodeAGENTS.md, geminiGEMINI.md, copilot.agent.md. kimi matches none of them, so it falls through to the default Claude Code branch, which writes a systemPrompt key into <worktree>/.claude/settings.local.json. Kimi doesn't read that file — its own default system prompt names AGENTS.md as the project-guidance file it merges hierarchically (system.md), and the kimi-code repo carries nested AGENTS.md files throughout.

The result is that SUB_TASK_MODE_PREAMBLE — the text instructing the child to commit and call land_self, and to use signal_done only when explicitly asked — never reaches a Kimi child. The whole point of the config plumbing is those two tools.

Fix should be one line, adding kimi to the existing AGENTS.md branch. PREAMBLE_MD_FILES already contains AGENTS.md, so land-time stripping needs no change. Please add a test asserting a Kimi child gets an AGENTS.md preamble rather than .claude/settings.local.json — none of the current coverage would catch this, since it all targets config-file mechanics rather than whether the child is instructed at all.

4. Two smaller robustness items

  • coordinator.ts:689writeKimiAutoDiscoveredMcpConfig sits in a bare for loop in setMCPServerInfo with no try/catch, and it throws on a missing worktree (isTrackedGitPathspawnSync error ENOENT), a non-git cwd (status 128), malformed JSON in either candidate, a pre-existing parallel-code entry, or both paths tracked. One task in that state aborts the config rewrite for every remaining sibling of the coordinator, and the throw escapes into the StartMCPServer handler at register.ts:1755, which doesn't catch it. A task can reach that state — cleanupTask returns early and keeps the task in this.tasks when deleteTask fails. Please log-and-continue per task.

  • coordinator.ts:1757 — if the history scan stays, execAsync is promisify(execFile) with no maxBuffer, so Node's 1 MiB default applies to git log --all -p. A project that tracks .mcp.json with real history — precisely the case that pushes the write to .kimi-code/mcp.json and reaches this code — can exceed that and reject with ERR_CHILD_PROCESS_STDIO_MAXBUFFER, which landSelf turns into an opaque escalation. Scoping to ${baseBranch}..HEAD instead of --all is both cheaper and closer to the actual question ("did this task commit the token"); set an explicit maxBuffer either way.

CI

The quality failure is a job cancelled after 15 minutes in the queue with zero steps executed — the Actions outage, not your code. I'll rerun it. I ran npm run typecheck on 1508018 locally and it passes; I did not run the suite this round.


To summarize what I need: the preamble fix in §3. §1 and §2 are decisions for you — tell me which direction you want and I'll review against that rather than adding requirements.

@LarryHu0217

Copy link
Copy Markdown
Contributor Author

Addressed the Kimi child preamble blocker in 7fc70ae.

  • injectSubTaskPreamble now routes Kimi commands to AGENTS.md, matching the file Kimi Code reads for project guidance.
  • Added a regression test that kimi-code --yolo receives an AGENTS.md preamble and does not create .claude/settings.local.json.

Validation passed locally: npm run test:unit -- electron/mcp/preamble.test.ts, npm run check, npm run check:static, npm run build:frontend && npm run build:mcp, git diff --check, and the pre-push full gate (1,776 unit tests / 23 skipped, 9 client tests). Hosted checks are running on the pushed head.

@LarryHu0217

Copy link
Copy Markdown
Contributor Author

For sections 1 and 2, I am keeping the current hardening in this PR and not expanding it into a coordinator-path refactor. The Kimi child path now uses an untracked discovery path, restores only its managed entry, and keeps the fail-closed safeguards; generalizing the coordinator and child lifecycle can be handled as a separate focused change. The section 3 preamble blocker is addressed in 7fc70ae.

@johannesjo johannesjo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed head 08efdb4. CI is green on that exact commit (both quality and GitGuardian).

§3 is fixed and verified. injectSubTaskPreamble routes kimi to AGENTS.md, and both PREAMBLE_MD_FILES and PREAMBLE_ARTIFACT_PATHS already carry AGENTS.md, so detection, land-time stripping, and the dirty-path allowance in prepareCleanSelfLandingWorktree all work with no further change. I also confirmed against Kimi's own loader that the discovery choice is right: resolveMcpJsonPaths merges {...user, ...projectRoot, ...project}, so <cwd>/.kimi-code/mcp.json wins over <projectRoot>/.mcp.json. findProjectRoot uses stat on .git, which succeeds on a worktree's .git file, so the .mcp.json fallback also resolves to the task worktree rather than the main checkout.

§1 and §2 are settled — you chose to keep the current hardening and defer the shared-helper refactor, which was one of the two options on the table. Not reopening either.

Three things below. None of them expand the scope of this PR; two of the three are deletions.


1. setMCPServerInfo still throws out of a bare loop (§4.1 from last round, not addressed)

coordinator.ts:690 still calls writeKimiAutoDiscoveredMcpConfig with no try/catch, and the method has five reachable throw sites: isTrackedGitPath spawn error, git status ∉ {0,1}, malformed JSON in either candidate, a pre-existing parallel-code entry, both paths tracked.

I verified the missing-worktree trigger empirically — spawnSync('git', …, { cwd: '/nonexistent' }) returns { error: ENOENT, status: null }, which isTrackedGitPath converts into a throw. And a task can sit in that state: cleanupTask returns early on deleteTask failure while leaving the task in this.tasks with mcpConfigPath still set, so the loop does not skip it.

Consequences are both of the ones you named. One bad task skips the config rewrite for every remaining sibling, and the throw escapes into register.ts:1760 — I checked the whole handler, its only try/catch is the .mcp.json read/parse near the top. So StartMCPServer rejects after the remote server was started, the Docker MCP server was copied and setDockerContainerName set, but before setCoordinatorSpawnDefaults, the coordinator .mcp.json write, and the returned mcpLaunchArgs: a half-started coordinator.

Log-and-continue per task.

2. History scan has no maxBuffer and still uses --all (§4.2 from last round, not addressed)

coordinator.ts:1768-1771 still runs git log --all -p --format= -- .mcp.json .kimi-code/mcp.json through promisify(execFile) with only { cwd }. I confirmed the failure mode on the Node version in use (v22.18.0): with exactly those options, 2 MiB of stdout rejects with ERR_CHILD_PROCESS_STDIO_MAXBUFFER. A repo that tracks .mcp.json with real history — precisely the case that forces the .kimi-code/mcp.json selection and reaches this code — can exceed 1 MiB, and landSelf turns that into an opaque landing_escalated.

Separately, --all walks every ref, and PARALLEL_CODE_MCP_TOKEN is the coordinator's subtaskToken, shared by all its children — so a token committed on one child's branch would block landing for unrelated siblings.

Worth restating the tradeoff now that the path selection has landed: the chosen file is guaranteed untracked and added to .git/info/exclude, and git add -A — the only automatic staging path, in both mergeTask and prepareCleanSelfLandingWorktree — skips excluded untracked files. So this scan only catches an agent that ran git add -f on its own MCP config, and it spends a full-history walk on every Kimi land and merge to do it.

Either fix it (${baseBranch}..HEAD plus an explicit maxBuffer) or take the offer from last round and drop assertManagedMcpTokensAbsentFromGitHistory along with the fail-closed land/merge coupling. Dropping it removes this risk entirely and is my preference, but either is fine — I'm not going to relitigate it a third time.

3. New: a problem in the unused candidate hard-blocks every Kimi task

coordinator.ts:1611-1629 reads both candidates eagerly in the candidates map, then the validation loop throws if either one defines mcpServers["parallel-code"] — all before a candidate is selected.

So a repo that tracks a .mcp.json containing a parallel-code server fails every Kimi child creation with .mcp.json already defines mcpServers["parallel-code"], even though .kimi-code/mcp.json is untracked, absent, and would have been the path actually used. Committing a .mcp.json for team-shared MCP servers is ordinary, and a project that uses this app's own server will name that key parallel-code. The malformed-JSON case is broader still: any tracked, malformed .mcp.json blocks Kimi through readMcpJsonContent in the same map, despite never being written to.

Fix is a simplification — select the candidate first, then read and validate only that one. That also shrinks the blast radius of item 1.

Smaller things

  • agent-args.ts:21 vs preamble.ts:88 — the two layers classify Kimi differently: the MCP layer matches basename === 'kimi', the preamble matches substring includes('kimi'). I checked the registry and @moonshot-ai/kimi-code@0.32.0 ships only a kimi bin, so the default path is consistent and nothing is broken today. But preamble.test.ts drives the new test with agentCommand: 'kimi-code --yolo' — a form the MCP layer classifies as non-Kimi, which falls through buildMcpLaunchArgs to ['--mcp-config', configPath] (the exact flag this PR exists to avoid) and skips the auto-discovered write entirely. Either align the two predicates or use kimi in the test, so it stops advertising a form that isn't wired.

  • docker/Dockerfile:53@moonshot-ai/kimi-code@0.32.0 is the only pinned CLI on that line and the reason for the pin (0.33's workspace-trust gate) exists only in dockerfile.test.ts and this thread. Please put a one-line comment at the pin itself; otherwise the next routine bump silently reintroduces the hang. The guard expect(dockerfile).not.toContain('@moonshot-ai/kimi-code ') is also brittle — it keys on a trailing space and passes on a reformatted line.

  • coordinator.ts:2044refreshTaskMcpConfigAfterLandingFailure also runs on the merge success path, to re-arm the child's config after a non-cleanup merge. The behavior is right, the name isn't.

  • coordinator.ts:1762 — the tokens.length === 0 guard is unreachable: restoreTaskAutoDiscoveredMcpConfig only returns a defined managedEntry when its fingerprint matches an entry we wrote, and every such entry carries both tokens.

  • coordinator.ts:994createTask's catch clears in-memory state and the MCP config but not the worktree/branch already created by createBackendTask. Pre-existing shape, but this PR adds four new throw sites between creation and spawnAgent, so orphaned worktrees get materially more reachable. Fixing item 3 removes most of that exposure.

Checked and not carrying forward

I looked at whether excluding only .kimi-code/mcp.json rather than the whole .kimi-code/ directory could leave other generated files dirty in the worktree — prepareCleanSelfLandingWorktree throws on any non-preamble dirty path, so it would have broken every land_self. It doesn't: per Kimi's data-locations doc, all runtime data (sessions, logs, credentials, plugins) lives under $KIMI_CODE_HOME, and project-local .kimi-code/ holds only user-authored config (mcp.json, skills/, agents/, AGENTS.md). The narrow single-file exclusion is the correct choice here — excluding the directory would have wrongly hidden a user's own .kimi-code/skills/ from git status. Good call.


Verdict: needs changes, though smaller ones than the header suggests. Item 3 is a new functional bug I'd want fixed. Items 1 and 2 were raised last round under "two smaller robustness items" and were neither addressed nor acknowledged — in fairness my summary that round said what I needed was §3, so I'm not treating them as a promise you broke, but they're still real and still open. All three fixes are small, and two of them remove code rather than add it.

The credential-lifecycle work from rounds 2–4 reads well, and the tests track it closely — restart round-trip, concurrent-edit preservation, refuse-overwrite, and both fail-closed land/merge paths. Thanks for the persistence through a long review.

@LarryHu0217

Copy link
Copy Markdown
Contributor Author

The latest head 061ec67 includes the remaining hardening from your last review: per-task Kimi config refresh failures are isolated, the token-history scan is scoped to the task branch with an explicit buffer, and only the selected auto-discovery candidate is parsed and validated. The Kimi command/preamble tests and Docker pin guard are also aligned. The branch is clean and ready for a fresh re-review when convenient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add native support for Kimi CLI / Kimi K2.6 agent

2 participants