diff --git a/.husky/pre-commit b/.husky/pre-commit index f653a3328..8678aafcb 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -1,18 +1,65 @@ #!/bin/sh +set -e + npx lint-staged npm run check # Verify package-lock.json is committed and in sync with package.json if git diff --cached --name-only | grep -q "package\.json$"; then - if ! git diff --cached --name-only | grep -q "package-lock\.json$"; then - echo "Error: package.json changed without updating package-lock.json" - echo "Run 'npm install' to update the lockfile" - exit 1 + if ! git diff --cached --name-only -- package-lock.json | grep -qx "package-lock\.json"; then + # Development commands are not recorded in the lockfile. Compare the index + # with HEAD, retaining all other fields and installation lifecycle scripts. + if ! node --input-type=module <<'NODE' +import { execFileSync } from 'node:child_process'; +import process from 'node:process'; +import { isDeepStrictEqual } from 'node:util'; + +function readManifest(revision) { + const manifest = JSON.parse(execFileSync('git', ['show', revision], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + })); + if (!manifest || typeof manifest !== 'object' || Array.isArray(manifest)) { + throw new Error('Invalid manifest'); + } + const scripts = manifest.scripts ?? {}; + if (typeof scripts !== 'object' || Array.isArray(scripts)) { + throw new Error('Invalid scripts'); + } + const installScripts = [ + 'preinstall', 'install', 'postinstall', + 'preprepare', 'prepare', 'postprepare', 'prepublish', + ].map((name) => scripts[name]); + delete manifest.scripts; + return { manifest, installScripts }; +} + +try { + const paths = execFileSync('git', ['diff', '--cached', '--name-only', '-z'], { encoding: 'utf8' }) + .split('\0').filter((path) => /(^|\/)package\.json$/.test(path)); + process.exitCode = paths.every((path) => isDeepStrictEqual( + readManifest('HEAD:' + path), readManifest(':' + path), + )) ? 0 : 1; +} catch { + process.exitCode = 1; +} +NODE + then + echo "Error: package.json changed without updating package-lock.json" + echo "Run 'npm install' to update the lockfile" + exit 1 + fi fi fi +# The lockfile must remain in the commit, not just exist in the working tree. +if ! git ls-files --error-unmatch -- package-lock.json >/dev/null 2>&1; then + echo "Error: package-lock.json must be tracked" + exit 1 +fi + # Ensure package-lock.json is not gitignored (supply chain: lockfile must be tracked) -if git check-ignore -q package-lock.json 2>/dev/null; then +if git check-ignore --no-index -q package-lock.json 2>/dev/null; then echo "Error: package-lock.json must not be gitignored — it provides integrity hashes" exit 1 fi diff --git a/AGENTS.md b/AGENTS.md index 6c54ef222..e6aa45b88 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,7 +22,7 @@ Electron desktop app for running coding agents in isolated Git worktrees. Deskto - CI tests that Semgrep rules work on fixtures; it does not scan the repository with Semgrep. `npm run lint:security` and `npm run lint:secrets` run separate scans and require Semgrep and Gitleaks respectively. - happy-dom cannot verify native Electron views. For browser-preview changes, follow the native smoke checks in `docs/browser-preview.md`. -When committing, use conventional commit messages, such as `fix(terminal): restore focus`. Git hooks enforce the format and run `lint-staged`, `npm run check`, and a lockfile check on commit; pushing runs `check` and `npm test`. Changes to `package.json` must include the corresponding `package-lock.json` update. +When committing, use conventional commit messages, such as `fix(terminal): restore focus`. Git hooks enforce the format and run `lint-staged`, `npm run check`, and a lockfile check on commit; pushing runs `check` and `npm test`. Changes to dependency or package metadata and installation lifecycle scripts in `package.json` must include the corresponding `package-lock.json` update. Development-script-only changes do not require a no-op lockfile update. The lockfile must remain tracked and must not be ignored. ## Architecture and conventions diff --git a/PRIVACY.md b/PRIVACY.md index c6a2f9992..83a939344 100644 --- a/PRIVACY.md +++ b/PRIVACY.md @@ -60,7 +60,7 @@ Unlike the AI CLIs above, the following network activity is initiated by Paralle - **Over Tailscale.** Traffic is carried by your tailnet — typically a direct WireGuard connection between your devices, but Tailscale's coordination service and (when direct connection is not possible) DERP relays may be involved per [Tailscale's network architecture](https://tailscale.com/kb/1257/connection-types). How Tailscale handles that traffic is governed by Tailscale's own policies, not this one. - **Sub-task coordinator (MCP)** — when sub-tasks run under a coordinator agent, Parallel Code starts a local token-protected HTTP/WebSocket server so sub-task agents can call back into the app (e.g. to signal completion). No traffic from this feature passes through infrastructure operated by the Parallel Code project. - **Bind address.** When MCP starts its own listener, it binds to `127.0.0.1` except on macOS Docker setups, where it binds to `0.0.0.0` so containers can reach it via `host.docker.internal` — this also makes the port reachable from other hosts on your LAN, though access still requires the token. If a Remote Access server is already running when a coordinator starts, the coordinator reuses that listener; because Remote Access binds to `0.0.0.0`, MCP routes inherit that LAN reach on any platform (including Linux), though access still requires the MCP token. - - **Where the token can land.** Token-bearing MCP data is written or passed in several places: a worktree `.mcp.json` when a worktree path is available, or a project-root `.mcp.json` otherwise, so the coordinator agent can auto-discover the server (Parallel Code also adds `.mcp.json` to your `.git/info/exclude` so it is not committed); a non-Docker coordinator config in your OS temp directory named `parallel-code-mcp-.json`; per-sub-task configs in your OS temp directory for host-mode sub-tasks (`parallel-code-subtask-.json`) or under the coordinator's `.parallel-code/` directory for Docker sub-tasks (`subtask-.json`); and short-lived `.parallel-code-atomic-.tmp` files written next to these configs during atomic-rename steps. These files are written with `0600` permissions where the platform supports it. + - **Where the token can land.** Token-bearing MCP data is written or passed in several places: a worktree `.mcp.json` when a worktree path is available, or a project-root `.mcp.json` otherwise, so the coordinator agent can auto-discover the server (Parallel Code also adds `.mcp.json` to your `.git/info/exclude` so it is not committed); an auto-discovered `.kimi-code/mcp.json` (or fallback `.mcp.json`) inside each Kimi sub-task worktree, with both that config and its adjacent `.parallel-code-atomic-*.tmp` files added to `.git/info/exclude` before credentials are written; a non-Docker coordinator config in your OS temp directory named `parallel-code-mcp-.json`; per-sub-task configs in your OS temp directory for host-mode sub-tasks (`parallel-code-subtask-.json`) or under the coordinator's `.parallel-code/` directory for Docker sub-tasks (`subtask-.json`); and short-lived `.parallel-code-atomic-.tmp` files written next to these configs during atomic-rename steps. These files are written with `0600` permissions where the platform supports it. - **Codex token in the command line.** For Codex specifically, the MCP token is passed as a literal command-line argument (`--config mcp_servers.parallel-code={... env = { PARALLEL_CODE_MCP_TOKEN = "..." }}`). Process command lines are visible to other processes — via `/proc//cmdline` on Linux or `ps` on macOS — so any local process that runs concurrently with a Codex sub-task can read that token and call back into the coordinator under its authority until the coordinator exits. Other agents receive the token through a token-protected file or env var instead. - **Docker task isolation** — when you enable Docker mode for a task, or when you opt coordinator sub-tasks into Docker-isolated mode, the agent launches in a container via `docker run --network host`. **Docker mode is not a security boundary.** It isolates the filesystem against the worktree, but does not isolate the network or credentials from the agent. - **Network.** `--network host` means the container shares the host's network namespace; its outbound reachability is the same as your host's, including loopback services and any LAN address your host can reach. diff --git a/README.md b/README.md index d53c9dbe2..5a547e30f 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@

- Works with Claude Code, Codex, and Gemini · Every change isolated in its own git worktree · Free, open source, no extra platform fee + Works with Claude Code, Codex, Gemini, and Docker-pinned Kimi Code · Every change isolated in its own git worktree · Free, open source, no extra platform fee

@@ -45,7 +45,7 @@ ## Why Parallel Code? -- **Use the AI coding tools you already trust** — [Claude Code](https://docs.anthropic.com/en/docs/claude-code), [Codex CLI](https://github.com/openai/codex), [Gemini CLI](https://github.com/google-gemini/gemini-cli), and [Copilot CLI](https://docs.github.com/en/copilot/concepts/agents/about-copilot-cli) — all from one interface. +- **Use the AI coding tools you already trust** — [Claude Code](https://docs.anthropic.com/en/docs/claude-code), [Codex CLI](https://github.com/openai/codex), [Gemini CLI](https://github.com/google-gemini/gemini-cli), Docker-pinned [Kimi Code CLI](https://github.com/MoonshotAI/kimi-code), and [Copilot CLI](https://docs.github.com/en/copilot/concepts/agents/about-copilot-cli) — all from one interface. - **Free and open source** — no extra subscription required. MIT licensed. - **Keep every change isolated and reviewable** — each task gets its own git branch and worktree automatically. - **Run agents in parallel, not in sequence** — five agents on five features at the same time, zero conflicts. @@ -121,10 +121,26 @@ When you're happy with the result, merge the branch back to main from the sideba - **macOS** — `.dmg` (universal) - **Linux** — `.AppImage` or `.deb` -2. **Install at least one AI coding CLI:** [Claude Code](https://docs.anthropic.com/en/docs/claude-code), [Codex CLI](https://github.com/openai/codex), [Gemini CLI](https://github.com/google-gemini/gemini-cli), [Antigravity CLI](https://antigravity.google/), or [Copilot CLI](https://docs.github.com/en/copilot/concepts/agents/about-copilot-cli) +2. **Install at least one AI coding CLI:** [Claude Code](https://docs.anthropic.com/en/docs/claude-code), [Codex CLI](https://github.com/openai/codex), [Gemini CLI](https://github.com/google-gemini/gemini-cli), [Antigravity CLI](https://antigravity.google/), or [Copilot CLI](https://docs.github.com/en/copilot/concepts/agents/about-copilot-cli). Kimi Code is currently supported through the bundled Docker image instead of a native installation. 3. **Open Parallel Code**, point it at a git repo, and start dispatching tasks. +

+Kimi Code: use Docker mode + +Parallel Code's Kimi integration writes an auto-discovered project MCP config into each fresh +task worktree. Kimi Code 0.33 added a workspace-trust prompt, and 0.36 defaults to declining +project MCP launch targets in that prompt. A current native Kimi installation can therefore +pause on every new worktree instead of starting the task unattended. + +Use Docker-isolated Kimi tasks for now. The bundled image pins Kimi Code 0.32, before the +workspace-trust gate. Native Kimi support is not currently claimed. +Kimi is hidden from native task agent pickers, and native launches (including saved/custom +`kimi` definitions) are rejected with an actionable Docker-mode error. Existing running +terminals can still reattach after a renderer reload. + +
+
Antigravity CLI: run natively, not in Docker isolation diff --git a/docker/Dockerfile b/docker/Dockerfile index f775de193..1063cfad1 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -50,7 +50,8 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ RUN ln -sf "$(command -v fdfind)" /usr/local/bin/fd 2>/dev/null || true # AI agent CLIs — must be present so Docker-mode tasks can execute them -RUN npm install -g @anthropic-ai/claude-code @openai/codex @google/gemini-cli opencode-ai +# Keep Kimi below 0.33: newer releases block fresh worktrees on workspace trust. +RUN npm install -g @anthropic-ai/claude-code @openai/codex @google/gemini-cli opencode-ai @moonshot-ai/kimi-code@0.32.0 # Antigravity CLI (agy) — distributed as a Go binary via the official installer # (not on npm). The installer's `--dir` flag drops the binary straight into a diff --git a/electron/ipc/agents.ts b/electron/ipc/agents.ts index e58b60be9..049f2a587 100644 --- a/electron/ipc/agents.ts +++ b/electron/ipc/agents.ts @@ -44,6 +44,15 @@ const DEFAULT_AGENTS: AgentDef[] = [ skip_permissions_args: getSkipPermissionsArgs('gemini'), description: "Google's Gemini CLI agent", }, + { + id: 'kimi', + name: 'Kimi Code CLI', + command: 'kimi', + args: [], + resume_args: ['--continue'], + skip_permissions_args: getSkipPermissionsArgs('kimi'), + description: "Moonshot AI's Kimi Code CLI agent", + }, { id: 'opencode', name: 'OpenCode', diff --git a/electron/ipc/git-exclude-batch.test.ts b/electron/ipc/git-exclude-batch.test.ts new file mode 100644 index 000000000..51ebec0f4 --- /dev/null +++ b/electron/ipc/git-exclude-batch.test.ts @@ -0,0 +1,126 @@ +import * as childProcess from 'child_process'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { appendGitInfoExcludeBlocks } from './git-exclude.js'; + +vi.mock('child_process', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, execFileSync: vi.fn(actual.execFileSync) }; +}); + +describe('batched Git exclusions', () => { + let dir: string; + let excludePath: string; + const patterns = ['/.kimi-code/mcp.json', '/.kimi-code/.parallel-code-atomic-*.tmp']; + const blocks = patterns.map((marker) => ({ marker, block: `${marker}\n` })); + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'git-exclude-batch-')); + childProcess.execFileSync('git', ['init', '-q', dir]); + excludePath = path.join(dir, '.git/info/exclude'); + vi.mocked(childProcess.execFileSync).mockClear(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it.each(['', '/.kimi-code/mcp.json\n', '/.kimi-code/.parallel-code-atomic-*.tmp\n'])( + 'resolves and reads once and appends only missing patterns after %j', + (existing) => { + fs.writeFileSync(excludePath, existing); + const read = vi.spyOn(fs, 'readFileSync'); + const append = vi.spyOn(fs, 'appendFileSync'); + expect(appendGitInfoExcludeBlocks(dir, blocks)).toBe('appended'); + expect(childProcess.execFileSync).toHaveBeenCalledTimes(1); + expect(read).toHaveBeenCalledTimes(1); + expect(append).toHaveBeenCalledTimes(1); + const written = append.mock.calls[0][1]; + for (const pattern of patterns) { + expect(String(written).includes(pattern)).toBe(!existing.includes(pattern)); + } + for (const file of ['.kimi-code/mcp.json', '.kimi-code/.parallel-code-atomic-test.tmp']) { + expect(() => + childProcess.execFileSync('git', ['check-ignore', '-q', file], { cwd: dir }), + ).not.toThrow(); + } + append.mockClear(); + expect(appendGitInfoExcludeBlocks(dir, blocks)).toBe('present'); + expect(append).not.toHaveBeenCalled(); + }, + ); + + it('recognizes Git-normalized existing lines without rewriting them', () => { + const existing = `${patterns[0]} \r\n${patterns[1]}\r\n# user rule\n/user-data\n`; + fs.writeFileSync(excludePath, existing); + const append = vi.spyOn(fs, 'appendFileSync'); + expect(appendGitInfoExcludeBlocks(dir, blocks)).toBe('present'); + expect(append).not.toHaveBeenCalled(); + expect(fs.readFileSync(excludePath, 'utf8')).toBe(existing); + }); + + it('creates a missing exclude file with both entries', () => { + fs.unlinkSync(excludePath); + expect(appendGitInfoExcludeBlocks(dir, blocks)).toBe('appended'); + expect(fs.readFileSync(excludePath, 'utf8')).toBe(patterns.join('\n') + '\n'); + }); + + it.each(['', '.kimi-code/'])('keeps %j exclusions root-anchored in real Git', (prefix) => { + const files = [`${prefix}mcp.json`, `${prefix}.parallel-code-atomic-test.tmp`]; + if (!prefix) files[0] = '.mcp.json'; + const rules = [`/${files[0]}`, `/${prefix}.parallel-code-atomic-*.tmp`]; + expect( + appendGitInfoExcludeBlocks( + dir, + rules.map((marker) => ({ marker, block: marker })), + ), + ).toBe('appended'); + for (const file of files) { + expect(childProcess.spawnSync('git', ['check-ignore', '-q', file], { cwd: dir }).status).toBe( + 0, + ); + expect( + childProcess.spawnSync('git', ['check-ignore', '-q', `nested/${file}`], { cwd: dir }) + .status, + ).toBe(1); + } + }); + + it('reports only the missing pattern when its append fails', () => { + fs.writeFileSync(excludePath, blocks[0].block); + const error = new Error('append denied'); + vi.spyOn(fs, 'appendFileSync').mockImplementationOnce(() => { + throw error; + }); + const onError = vi.fn(); + expect(appendGitInfoExcludeBlocks(dir, blocks, onError)).toBe('failed'); + expect(onError).toHaveBeenCalledWith(error, [patterns[1]]); + expect(fs.readFileSync(excludePath, 'utf8')).toBe(blocks[0].block); + }); + + it('fails without writing when the exclude file cannot be read', () => { + const error = Object.assign(new Error('read denied'), { code: 'EACCES' }); + vi.spyOn(fs, 'readFileSync').mockImplementationOnce(() => { + throw error; + }); + const append = vi.spyOn(fs, 'appendFileSync'); + const onError = vi.fn(); + expect(appendGitInfoExcludeBlocks(dir, blocks, onError)).toBe('failed'); + expect(onError).toHaveBeenCalledWith(error, patterns); + expect(append).not.toHaveBeenCalled(); + }); + + it('does not read or write if Git cannot resolve the common directory', () => { + vi.mocked(childProcess.execFileSync).mockImplementationOnce(() => { + throw new Error('git timeout'); + }); + const read = vi.spyOn(fs, 'readFileSync'); + const append = vi.spyOn(fs, 'appendFileSync'); + expect(appendGitInfoExcludeBlocks(dir, blocks)).toBe('missing'); + expect(read).not.toHaveBeenCalled(); + expect(append).not.toHaveBeenCalled(); + }); +}); diff --git a/electron/ipc/git-exclude.ts b/electron/ipc/git-exclude.ts index cd99b3be4..cfb144cab 100644 --- a/electron/ipc/git-exclude.ts +++ b/electron/ipc/git-exclude.ts @@ -91,3 +91,41 @@ export function appendGitInfoExcludeBlock( if (!excludePath) return 'missing'; return appendGitInfoExcludeBlockAtPath(excludePath, marker, block, onError); } + +/** Resolve and read once, then append only the missing blocks in a single write. */ +export function appendGitInfoExcludeBlocks( + worktreePath: string, + blocks: ReadonlyArray<{ marker: string; block: string }>, + onError?: (err: unknown, markers: string[]) => void, +): AppendGitInfoExcludeResult { + if (blocks.length === 0) return 'present'; + const excludePath = resolveGitInfoExcludePath(worktreePath); + if (!excludePath) return 'missing'; + let existing = ''; + try { + existing = fs.readFileSync(excludePath, 'utf8'); + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { + onError?.( + err, + blocks.map(({ marker }) => marker), + ); + return 'failed'; + } + } + const known = new Set(existing.split('\n').map(normalizeExcludeLine)); + const missing = blocks.filter(({ marker }) => !known.has(marker)); + if (missing.length === 0) return 'present'; + return appendGitInfoExcludeBlockAtPath( + excludePath, + missing[0].marker, + missing.map(({ block }) => (block.endsWith('\n') ? block : `${block}\n`)).join(''), + (err) => + onError?.( + err, + missing.map(({ marker }) => marker), + ), + existing, + true, + ); +} diff --git a/electron/ipc/plans.test.ts b/electron/ipc/plans.test.ts index 82ca2cc2c..eadab8713 100644 --- a/electron/ipc/plans.test.ts +++ b/electron/ipc/plans.test.ts @@ -2,6 +2,7 @@ import fs from 'node:fs'; import { execFileSync } from 'node:child_process'; import os from 'node:os'; import path from 'node:path'; +import { EventEmitter } from 'node:events'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { IPC } from './channels.js'; import { readPlanForWorktree, startPlanWatcher, stopAllPlanWatchers } from './plans.js'; @@ -15,6 +16,7 @@ beforeEach(() => { afterEach(() => { stopAllPlanWatchers(); + vi.restoreAllMocks(); fs.rmSync(worktreePath, { recursive: true, force: true }); }); @@ -49,6 +51,60 @@ function watchPlans() { } describe('root-level plan files', () => { + it.each(['example-plan.md', null])( + 'ignores delayed startup notifications for %s but publishes a real edit', + async (fileName) => { + writeFile('example-plan.md', '# Leftover'); + let notify: ((event: string, name: string | null) => void) | undefined; + vi.spyOn(fs, 'watch').mockImplementation((dir, listener) => { + if (String(dir) === worktreePath) notify = listener as typeof notify; + const watcher = new EventEmitter(); + return Object.assign(watcher, { + close: () => watcher.emit('close'), + }) as unknown as fs.FSWatcher; + }); + const send = watchPlans(); + if (!notify) throw new Error('Root watcher was not registered'); + notify('rename', fileName); + await vi.waitFor(() => + expect(send).toHaveBeenLastCalledWith( + IPC.PlanContent, + expect.objectContaining({ content: '# Leftover', recovered: true }), + ), + ); + await new Promise((resolve) => setTimeout(resolve, 350)); + expect(send).toHaveBeenCalledTimes(1); + + writeFile('example-plan.md', '# New live content'); + notify('change', fileName); + await vi.waitFor(() => + expect(send).toHaveBeenLastCalledWith( + IPC.PlanContent, + expect.objectContaining({ content: '# New live content', recovered: undefined }), + ), + ); + }, + ); + + it('discovers a new plan even when the native watcher drops its notification', async () => { + vi.spyOn(fs, 'watch').mockImplementation(() => { + const watcher = new EventEmitter(); + return Object.assign(watcher, { + close: () => watcher.emit('close'), + }) as unknown as fs.FSWatcher; + }); + const send = watchPlans(); + writeFile('example-plan.md', '# Missed notification'); + await vi.waitFor(() => + expect(send).toHaveBeenLastCalledWith( + IPC.PlanContent, + expect.objectContaining({ content: '# Missed notification', recovered: undefined }), + ), + ); + await new Promise((resolve) => setTimeout(resolve, 350)); + expect(send).toHaveBeenCalledTimes(1); + }); + it.each(['example-plan.md', 'PLAN.md', 'implementation_plan.md', 'plan.v2.md'])( 'reads %s for plan review and restores it by filename', (fileName) => { diff --git a/electron/ipc/plans.ts b/electron/ipc/plans.ts index 27952410e..a6fe822e7 100644 --- a/electron/ipc/plans.ts +++ b/electron/ipc/plans.ts @@ -221,9 +221,38 @@ function isPlanInDirs( /** Start watching a single directory. Returns the watcher or null on failure. */ function watchDir(worktreePath: string, dir: string, onChange: () => void): fs.FSWatcher | null { try { + // Filesystem notifications may arrive after watch registration for writes + // that already happened. Only a change from the registration snapshot is + // live; otherwise startup recovery must retain its recovered semantics. + const snapshot = () => { + const files = new Map(); + for (const name of planFileNames(worktreePath, dir)) { + try { + const stat = fs.statSync(path.join(dir, name)); + files.set(name, `${stat.ino}:${stat.size}:${stat.mtimeMs}:${stat.ctimeMs}`); + } catch { + // A plan may disappear while the directory is being scanned. + } + } + return files; + }; + let previous = snapshot(); + const reconcile = () => { + const current = snapshot(); + const changed = + current.size !== previous.size || + [...current].some(([name, state]) => previous.get(name) !== state); + previous = current; + if (changed) onChange(); + }; const watcher = fs.watch(dir, (_event, fileName) => { - if (fileName === null || isPlanFile(worktreePath, dir, fileName.toString())) onChange(); + if (fileName === null || isPlanFile(worktreePath, dir, fileName.toString())) reconcile(); }); + // Native watchers can miss a write immediately after a directory is created + // and registered (notably on macOS). Reconcile the same snapshot so a lost + // notification neither hides a new plan nor produces duplicate publishes. + const reconcileTimer = setInterval(reconcile, 250); + watcher.on('close', () => clearInterval(reconcileTimer)); watcher.on('error', (err) => { console.warn(`Plan watcher error for ${dir}:`, err); }); diff --git a/electron/ipc/pty.test.ts b/electron/ipc/pty.test.ts index 23cedd10c..e0f4ff77f 100644 --- a/electron/ipc/pty.test.ts +++ b/electron/ipc/pty.test.ts @@ -459,6 +459,7 @@ describe('spawnAgent docker mode', () => { ['opencode', '.config/opencode'], ['copilot', '.config/github-copilot'], ['agy', '.gemini/antigravity-cli'], + ['kimi', '.kimi-code'], ])( '%s bind-mounts a user-owned host directory when shareDockerAgentAuth is enabled', async (command, relDir) => { @@ -1031,6 +1032,46 @@ describe('spawnAgent session reattach', () => { }); }); +describe('Kimi Docker-only support', () => { + it.each(['kimi', '/opt/bin/kimi'])('rejects a native %s before spawning', async (command) => { + await expect( + spawnAgent(createMockNotify(), buildSpawnArgs({ command, dockerMode: false })), + ).rejects.toThrow('Kimi Code requires Docker mode'); + expect(mockPtySpawn).not.toHaveBeenCalled(); + expect(mockExecFileSync).not.toHaveBeenCalled(); + }); + + it('allows a Docker Kimi launch without a host Kimi installation', async () => { + await spawnAgent(createMockNotify(), buildSpawnArgs({ command: 'kimi' })); + expect(getLastSpawnCall().command).toBe('docker'); + expect(getLastSpawnCall().args).toContain('kimi'); + expect(mockExecFileSync).not.toHaveBeenCalledWith('which', ['kimi'], expect.anything()); + }); + + it('does not kill an existing PTY when a native Kimi replacement is rejected', async () => { + const notify = createMockNotify(); + const args = buildSpawnArgs(); + await spawnAgent(notify, args); + const proc = mockPtySpawn.mock.results[0].value; + await expect( + spawnAgent(notify, { ...args, command: 'kimi', dockerMode: false }), + ).rejects.toThrow('Kimi Code requires Docker mode'); + expect(proc.kill).not.toHaveBeenCalled(); + }); + + it('reattaches before applying new-launch eligibility checks', async () => { + const notify = createMockNotify(); + const args = buildSpawnArgs({ command: 'kimi' }); + await spawnAgent(notify, args); + const proc = mockPtySpawn.mock.results[0].value; + await expect( + spawnAgent(notify, { ...args, dockerMode: false, attachExisting: true }), + ).resolves.not.toThrow(); + expect(mockPtySpawn).toHaveBeenCalledTimes(1); + expect(proc.resume).toHaveBeenCalled(); + }); +}); + describe('spawnAgent output batching', () => { function dataMessages(notify: Mock): string[] { return vi diff --git a/electron/ipc/pty.ts b/electron/ipc/pty.ts index 99060168a..ef0abdaa0 100644 --- a/electron/ipc/pty.ts +++ b/electron/ipc/pty.ts @@ -29,6 +29,7 @@ import { } from '../agent-hooks/observations.js'; import { HOOK_PTY_ENV_KEYS } from '../agent-hooks/hook-script.js'; import { isClaudeCommand, withClaudeHookSettings } from '../agent-hooks/launch-args.js'; +import { isAgentSupportedInMode } from '../shared/agent-support.js'; import { debug as logDebug, warn as logWarn } from '../log.js'; const __filename = fileURLToPath(import.meta.url); @@ -776,6 +777,9 @@ export async function spawnAgent( } // In Docker mode, we validate `docker` exists rather than the inner command + if (!isAgentSupportedInMode(command, args.dockerMode)) { + throw new Error('Kimi Code requires Docker mode. Enable Docker isolation to start this agent.'); + } if (!args.dockerMode) { validateCommand(command); } else { @@ -1239,6 +1243,7 @@ const AGENT_CONFIG_DIRS: Record = { opencode: ['.config/opencode'], copilot: ['.config/github-copilot'], agy: ['.gemini/antigravity-cli'], + kimi: ['.kimi-code'], }; // Config files (not directories) each agent CLI uses for auth, relative to HOME. diff --git a/electron/ipc/register-mcp.test.ts b/electron/ipc/register-mcp.test.ts index e31b558e7..ffa8c145d 100644 --- a/electron/ipc/register-mcp.test.ts +++ b/electron/ipc/register-mcp.test.ts @@ -13,6 +13,8 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; +import { execFileSync } from 'child_process'; +import * as atomic from '../mcp/atomic.js'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { buildCoordinatorLaunchArgs, @@ -20,6 +22,7 @@ import { removeCoordinatorTempConfig, selectMcpJsonDir, validateStartMCPServerArgs, + writeCoordinatorMcpJson, } from './register.js'; import { getDockerMcpServerDestPath } from './mcp-paths.js'; import { getMCPRemoteServerUrl } from '../mcp/config.js'; @@ -51,6 +54,42 @@ afterEach(() => { // Layer 3: Spawn-path integration // ───────────────────────────────────────────────────────────────────────────── +describe('coordinator MCP Git exclusions', () => { + it('installs both exclusions before the atomic writer, including on an existing config', () => { + const dir = mkTemp(); + execFileSync('git', ['init', '-q', dir]); + fs.writeFileSync(path.join(dir, '.git/info/exclude'), '.mcp.json\n'); + const actualWrite = atomic.atomicWriteFileSync; + const write = vi.spyOn(atomic, 'atomicWriteFileSync').mockImplementation((file, data, opts) => { + for (const name of ['.mcp.json', '.parallel-code-atomic-probe.tmp']) { + expect( + execFileSync('git', ['check-ignore', name], { cwd: dir, encoding: 'utf8' }).trim(), + ).toBe(name); + } + actualWrite(file, data, opts); + }); + const target = path.join(dir, '.mcp.json'); + writeCoordinatorMcpJson(target, '{"mcpServers":{}}'); + expect(write).toHaveBeenCalledOnce(); + expect(fs.readFileSync(target, 'utf8')).toBe('{"mcpServers":{}}'); + expect(fs.statSync(target).mode & 0o777).toBe(0o600); + }); + + it('keeps a crash-left temporary file ignored when the atomic write fails', () => { + const dir = mkTemp(); + execFileSync('git', ['init', '-q', dir]); + vi.spyOn(atomic, 'atomicWriteFileSync').mockImplementation(() => { + fs.writeFileSync(path.join(dir, '.parallel-code-atomic-crash.tmp'), 'synthetic-token'); + throw new Error('simulated crash before rename'); + }); + expect(() => writeCoordinatorMcpJson(path.join(dir, '.mcp.json'), '{}')).toThrow( + 'simulated crash before rename', + ); + expect(execFileSync('git', ['status', '--porcelain'], { cwd: dir, encoding: 'utf8' })).toBe(''); + expect(fs.existsSync(path.join(dir, '.parallel-code-atomic-crash.tmp'))).toBe(true); + }); +}); + describe('Layer 3 — MCP startup pipeline (no Electron, real FS)', () => { it('generated .mcp.json in worktree matches production structure', () => { const worktreePath = mkTemp(); diff --git a/electron/ipc/register.ts b/electron/ipc/register.ts index 925e36e59..8e4924e34 100644 --- a/electron/ipc/register.ts +++ b/electron/ipc/register.ts @@ -177,6 +177,20 @@ export function selectMcpJsonDir(worktreePath: string | undefined, projectRoot: return worktreePath ?? projectRoot; } +/** Exclude both the final config and crash-left atomic files before either is written. */ +export function writeCoordinatorMcpJson(configPath: string, content: string): void { + const configDir = path.dirname(configPath); + for (const pattern of ['/.mcp.json', '/.parallel-code-atomic-*.tmp']) { + appendGitInfoExcludeBlock( + configDir, + pattern, + `# Parallel Code MCP config (contains ephemeral token)\n${pattern}\n`, + (err) => console.warn(`[MCP] Could not git-exclude ${pattern}:`, err), + ); + } + atomicWriteFileSync(configPath, content, { mode: 0o600 }); +} + export interface CoordinatorMCPConfigOpts { mcpServerPath: string; serverUrl: string; @@ -2014,6 +2028,7 @@ export function registerAllHandlers(win: BrowserWindow): void { landingSummary?: string; landedMetadata?: import('../mcp/types.js').LandedMetadata; mcpConfigPath?: string; + autoDiscoveredMcpConfig?: import('../mcp/types.js').AutoDiscoveredMcpConfigState; agentCommand?: string; preambleFileExistedBefore?: boolean; initialPrompt?: string; @@ -2068,6 +2083,7 @@ export function registerAllHandlers(win: BrowserWindow): void { landingSummary: args.landingSummary, landedMetadata: args.landedMetadata, mcpConfigPath: args.mcpConfigPath, + autoDiscoveredMcpConfig: args.autoDiscoveredMcpConfig, agentCommand: args.agentCommand, preambleFileExistedBefore: args.preambleFileExistedBefore, initialPrompt: args.initialPrompt, @@ -2247,7 +2263,7 @@ export function registerAllHandlers(win: BrowserWindow): void { // parallel-code key so we don't destroy user-defined entries. Track whether // we created the file so deregisterCoordinator can clean up correctly. if (mcpJsonDir && worktreeMcpPath && mergedMcpJson !== undefined) { - atomicWriteFileSync(worktreeMcpPath, mergedMcpJson, { mode: 0o600 }); + writeCoordinatorMcpJson(worktreeMcpPath, mergedMcpJson); const writtenMcpParallelCode: unknown = mcpConfig.mcpServers['parallel-code']; coordinator.setMcpJsonInfo( args.coordinatorTaskId, @@ -2257,14 +2273,6 @@ export function registerAllHandlers(win: BrowserWindow): void { writtenMcpParallelCode, ); - // Append to .git/info/exclude (local-only gitignore, not committed) - appendGitInfoExcludeBlock( - mcpJsonDir, - '.mcp.json', - '# Parallel Code MCP config (contains ephemeral token)\n.mcp.json\n', - (err) => console.warn('[MCP] Could not git-exclude .mcp.json:', err), - ); - console.warn('[MCP] .mcp.json written to:', worktreeMcpPath); const staleWarning = detectStaleDockerMCPUrl(serverUrl, args.dockerContainerName); diff --git a/electron/ipc/shared-types.ts b/electron/ipc/shared-types.ts index 3d0d5a202..8676cea09 100644 --- a/electron/ipc/shared-types.ts +++ b/electron/ipc/shared-types.ts @@ -1,3 +1,9 @@ +/** Persisted ownership fingerprint for an auto-discovered MCP configuration. */ +export interface AutoDiscoveredMcpConfigState { + path: string; + writtenParallelCodeFingerprint: string; +} + export type PtyOutput = | { type: 'Data'; data: Uint8Array } // raw terminal bytes | { diff --git a/electron/mcp/agent-args.test.ts b/electron/mcp/agent-args.test.ts index de837fd20..7c831b46c 100644 --- a/electron/mcp/agent-args.test.ts +++ b/electron/mcp/agent-args.test.ts @@ -6,6 +6,7 @@ import { isAntigravityCommand, isCodexCommand, isCopilotCommand, + isKimiCommand, } from './agent-args.js'; const config = { @@ -75,6 +76,16 @@ describe('MCP agent launch args', () => { expect(buildMcpLaunchArgs('agy', '/tmp/config.json', config)).toEqual([]); }); + it('detects Kimi commands by executable name', () => { + expect(isKimiCommand('kimi')).toBe(true); + expect(isKimiCommand('/home/agent/.local/bin/kimi')).toBe(true); + expect(isKimiCommand('claude')).toBe(false); + }); + + it('emits no --mcp-config for Kimi Code', () => { + expect(buildMcpLaunchArgs('kimi', '/tmp/config.json', config)).toEqual([]); + }); + it('detects copilot commands by executable name', () => { expect(isCopilotCommand('copilot')).toBe(true); expect(isCopilotCommand('/opt/homebrew/bin/copilot')).toBe(true); diff --git a/electron/mcp/agent-args.ts b/electron/mcp/agent-args.ts index 58265cd4d..915f8af7d 100644 --- a/electron/mcp/agent-args.ts +++ b/electron/mcp/agent-args.ts @@ -18,6 +18,10 @@ export function isAntigravityCommand(command: string): boolean { return command.split('/').pop() === 'agy'; } +export function isKimiCommand(command: string): boolean { + return command.split('/').pop() === 'kimi'; +} + export function isCopilotCommand(command: string): boolean { return command.split('/').pop() === 'copilot'; } @@ -54,6 +58,11 @@ export function buildMcpLaunchArgs( if (isAntigravityCommand(command)) { return []; } + // Kimi Code auto-discovers user and project MCP config files and does not + // accept the generic `--mcp-config` flag. + if (isKimiCommand(command)) { + return []; + } // Copilot has no `--mcp-config` flag — passing it makes Copilot exit immediately // with "unknown option" before the prompt is ever sent (#146). It accepts // `--additional-mcp-config <@file|json>` (and also auto-discovers a workspace diff --git a/electron/mcp/coordinator-hydration.test.ts b/electron/mcp/coordinator-hydration.test.ts new file mode 100644 index 000000000..de416c509 --- /dev/null +++ b/electron/mcp/coordinator-hydration.test.ts @@ -0,0 +1,185 @@ +import { randomUUID } from 'crypto'; +import os from 'os'; +import { join } from 'path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + setupCoordinatorHarness, + resetCoordinatorMocks, + mockReadFileSync, + mockExistsSync, + mockUnlinkSync, + mockAtomicWriteFileSync, + mockAppendGitInfoExcludeBlocks, + mockNotifyRenderer, + mockNotify, +} from './coordinator-test-harness.js'; + +const { Coordinator } = await setupCoordinatorHarness(); +const fs = await vi.importActual('fs'); + +describe('existing Kimi task hydration failure', () => { + let dir: string; + let configPath: string; + let worktreeConfig: string; + let coordinator: InstanceType; + let args: Parameters['hydrateTask']>[0]; + + function getHydratedTask() { + const task = coordinator.getTask(args.id); + if (!task) throw new Error('Expected the fixture task to be hydrated'); + return task; + } + + beforeEach(() => { + resetCoordinatorMocks(); + dir = fs.mkdtempSync(join(os.tmpdir(), 'kimi-hydration-')); + fs.mkdirSync(join(dir, '.kimi-code')); + const id = randomUUID(); + configPath = join(os.tmpdir(), `parallel-code-subtask-${id}.json`); + worktreeConfig = join(dir, '.kimi-code/mcp.json'); + mockExistsSync.mockImplementation((file) => fs.existsSync(file)); + mockReadFileSync.mockImplementation((file, encoding) => fs.readFileSync(file, encoding)); + mockUnlinkSync.mockImplementation((file) => fs.unlinkSync(file)); + mockAtomicWriteFileSync.mockImplementation((file, data) => + fs.writeFileSync(file, data, { mode: 0o600 }), + ); + coordinator = new Coordinator(); + coordinator.setNotify(mockNotify); + coordinator.registerCoordinator('coord', 'project'); + coordinator.setMCPServerInfo( + 'coord', + 'http://localhost:3001', + 'coord-token', + 'old-token', + '/server.js', + ); + args = { + id, + name: 'child', + projectId: 'project', + projectRoot: dir, + branchName: 'task/child', + worktreePath: dir, + agentId: 'agent', + coordinatorTaskId: 'coord', + agentCommand: 'kimi', + mcpConfigPath: configPath, + }; + coordinator.hydrateTask(args); + }); + + afterEach(() => { + fs.rmSync(configPath, { force: true }); + fs.rmSync(dir, { recursive: true, force: true }); + resetCoordinatorMocks(); + }); + + it.each(['exclude', 'unresolved exclude', 'worktree write', 'per-task write'])( + 'preserves both configs and live state after a failed %s, then permits retry', + (failure) => { + const task = getHydratedTask(); + // Older persisted tasks can lack a done token; a failed hydration must not + // publish the newly generated one to just one of the two config files. + delete task.doneToken; + const before = { ...task }; + const originalConfig = fs.readFileSync(configPath, 'utf8'); + const originalWorktree = fs.readFileSync(worktreeConfig, 'utf8'); + mockNotifyRenderer.mockClear(); + if (failure === 'exclude' || failure === 'unresolved exclude') { + mockAppendGitInfoExcludeBlocks.mockReturnValueOnce( + failure === 'exclude' ? 'failed' : 'missing', + ); + } else { + const failingPath = failure === 'worktree write' ? worktreeConfig : configPath; + mockAtomicWriteFileSync.mockImplementationOnce((file, data) => { + if (file === failingPath) throw new Error('simulated write failure'); + fs.writeFileSync(file, data, { mode: 0o600 }); + mockAtomicWriteFileSync.mockImplementationOnce(() => { + throw new Error('simulated write failure'); + }); + }); + } + + expect(() => + coordinator.hydrateTask({ ...args, agentCommand: '/usr/local/bin/kimi' }), + ).toThrow(); + expect(fs.readFileSync(configPath, 'utf8')).toBe(originalConfig); + expect(fs.readFileSync(worktreeConfig, 'utf8')).toBe(originalWorktree); + expect(coordinator.getTask(args.id)).toBe(task); + expect(task).toEqual(before); + expect(mockNotifyRenderer).not.toHaveBeenCalledWith('mcp_task_state_sync', expect.anything()); + + const result = coordinator.hydrateTask({ ...args, agentCommand: '/usr/local/bin/kimi' }); + const config = JSON.parse(fs.readFileSync(configPath, 'utf8')); + const worktree = JSON.parse(fs.readFileSync(worktreeConfig, 'utf8')); + expect(config.mcpServers['parallel-code']).toEqual(worktree.mcpServers['parallel-code']); + expect(task.agentCommand).toBe('/usr/local/bin/kimi'); + expect(task.doneToken).toBeTruthy(); + expect(result.autoDiscoveredMcpConfig).toEqual(task.autoDiscoveredMcpConfig); + }, + ); + + it('removes only the newly created per-task config when worktree setup fails', () => { + fs.unlinkSync(configPath); + const task = getHydratedTask(); + const before = { ...task }; + const originalWorktree = fs.readFileSync(worktreeConfig, 'utf8'); + mockAppendGitInfoExcludeBlocks.mockReturnValueOnce('failed'); + + expect(() => coordinator.hydrateTask(args)).toThrow('Unable to git-exclude'); + expect(fs.existsSync(configPath)).toBe(false); + expect(fs.readFileSync(worktreeConfig, 'utf8')).toBe(originalWorktree); + expect(task).toEqual(before); + expect(() => coordinator.hydrateTask(args)).not.toThrow(); + expect(fs.existsSync(configPath)).toBe(true); + }); + + it('does not write either config if the previous per-task config cannot be read', () => { + const task = getHydratedTask(); + const before = { ...task }; + mockAtomicWriteFileSync.mockClear(); + mockReadFileSync.mockImplementationOnce(() => { + throw Object.assign(new Error('permission denied'), { code: 'EACCES' }); + }); + + expect(() => coordinator.hydrateTask(args)).toThrow('permission denied'); + expect(mockAtomicWriteFileSync).not.toHaveBeenCalled(); + expect(task).toEqual(before); + }); + + it('surfaces a rollback failure without publishing staged task state', () => { + const task = getHydratedTask(); + delete task.doneToken; + const before = { ...task }; + mockNotifyRenderer.mockClear(); + mockAppendGitInfoExcludeBlocks.mockReturnValueOnce('failed'); + mockAtomicWriteFileSync + .mockImplementationOnce((file, data) => fs.writeFileSync(file, data, { mode: 0o600 })) + .mockImplementationOnce(() => { + throw new Error('rollback write failed'); + }); + + expect(() => coordinator.hydrateTask(args)).toThrow( + 'Task hydration failed and the previous per-task MCP config could not be restored.', + ); + expect(task).toEqual(before); + expect(mockNotifyRenderer).not.toHaveBeenCalled(); + }); + + it('keeps both committed configs and live state consistent if renderer notification fails', () => { + const task = getHydratedTask(); + delete task.doneToken; + mockNotifyRenderer.mockImplementationOnce(() => { + throw new Error('renderer unavailable'); + }); + + expect(() => coordinator.hydrateTask(args)).toThrow('renderer unavailable'); + const config = JSON.parse(fs.readFileSync(configPath, 'utf8')); + const worktree = JSON.parse(fs.readFileSync(worktreeConfig, 'utf8')); + expect(config.mcpServers['parallel-code']).toEqual(worktree.mcpServers['parallel-code']); + expect(config.mcpServers['parallel-code'].env.PARALLEL_CODE_MCP_DONE_TOKEN).toBe( + task.doneToken, + ); + expect(() => coordinator.hydrateTask(args)).not.toThrow(); + }); +}); diff --git a/electron/mcp/coordinator-test-harness.ts b/electron/mcp/coordinator-test-harness.ts index ca9a4046f..82915ebb5 100644 --- a/electron/mcp/coordinator-test-harness.ts +++ b/electron/mcp/coordinator-test-harness.ts @@ -25,6 +25,7 @@ const enoent = () => Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); const mocks = vi.hoisted(() => { const mockExecFile = vi.fn(); + const mockSpawnSync = vi.fn(); const mockWriteFileSync = vi.fn(); const mockReadFileSync = vi.fn(); const mockExistsSync = vi.fn(); @@ -37,6 +38,7 @@ const mocks = vi.hoisted(() => { const mockFsMkdir = vi.fn(); const mockAtomicWriteFileSync = vi.fn(); const mockAtomicWriteFile = vi.fn(); + const mockAppendGitInfoExcludeBlocks = vi.fn(); const mockNotifyRenderer = vi.fn(); const mockLogInfo = vi.fn(); const mockLogWarn = vi.fn(); @@ -62,6 +64,7 @@ const mocks = vi.hoisted(() => { return { mockExecFile, + mockSpawnSync, mockWriteFileSync, mockReadFileSync, mockExistsSync, @@ -75,6 +78,7 @@ const mocks = vi.hoisted(() => { mockFsMkdir, mockAtomicWriteFileSync, mockAtomicWriteFile, + mockAppendGitInfoExcludeBlocks, mockNotifyRenderer, mockLogInfo, mockLogWarn, @@ -101,6 +105,7 @@ const mocks = vi.hoisted(() => { vi.mock('child_process', () => ({ execFile: mocks.mockExecFile, + spawnSync: mocks.mockSpawnSync, })); vi.mock('fs', () => ({ @@ -125,6 +130,10 @@ vi.mock('./atomic.js', () => ({ atomicWriteFile: mocks.mockAtomicWriteFile, })); +vi.mock('../ipc/git-exclude.js', () => ({ + appendGitInfoExcludeBlocks: mocks.mockAppendGitInfoExcludeBlocks, +})); + vi.mock('../shared/prompt-detect.js', () => ({ stripAnsi: (s: string) => s.replace( @@ -247,6 +256,7 @@ vi.mock('../log.js', () => ({ export const { mockExecFile, + mockSpawnSync, mockWriteFileSync, mockReadFileSync, mockExistsSync, @@ -260,6 +270,7 @@ export const { mockFsMkdir, mockAtomicWriteFileSync, mockAtomicWriteFile, + mockAppendGitInfoExcludeBlocks, mockNotifyRenderer, mockLogInfo, mockLogWarn, @@ -312,6 +323,8 @@ export function resetCoordinatorMocks(): void { return { on: vi.fn() }; }, ); + mockSpawnSync.mockReset(); + mockSpawnSync.mockReturnValue({ status: 1, error: undefined, stderr: Buffer.alloc(0) }); mockWriteFileSync.mockReset(); mockReadFileSync.mockReset(); @@ -337,6 +350,8 @@ export function resetCoordinatorMocks(): void { mockAtomicWriteFileSync.mockReset(); mockAtomicWriteFile.mockReset(); mockAtomicWriteFile.mockResolvedValue(undefined); + mockAppendGitInfoExcludeBlocks.mockReset(); + mockAppendGitInfoExcludeBlocks.mockReturnValue('appended'); mockNotifyRenderer.mockReset(); mockLogInfo.mockReset(); diff --git a/electron/mcp/coordinator.test.ts b/electron/mcp/coordinator.test.ts index d28c2145f..8fd29690d 100644 --- a/electron/mcp/coordinator.test.ts +++ b/electron/mcp/coordinator.test.ts @@ -16,6 +16,7 @@ import { import { setupCoordinatorHarness, mockExecFile, + mockSpawnSync, mockReadFileSync, mockExistsSync, mockUnlinkSync, @@ -23,8 +24,10 @@ import { mockFsAccess, mockAtomicWriteFileSync, mockAtomicWriteFile, + mockAppendGitInfoExcludeBlocks, mockNotifyRenderer, mockLogInfo, + mockLogWarn, mockSpawnAgent, mockWriteToAgent, mockSubscribeToAgent, @@ -1546,6 +1549,481 @@ describe('Coordinator land_self', () => { }); }); + it('restores the Kimi auto-discovered config before checking and merging the worktree', async () => { + const configPath = '/tmp/test/.kimi-code/mcp.json'; + const originalConfig = JSON.stringify({ + mcpServers: { other: { command: 'user-owned-server' } }, + }); + let currentConfig = originalConfig; + mockExistsSync.mockImplementation((path) => path === configPath); + mockReadFileSync.mockImplementation((path) => + path === configPath ? currentConfig : '# existing\n', + ); + mockAtomicWriteFileSync.mockImplementation((path, raw) => { + if (path === configPath) currentConfig = raw as string; + }); + mockExecFile.mockImplementation( + ( + _cmd: string, + args: string[], + _opts: unknown, + cb: (err: Error | null, stdout: string, stderr: string) => void, + ) => { + if (args.join(' ') === 'rev-parse --abbrev-ref HEAD') { + cb(null, 'task/test\n', ''); + return; + } + if (args[0] === 'status') { + cb(null, '', ''); + return; + } + if (args.join(' ') === 'rev-parse HEAD') { + cb(null, 'landed-sha\n', ''); + return; + } + cb(null, '', ''); + }, + ); + + const kimiCoordinator = new Coordinator(); + kimiCoordinator.setNotify(mockNotify); + kimiCoordinator.setDefaultProject('proj-1', '/tmp/project'); + kimiCoordinator.registerCoordinator('coord-kimi', 'proj-1', { + worktreePath: '/tmp/project', + }); + kimiCoordinator.setCoordinatorSpawnDefaults('coord-kimi', 'kimi', []); + kimiCoordinator.setMCPServerInfo( + 'coord-kimi', + 'http://localhost:3001', + 'coordinator-token', + 'subtask-token', + '/path/server.js', + ); + await kimiCoordinator.createTask({ + name: 'test', + prompt: 'do', + coordinatorTaskId: 'coord-kimi', + }); + + await kimiCoordinator.landSelf('task-1', { verification }); + + const restored = JSON.parse(currentConfig) as { mcpServers: Record }; + expect(restored.mcpServers['parallel-code']).toBeUndefined(); + expect(restored.mcpServers.other).toEqual({ command: 'user-owned-server' }); + expect(vi.mocked(mergeTask)).toHaveBeenCalled(); + }); + + it.each(['file', 'entry'])('recovers missing Kimi %s', async (missing) => { + const configPath = '/tmp/test/.kimi-code/mcp.json'; + let configExists = true; + let taskConfig = ''; + let currentConfig = JSON.stringify({ + mcpServers: { other: { command: 'user-owned-server' } }, + }); + mockExistsSync.mockImplementation( + (path) => + (path === configPath && configExists) || + (typeof path === 'string' && path.includes('parallel-code-subtask-')), + ); + mockReadFileSync.mockImplementation((path) => { + if (path === configPath) return currentConfig; + if (typeof path === 'string' && path.includes('parallel-code-subtask-')) return taskConfig; + return '# existing\n'; + }); + mockAtomicWriteFileSync.mockImplementation((path, raw) => { + if (path === configPath) currentConfig = raw as string; + }); + mockAtomicWriteFile.mockImplementation(async (path, raw) => { + if (typeof path === 'string' && path.includes('parallel-code-subtask-')) { + taskConfig = raw as string; + } + }); + + const kimiCoordinator = new Coordinator(); + kimiCoordinator.setNotify(mockNotify); + kimiCoordinator.setDefaultProject('proj-1', '/tmp/project'); + kimiCoordinator.registerCoordinator('coord-kimi', 'proj-1', { + worktreePath: '/tmp/project', + }); + kimiCoordinator.setCoordinatorSpawnDefaults('coord-kimi', 'kimi', []); + kimiCoordinator.setMCPServerInfo( + 'coord-kimi', + 'http://localhost:3001', + 'coordinator-token', + 'subtask-token', + '/path/server.js', + ); + await kimiCoordinator.createTask({ + name: 'test', + prompt: 'do', + coordinatorTaskId: 'coord-kimi', + }); + + const withoutManagedEntry = JSON.parse(currentConfig) as { + mcpServers: Record; + }; + delete withoutManagedEntry.mcpServers['parallel-code']; + currentConfig = JSON.stringify(withoutManagedEntry); + if (missing === 'file') configExists = false; + + await kimiCoordinator.landSelf('task-1', { verification }); + + expect(JSON.parse(currentConfig).mcpServers).toEqual({ + other: { command: 'user-owned-server' }, + }); + expect(vi.mocked(mergeTask)).toHaveBeenCalled(); + }); + + it.each(['file', 'entry'])('rejects unrecoverable Kimi %s', async (missing) => { + const configPath = '/tmp/test/.kimi-code/mcp.json'; + let configExists = true; + let currentConfig = JSON.stringify({ + mcpServers: { other: { command: 'user-owned-server' } }, + }); + mockExistsSync.mockImplementation((path) => path === configPath && configExists); + mockReadFileSync.mockImplementation((path) => + path === configPath ? currentConfig : '# existing\n', + ); + mockAtomicWriteFileSync.mockImplementation((path, raw) => { + if (path === configPath) currentConfig = raw as string; + }); + + const kimiCoordinator = new Coordinator(); + kimiCoordinator.setNotify(mockNotify); + kimiCoordinator.setDefaultProject('proj-1', '/tmp/project'); + kimiCoordinator.registerCoordinator('coord-kimi', 'proj-1', { + worktreePath: '/tmp/project', + }); + kimiCoordinator.setCoordinatorSpawnDefaults('coord-kimi', 'kimi', []); + kimiCoordinator.setMCPServerInfo( + 'coord-kimi', + 'http://localhost:3001', + 'coordinator-token', + 'subtask-token', + '/path/server.js', + ); + await kimiCoordinator.createTask({ + name: 'test', + prompt: 'do', + coordinatorTaskId: 'coord-kimi', + }); + + const withoutManagedEntry = JSON.parse(currentConfig) as { + mcpServers: Record; + }; + delete withoutManagedEntry.mcpServers['parallel-code']; + currentConfig = JSON.stringify(withoutManagedEntry); + if (missing === 'file') configExists = false; + + await expect(kimiCoordinator.landSelf('task-1', { verification })).rejects.toThrow( + 'Unable to restore managed Kimi MCP config', + ); + + expect(vi.mocked(mergeTask)).not.toHaveBeenCalled(); + expect(kimiCoordinator.getTask('task-1')?.autoDiscoveredMcpConfig).toBeDefined(); + expect(kimiCoordinator.getTask('task-1')?.landingState).toBe('landing_escalated'); + // Landing failed closed, but the child keeps a working parallel-code server + // so it can report the escalation instead of going silent. + const rearmed = JSON.parse(currentConfig) as { + mcpServers: Record }>; + }; + expect(rearmed.mcpServers['parallel-code'].env['PARALLEL_CODE_MCP_TOKEN']).toBe( + 'subtask-token', + ); + }); + + it('leaves a foreign parallel-code entry alone when landing fails closed', async () => { + const configPath = '/tmp/test/.kimi-code/mcp.json'; + let currentConfig = JSON.stringify({ + mcpServers: { other: { command: 'user-owned-server' } }, + }); + mockExistsSync.mockImplementation((path) => path === configPath); + mockReadFileSync.mockImplementation((path) => + path === configPath ? currentConfig : '# existing\n', + ); + mockAtomicWriteFileSync.mockImplementation((path, raw) => { + if (path === configPath) currentConfig = raw as string; + }); + + const kimiCoordinator = new Coordinator(); + kimiCoordinator.setNotify(mockNotify); + kimiCoordinator.setDefaultProject('proj-1', '/tmp/project'); + kimiCoordinator.registerCoordinator('coord-kimi', 'proj-1', { + worktreePath: '/tmp/project', + }); + kimiCoordinator.setCoordinatorSpawnDefaults('coord-kimi', 'kimi', []); + kimiCoordinator.setMCPServerInfo( + 'coord-kimi', + 'http://localhost:3001', + 'coordinator-token', + 'subtask-token', + '/path/server.js', + ); + await kimiCoordinator.createTask({ + name: 'test', + prompt: 'do', + coordinatorTaskId: 'coord-kimi', + }); + + const tampered = JSON.parse(currentConfig) as { mcpServers: Record }; + tampered.mcpServers['parallel-code'] = { command: 'not-ours' }; + currentConfig = JSON.stringify(tampered); + + await expect(kimiCoordinator.landSelf('task-1', { verification })).rejects.toThrow( + 'Unable to restore managed Kimi MCP config', + ); + + expect(vi.mocked(mergeTask)).not.toHaveBeenCalled(); + expect( + (JSON.parse(currentConfig) as { mcpServers: Record }).mcpServers[ + 'parallel-code' + ], + ).toEqual({ command: 'not-ours' }); + }); + + it('fails closed on token-bearing history even when the discovery config was deleted', async () => { + const configPath = '/tmp/test/.kimi-code/mcp.json'; + let autoConfigExists = true; + let taskConfig = ''; + let currentConfig = JSON.stringify({ + mcpServers: { other: { command: 'user-owned-server' } }, + }); + mockExistsSync.mockImplementation( + (path) => + (path === configPath && autoConfigExists) || + (typeof path === 'string' && path.includes('parallel-code-subtask-')), + ); + mockReadFileSync.mockImplementation((path) => { + if (path === configPath) return currentConfig; + if (typeof path === 'string' && path.includes('parallel-code-subtask-')) return taskConfig; + return '# existing\n'; + }); + mockAtomicWriteFileSync.mockImplementation((path, raw) => { + if (path === configPath) currentConfig = raw as string; + }); + mockAtomicWriteFile.mockImplementation(async (path, raw) => { + if (typeof path === 'string' && path.includes('parallel-code-subtask-')) { + taskConfig = raw as string; + } + }); + mockExecFile.mockImplementation( + ( + _cmd: string, + args: string[], + _opts: unknown, + cb: (err: Error | null, stdout: string, stderr: string) => void, + ) => { + if (args[0] === 'log') { + cb(null, '+ PARALLEL_CODE_MCP_TOKEN=subtask-token\n', ''); + return; + } + cb(null, '', ''); + }, + ); + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-token', + 'subtask-token', + '/path/server.js', + ); + await coordinator.createTask({ name: 'test', prompt: 'do', coordinatorTaskId: 'coord-1' }); + autoConfigExists = false; + + await expect(coordinator.landSelf('task-1', { verification })).rejects.toThrow( + 'Managed Kimi MCP token was found in task Git history', + ); + + expect(vi.mocked(mergeTask)).not.toHaveBeenCalled(); + expect(coordinator.getTask('task-1')?.landingState).toBe('landing_escalated'); + const historyCall = mockExecFile.mock.calls.find(([, args]) => args[0] === 'log'); + expect(historyCall?.[1]).toEqual(expect.arrayContaining(['-m', '--text', '--no-textconv'])); + expect(historyCall?.[1]).toContain(':(glob)**/.parallel-code-atomic-*.tmp'); + + // Exercise the production pathspec with real Git: a bare glob misses nested files. + const realFs = await vi.importActual('fs'); + const realProcess = await vi.importActual('child_process'); + const repo = realFs.mkdtempSync(join(os.tmpdir(), 'kimi-token-history-')); + try { + realProcess.execFileSync('git', ['init', '-q', repo]); + realFs.mkdirSync(join(repo, '.kimi-code')); + realFs.writeFileSync(join(repo, '.parallel-code-atomic-root.tmp'), 'synthetic-root-token'); + realFs.writeFileSync( + join(repo, '.kimi-code/.parallel-code-atomic-child.tmp'), + 'synthetic-child-token', + ); + realProcess.execFileSync('git', ['add', '-f', '.'], { cwd: repo }); + realProcess.execFileSync( + 'git', + [ + '-c', + 'user.name=Test', + '-c', + 'user.email=test@example.com', + '-c', + 'commit.gpgsign=false', + 'commit', + '-qm', + 'fixture', + ], + { cwd: repo }, + ); + if (!historyCall) throw new Error('Expected a production Git history query'); + const args = [...historyCall[1]]; + args[1] = 'HEAD'; + const history = realProcess.execFileSync('git', args, { cwd: repo, encoding: 'utf8' }); + expect(history).toContain('synthetic-root-token'); + expect(history).toContain('synthetic-child-token'); + const oldHistory = realProcess.execFileSync( + 'git', + args.filter((arg) => arg !== ':(glob)**/.parallel-code-atomic-*.tmp'), + { cwd: repo, encoding: 'utf8' }, + ); + expect(oldHistory).toBe(''); + } finally { + realFs.rmSync(repo, { recursive: true, force: true }); + } + }); + + it('fails closed before self-landing when Kimi MCP restoration fingerprint mismatches', async () => { + const configPath = '/tmp/test/.kimi-code/mcp.json'; + let currentConfig = JSON.stringify({ + mcpServers: { other: { command: 'user-owned-server' } }, + }); + mockExistsSync.mockImplementation((path) => path === configPath); + mockReadFileSync.mockImplementation((path) => + path === configPath ? currentConfig : '# existing\n', + ); + mockAtomicWriteFileSync.mockImplementation((path, raw) => { + if (path === configPath) currentConfig = raw as string; + }); + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-token', + 'subtask-token', + '/path/server.js', + ); + await coordinator.createTask({ name: 'test', prompt: 'do', coordinatorTaskId: 'coord-1' }); + + currentConfig = JSON.stringify({ + mcpServers: { 'parallel-code': { command: 'changed-generated-entry' } }, + }); + mockExecFile.mockClear(); + + const editedConfig = currentConfig; + const failure = await coordinator + .landSelf('task-1', { verification }) + .catch((error: unknown) => error); + expect(failure).toBeInstanceOf(Error); + const message = (failure as Error).message; + expect(message).toContain('Unable to restore managed Kimi MCP config'); + expect(message).toContain(configPath); + expect(message).toContain('remove only that entry and retry'); + expect(message).toContain('keep other MCP servers intact'); + expect(message).not.toContain('subtask-token'); + expect(currentConfig).toBe(editedConfig); + + expect(vi.mocked(mergeTask)).not.toHaveBeenCalled(); + expect(mockExecFile).not.toHaveBeenCalledWith( + 'git', + expect.arrayContaining(['status']), + expect.anything(), + expect.anything(), + ); + expect(coordinator.getTask('task-1')?.autoDiscoveredMcpConfig).toBeDefined(); + expect(coordinator.getTask('task-1')?.landingState).toBe('landing_escalated'); + }); + + it('fails closed before merge staging when Kimi MCP restoration fingerprint mismatches', async () => { + const configPath = '/tmp/test/.kimi-code/mcp.json'; + let currentConfig = JSON.stringify({ + mcpServers: { other: { command: 'user-owned-server' } }, + }); + mockExistsSync.mockImplementation((path) => path === configPath); + mockReadFileSync.mockImplementation((path) => + path === configPath ? currentConfig : '# existing\n', + ); + mockAtomicWriteFileSync.mockImplementation((path, raw) => { + if (path === configPath) currentConfig = raw as string; + }); + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-token', + 'subtask-token', + '/path/server.js', + ); + const task = await coordinator.createTask({ + name: 'test', + prompt: 'do', + coordinatorTaskId: 'coord-1', + }); + task.signalDoneAt = new Date(); + currentConfig = JSON.stringify({ + mcpServers: { 'parallel-code': { command: 'changed-generated-entry' } }, + }); + mockExecFile.mockClear(); + + const editedConfig = currentConfig; + const failure = await coordinator.mergeTask('task-1').catch((error: unknown) => error); + expect(failure).toBeInstanceOf(Error); + const message = (failure as Error).message; + expect(message).toContain('Unable to restore managed Kimi MCP config'); + expect(message).toContain(configPath); + expect(message).toContain('remove only that entry and retry'); + expect(message).toContain('Do not commit this config or its tokens'); + expect(message).not.toContain('subtask-token'); + expect(currentConfig).toBe(editedConfig); + + expect(mockExecFile).not.toHaveBeenCalledWith( + 'git', + expect.arrayContaining(['add', '-A']), + expect.anything(), + expect.anything(), + ); + expect(vi.mocked(mergeTask)).not.toHaveBeenCalled(); + expect(coordinator.getTask('task-1')?.autoDiscoveredMcpConfig).toBeDefined(); + }); + + it('refuses desktop-approved integration when the Kimi MCP entry was changed', async () => { + const task = coordinator.getTask('task-1'); + expect(task).toBeDefined(); + if (!task) return; + task.integrationPolicy = 'review'; + task.baseBranch = 'main'; + task.signalDoneAt = new Date(); + task.agentCommand = 'kimi'; + task.autoDiscoveredMcpConfig = { + path: '/tmp/test/.kimi-code/mcp.json', + writtenParallelCodeFingerprint: 'a'.repeat(64), + }; + mockReadFileSync.mockReturnValue( + JSON.stringify({ mcpServers: { 'parallel-code': { command: 'user-changed' } } }), + ); + mockExecFile.mockClear(); + + await expect( + coordinator.approveAndMergeTask(task.id, { + expectedCommit: 'a'.repeat(40), + expectedTargetBranch: 'main', + expectedTargetCommit: 'b'.repeat(40), + }), + ).rejects.toThrow('Unable to restore managed Kimi MCP config'); + + expect(mockUnlinkSync).not.toHaveBeenCalled(); + expect(mockExecFile).not.toHaveBeenCalledWith( + 'git', + expect.arrayContaining(['add', '-A']), + expect.anything(), + expect.anything(), + ); + expect(vi.mocked(mergeTask)).not.toHaveBeenCalled(); + }); + it('stages a landed notification so the coordinator hears about successful self-land', async () => { await coordinator.landSelf('task-1', { verification, summary: 'done' }); @@ -1580,6 +2058,35 @@ describe('Coordinator land_self', () => { outputTail: 'ok\n', }; + it.each(['passed', 'failed'] as const)( + 'auto-commits before merge verification even when verification is %s', + async (status) => { + coordinator.registerCoordinator('coord-1', 'proj-1', { verifyCommand: 'npm test' }); + const task = coordinator.getTask('task-1'); + if (!task) throw new Error('Expected the fixture task'); + task.status = 'exited'; + mockExecFile.mockClear(); + mockVerifyStart.mockImplementationOnce(async () => { + expect(mockExecFile).toHaveBeenCalledWith( + 'git', + ['commit', '-m', 'WIP: auto-commit before merge'], + expect.objectContaining({ cwd: '/tmp/test' }), + expect.any(Function), + ); + expect(vi.mocked(mergeTask)).not.toHaveBeenCalled(); + return { ...passedRun, status, exitCode: status === 'passed' ? 0 : 1 }; + }); + if (status === 'passed') { + await coordinator.mergeTask('task-1'); + expect(vi.mocked(mergeTask)).toHaveBeenCalled(); + } else { + await expect(coordinator.mergeTask('task-1')).rejects.toThrow('Verification failed'); + expect(vi.mocked(mergeTask)).not.toHaveBeenCalled(); + } + expect(mockVerifyStart).toHaveBeenCalledTimes(1); + }, + ); + it('runs the command in the task worktree and lands when it passes', async () => { coordinator.registerCoordinator('coord-1', 'proj-1', { verifyCommand: 'npm test' }); mockVerifyStart.mockResolvedValueOnce(passedRun); @@ -1630,6 +2137,58 @@ describe('Coordinator land_self', () => { }); }); + it('restores the managed Kimi MCP entry when verification fails', async () => { + const configPath = '/tmp/test/.kimi-code/mcp.json'; + let currentConfig = JSON.stringify({ + mcpServers: { other: { command: 'user-owned-server' } }, + }); + mockExistsSync.mockImplementation((path) => path === configPath); + mockReadFileSync.mockImplementation((path) => + path === configPath ? currentConfig : '# existing\n', + ); + mockAtomicWriteFileSync.mockImplementation((path, raw) => { + if (path === configPath) currentConfig = raw as string; + }); + const kimiCoordinator = new Coordinator(); + kimiCoordinator.setNotify(mockNotify); + kimiCoordinator.setDefaultProject('proj-1', '/tmp/project'); + kimiCoordinator.registerCoordinator('coord-kimi', 'proj-1', { + worktreePath: '/tmp/project', + verifyCommand: 'npm test', + }); + kimiCoordinator.setCoordinatorSpawnDefaults('coord-kimi', 'kimi', []); + kimiCoordinator.setMCPServerInfo( + 'coord-kimi', + 'http://localhost:3001', + 'coordinator-token', + 'subtask-token', + '/path/server.js', + ); + await kimiCoordinator.createTask({ + name: 'test', + prompt: 'do', + coordinatorTaskId: 'coord-kimi', + }); + mockVerifyStart.mockResolvedValueOnce({ + ...passedRun, + status: 'failed', + exitCode: 1, + outputTail: '1 failed\n', + }); + + await expect(kimiCoordinator.landSelf('task-1', { verification })).rejects.toThrow( + 'Verification failed', + ); + + const refreshed = JSON.parse(currentConfig) as { + mcpServers: { 'parallel-code': { env: Record } }; + }; + expect(refreshed.mcpServers['parallel-code'].env['PARALLEL_CODE_MCP_TOKEN']).toBe( + 'subtask-token', + ); + expect(vi.mocked(mergeTask)).not.toHaveBeenCalled(); + }); + it('skips the check when the coordinator has no verify command', async () => { await coordinator.landSelf('task-1', { verification, summary: 'done' }); @@ -3268,6 +3827,8 @@ describe('Coordinator sub-task MCP config isolation', () => { beforeEach(() => { vi.clearAllMocks(); + mockSpawnSync.mockReset(); + mockSpawnSync.mockReturnValue({ status: 1, error: undefined, stderr: Buffer.alloc(0) }); mockExistsSync.mockReturnValue(false); coordinator = new Coordinator(); coordinator.setNotify(mockNotify); @@ -3334,6 +3895,483 @@ describe('Coordinator sub-task MCP config isolation', () => { expect(configPaths[0]).not.toBe(configPaths[1]); }); + + it('writes isolated Kimi child configs to each worktree for auto-discovery', async () => { + mockCreateBackendTask + .mockResolvedValueOnce({ id: 'task-a', branch_name: 'task/a', worktree_path: '/tmp/a' }) + .mockResolvedValueOnce({ id: 'task-b', branch_name: 'task/b', worktree_path: '/tmp/b' }); + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-tok', + 'subtask-tok', + '/path/server.js', + ); + + await coordinator.createTask({ name: 'task-a', prompt: 'do a', coordinatorTaskId: 'coord-1' }); + await coordinator.createTask({ name: 'task-b', prompt: 'do b', coordinatorTaskId: 'coord-1' }); + + const childWrites = mockAtomicWriteFileSync.mock.calls.filter( + ([configPath]) => + configPath === '/tmp/a/.kimi-code/mcp.json' || configPath === '/tmp/b/.kimi-code/mcp.json', + ); + expect(childWrites).toHaveLength(2); + const childConfigs = childWrites.map( + ([, raw]) => + JSON.parse(raw as string) as { + mcpServers: { + 'parallel-code': { args: string[]; env: Record }; + }; + }, + ); + + expect(childConfigs[0].mcpServers['parallel-code'].args).toContain('task-a'); + expect(childConfigs[1].mcpServers['parallel-code'].args).toContain('task-b'); + expect(childConfigs[0].mcpServers['parallel-code'].env['PARALLEL_CODE_MCP_TOKEN']).toBe( + 'subtask-tok', + ); + expect( + childConfigs[0].mcpServers['parallel-code'].env['PARALLEL_CODE_MCP_DONE_TOKEN'], + ).not.toBe(childConfigs[1].mcpServers['parallel-code'].env['PARALLEL_CODE_MCP_DONE_TOKEN']); + expect(mockAppendGitInfoExcludeBlocks).toHaveBeenCalledWith( + '/tmp/a', + [ + { marker: '/.kimi-code/mcp.json', block: expect.stringContaining('/.kimi-code/mcp.json') }, + { + marker: '/.kimi-code/.parallel-code-atomic-*.tmp', + block: expect.stringContaining('/.kimi-code/.parallel-code-atomic-*.tmp'), + }, + ], + expect.any(Function), + ); + for (const [, spawnOpts] of mockSpawnAgent.mock.calls) { + expect(spawnOpts).toEqual( + expect.objectContaining({ + command: 'kimi', + args: expect.not.arrayContaining(['--mcp-config']), + }), + ); + } + }); + + it('uses the alternate Kimi discovery path when the preferred path is tracked', async () => { + mockSpawnSync.mockImplementation((_command: string, args: string[]) => ({ + status: args[args.length - 1] === '.kimi-code/mcp.json' ? 0 : 1, + error: undefined, + stderr: Buffer.alloc(0), + })); + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-tok', + 'subtask-tok', + '/path/server.js', + ); + + const task = await coordinator.createTask({ + name: 'test', + prompt: 'do', + coordinatorTaskId: 'coord-1', + }); + + expect(task.autoDiscoveredMcpConfig?.path).toBe('/tmp/test/.mcp.json'); + expect(mockAtomicWriteFileSync).toHaveBeenCalledWith( + '/tmp/test/.mcp.json', + expect.stringContaining('subtask-tok'), + { mode: 0o600 }, + ); + // Root-level patterns carry no slash of their own, so they must be anchored + // explicitly or git would also match a user's nested `.mcp.json`. + expect(mockAppendGitInfoExcludeBlocks).toHaveBeenCalledWith( + '/tmp/test', + [ + { marker: '/.mcp.json', block: expect.stringContaining('/.mcp.json') }, + { + marker: '/.parallel-code-atomic-*.tmp', + block: expect.stringContaining('/.parallel-code-atomic-*.tmp'), + }, + ], + expect.any(Function), + ); + }); + + it('rejects a tracked preferred Kimi config that would override the fallback server', async () => { + const preferredPath = '/tmp/test/.kimi-code/mcp.json'; + mockSpawnSync.mockImplementation((_command: string, args: string[]) => ({ + status: args[args.length - 1] === '.kimi-code/mcp.json' ? 0 : 1, + error: undefined, + stderr: Buffer.alloc(0), + })); + mockExistsSync.mockImplementation((path) => path === preferredPath); + mockReadFileSync.mockImplementation((path) => + path === preferredPath + ? JSON.stringify({ mcpServers: { 'parallel-code': { command: 'tracked-server' } } }) + : '# existing\n', + ); + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-tok', + 'subtask-tok', + '/path/server.js', + ); + + await expect( + coordinator.createTask({ name: 'test', prompt: 'do', coordinatorTaskId: 'coord-1' }), + ).rejects.toThrow('.kimi-code/mcp.json already defines mcpServers["parallel-code"]'); + expect(mockAtomicWriteFileSync).not.toHaveBeenCalledWith( + '/tmp/test/.mcp.json', + expect.anything(), + expect.anything(), + ); + expect(mockSpawnAgent).not.toHaveBeenCalled(); + }); + + it('only parses the selected Kimi discovery path', async () => { + mockSpawnSync.mockImplementation((_command: string, args: string[]) => ({ + status: args[args.length - 1] === '.mcp.json' ? 0 : 1, + error: undefined, + stderr: Buffer.alloc(0), + })); + mockExistsSync.mockImplementation((path) => path === '/tmp/test/.mcp.json'); + mockReadFileSync.mockImplementation((path) => { + if (path === '/tmp/test/.mcp.json') throw new Error('unused candidate must not be parsed'); + return '# existing\n'; + }); + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-tok', + 'subtask-tok', + '/path/server.js', + ); + + await expect( + coordinator.createTask({ name: 'test', prompt: 'do', coordinatorTaskId: 'coord-1' }), + ).resolves.toBeDefined(); + expect(mockReadFileSync).not.toHaveBeenCalledWith('/tmp/test/.mcp.json', 'utf-8'); + }); + + it('keeps restarting sibling Kimi configs after one task refresh fails', async () => { + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-tok', + 'subtask-tok', + '/path/server.js', + ); + await coordinator.createTask({ name: 'test', prompt: 'do', coordinatorTaskId: 'coord-1' }); + mockSpawnSync.mockImplementation(() => ({ + status: null, + error: new Error('worktree disappeared'), + stderr: Buffer.alloc(0), + })); + + expect(() => + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3002', + 'coordinator-tok-2', + 'subtask-tok-2', + '/path/server.js', + ), + ).not.toThrow(); + expect(mockLogWarn).toHaveBeenCalledWith( + 'coordinator.kimi_mcp', + 'failed to refresh Kimi child MCP config', + expect.objectContaining({ taskId: 'task-1' }), + ); + }); + + it('fails task creation when both Kimi discovery paths are tracked', async () => { + mockSpawnSync.mockReturnValue({ status: 0, error: undefined, stderr: Buffer.alloc(0) }); + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-tok', + 'subtask-tok', + '/path/server.js', + ); + + await expect( + coordinator.createTask({ name: 'test', prompt: 'do', coordinatorTaskId: 'coord-1' }), + ).rejects.toThrow('both .kimi-code/mcp.json and .mcp.json are tracked by Git'); + + expect(mockAtomicWriteFileSync).not.toHaveBeenCalledWith( + expect.stringMatching(/(?:\.kimi-code\/mcp|\.mcp)\.json$/), + expect.anything(), + expect.anything(), + ); + expect(mockSpawnAgent).not.toHaveBeenCalled(); + }); + + it('fails task creation instead of persisting a pre-existing parallel-code entry', async () => { + const configPath = '/tmp/test/.kimi-code/mcp.json'; + mockExistsSync.mockImplementation((path) => path === configPath); + mockReadFileSync.mockImplementation((path) => + path === configPath + ? JSON.stringify({ + mcpServers: { + 'parallel-code': { + command: 'user-owned-server', + env: { API_KEY: 'must-not-be-persisted' }, + }, + }, + }) + : '# existing\n', + ); + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-tok', + 'subtask-tok', + '/path/server.js', + ); + + await expect( + coordinator.createTask({ name: 'test', prompt: 'do', coordinatorTaskId: 'coord-1' }), + ).rejects.toThrow('already defines mcpServers["parallel-code"]'); + + expect(JSON.stringify(coordinator.getTask('task-1')) ?? '').not.toContain( + 'must-not-be-persisted', + ); + expect(mockNotifyRenderer).not.toHaveBeenCalledWith( + 'mcp_task_created', + expect.objectContaining({ autoDiscoveredMcpConfig: expect.anything() }), + ); + }); + + it('preserves concurrent Kimi config edits while removing its managed entry', async () => { + const configPath = '/tmp/test/.kimi-code/mcp.json'; + let currentConfig = JSON.stringify({ + mcpServers: { + other: { command: 'other-server' }, + }, + setting: true, + }); + mockExistsSync.mockImplementation((path) => path === configPath); + mockReadFileSync.mockImplementation((path) => + path === configPath ? currentConfig : '# existing\n', + ); + mockAtomicWriteFileSync.mockImplementation((path, raw) => { + if (path === configPath) currentConfig = raw as string; + }); + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-tok', + 'subtask-tok', + '/path/server.js', + ); + + await coordinator.createTask({ name: 'test', prompt: 'do', coordinatorTaskId: 'coord-1' }); + const concurrentlyEdited = JSON.parse(currentConfig) as { + mcpServers: Record; + setting: boolean | string; + }; + concurrentlyEdited.mcpServers.other = { command: 'edited-server' }; + concurrentlyEdited.setting = 'edited'; + currentConfig = JSON.stringify(concurrentlyEdited); + coordinator.deregisterCoordinator('coord-1'); + + const restored = JSON.parse(currentConfig) as { + mcpServers: Record; + setting: boolean | string; + }; + expect(restored.mcpServers['parallel-code']).toBeUndefined(); + expect(restored.mcpServers.other).toEqual({ command: 'edited-server' }); + expect(restored.setting).toBe('edited'); + }); + + it('persists only non-secret Kimi restoration metadata across restart hydration', async () => { + const configPath = '/tmp/test/.kimi-code/mcp.json'; + let currentConfig = JSON.stringify({ + mcpServers: { + other: { command: 'other-server' }, + }, + }); + mockExistsSync.mockImplementation((path) => path === configPath); + mockReadFileSync.mockImplementation((path) => + path === configPath ? currentConfig : '# existing\n', + ); + mockAtomicWriteFileSync.mockImplementation((path, raw) => { + if (path === configPath) currentConfig = raw as string; + }); + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'old-coordinator-token', + 'old-subtask-token', + '/path/server.js', + ); + const task = await coordinator.createTask({ + name: 'test', + prompt: 'do', + coordinatorTaskId: 'coord-1', + }); + const persistedState = task.autoDiscoveredMcpConfig; + expect(persistedState).toEqual({ + path: configPath, + writtenParallelCodeFingerprint: expect.stringMatching(/^[a-f0-9]{64}$/), + }); + expect(JSON.stringify(persistedState)).not.toContain('old-subtask-token'); + expect(JSON.stringify(persistedState)).not.toContain('other-server'); + + const restarted = new Coordinator(); + restarted.setNotify(mockNotify); + restarted.setDefaultProject('proj-1', '/tmp/project'); + restarted.registerCoordinator('coord-1', 'proj-1'); + restarted.setMCPServerInfo( + 'coord-1', + 'http://localhost:3002', + 'new-coordinator-token', + 'new-subtask-token', + '/path/server.js', + ); + const result = restarted.hydrateTask({ + id: task.id, + name: task.name, + projectId: task.projectId, + projectRoot: task.projectRoot, + branchName: task.branchName, + worktreePath: task.worktreePath, + agentId: task.agentId, + coordinatorTaskId: task.coordinatorTaskId, + mcpConfigPath: task.mcpConfigPath, + autoDiscoveredMcpConfig: persistedState, + agentCommand: 'kimi', + }); + + expect(result.autoDiscoveredMcpConfig).toEqual({ + path: configPath, + writtenParallelCodeFingerprint: expect.stringMatching(/^[a-f0-9]{64}$/), + }); + const refreshed = JSON.parse(currentConfig) as { + mcpServers: { 'parallel-code': { env: Record } }; + }; + expect(refreshed.mcpServers['parallel-code'].env['PARALLEL_CODE_MCP_TOKEN']).toBe( + 'new-subtask-token', + ); + + restarted.deregisterCoordinator('coord-1'); + + const restored = JSON.parse(currentConfig) as { mcpServers: Record }; + expect(restored.mcpServers['parallel-code']).toBeUndefined(); + expect(restored.mcpServers.other).toEqual({ command: 'other-server' }); + }); + + it('does not overwrite a Kimi child MCP entry changed after creation', async () => { + const configPath = '/tmp/test/.kimi-code/mcp.json'; + let configExists = false; + let currentConfig = ''; + mockExistsSync.mockImplementation((path) => path === configPath && configExists); + mockReadFileSync.mockImplementation((path) => + path === configPath ? currentConfig : '# existing\n', + ); + mockAtomicWriteFileSync.mockImplementation((path, raw) => { + if (path === configPath) { + configExists = true; + currentConfig = raw as string; + } + }); + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-tok', + 'subtask-tok', + '/path/server.js', + ); + await coordinator.createTask({ name: 'test', prompt: 'do', coordinatorTaskId: 'coord-1' }); + + const userEntry = { command: 'user-replacement' }; + const changed = JSON.parse(currentConfig) as { + mcpServers: Record; + }; + changed.mcpServers['parallel-code'] = userEntry; + currentConfig = JSON.stringify(changed); + mockAtomicWriteFileSync.mockClear(); + + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3002', + 'new-coordinator-tok', + 'new-subtask-tok', + '/path/server.js', + ); + + expect(mockAtomicWriteFileSync.mock.calls.some(([path]) => path === configPath)).toBe(false); + expect(JSON.parse(currentConfig).mcpServers['parallel-code']).toEqual(userEntry); + expect(mockLogWarn).toHaveBeenCalledWith( + 'coordinator.kimi_mcp', + expect.stringContaining('refusing overwrite'), + expect.objectContaining({ taskId: 'task-1', configPath }), + ); + }); + + it('recreates a missing managed Kimi child MCP entry during refresh', async () => { + const configPath = '/tmp/test/.kimi-code/mcp.json'; + let currentConfig = JSON.stringify({ + mcpServers: { other: { command: 'other-server' } }, + }); + mockExistsSync.mockImplementation((path) => path === configPath); + mockReadFileSync.mockImplementation((path) => + path === configPath ? currentConfig : '# existing\n', + ); + mockAtomicWriteFileSync.mockImplementation((path, raw) => { + if (path === configPath) currentConfig = raw as string; + }); + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-tok', + 'subtask-tok', + '/path/server.js', + ); + await coordinator.createTask({ name: 'test', prompt: 'do', coordinatorTaskId: 'coord-1' }); + + const withoutManagedEntry = JSON.parse(currentConfig) as { + mcpServers: Record; + }; + delete withoutManagedEntry.mcpServers['parallel-code']; + currentConfig = JSON.stringify(withoutManagedEntry); + mockAtomicWriteFileSync.mockClear(); + mockLogWarn.mockClear(); + + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3002', + 'new-coordinator-tok', + 'new-subtask-tok', + '/path/server.js', + ); + + const refreshed = JSON.parse(currentConfig) as { + mcpServers: { + other: { command: string }; + 'parallel-code': { env: Record }; + }; + }; + expect(refreshed.mcpServers.other).toEqual({ command: 'other-server' }); + expect(refreshed.mcpServers['parallel-code'].env['PARALLEL_CODE_MCP_TOKEN']).toBe( + 'new-subtask-tok', + ); + expect(mockLogWarn).not.toHaveBeenCalledWith( + 'coordinator.kimi_mcp', + expect.stringContaining('refusing overwrite'), + expect.anything(), + ); + }); }); // ─── MCP config restart rewrite tests ──────────────────────────────────────── @@ -3616,6 +4654,78 @@ describe('Coordinator hydrateTask — restart hydration', () => { expect(task?.status).toBe('exited'); }); + it('hydrateTask keeps Kimi launch args empty when the existing task command is reused', async () => { + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-token', + 'subtask-token', + '/path/server.js', + ); + const task = await coordinator.createTask({ + name: 'kimi-task', + prompt: 'do', + coordinatorTaskId: 'coord-1', + }); + + const result = coordinator.hydrateTask({ + id: task.id, + name: task.name, + projectId: task.projectId, + projectRoot: task.projectRoot, + branchName: task.branchName, + worktreePath: task.worktreePath, + agentId: task.agentId, + coordinatorTaskId: task.coordinatorTaskId, + mcpConfigPath: task.mcpConfigPath, + }); + + expect(result.mcpLaunchArgs).toEqual([]); + }); + + it('hydrateTask preserves live Kimi MCP state when persisted state is invalid', async () => { + coordinator.setCoordinatorSpawnDefaults('coord-1', 'kimi', []); + coordinator.setMCPServerInfo( + 'coord-1', + 'http://localhost:3001', + 'coordinator-token', + 'subtask-token', + '/path/server.js', + ); + const task = await coordinator.createTask({ + name: 'kimi-task', + prompt: 'do', + coordinatorTaskId: 'coord-1', + }); + const liveState = task.autoDiscoveredMcpConfig; + expect(liveState).toBeDefined(); + + const result = coordinator.hydrateTask({ + id: task.id, + name: task.name, + projectId: task.projectId, + projectRoot: task.projectRoot, + branchName: task.branchName, + worktreePath: task.worktreePath, + agentId: task.agentId, + coordinatorTaskId: task.coordinatorTaskId, + mcpConfigPath: task.mcpConfigPath, + agentCommand: task.agentCommand, + autoDiscoveredMcpConfig: { + path: '/tmp/not-this-task/.kimi-code/mcp.json', + writtenParallelCodeFingerprint: 'a'.repeat(64), + }, + }); + + expect(result.autoDiscoveredMcpConfig?.path).toBe(liveState?.path); + expect(mockLogWarn).toHaveBeenCalledWith( + 'coordinator.kimi_mcp', + 'ignored invalid persisted Kimi MCP state; preserving live state', + { taskId: task.id }, + ); + }); + it('hydrateTask restores an undelivered initial prompt for backend delivery', () => { coordinator.hydrateTask({ id: 'hydrated-1', diff --git a/electron/mcp/coordinator.ts b/electron/mcp/coordinator.ts index eee97b71a..c64c81f28 100644 --- a/electron/mcp/coordinator.ts +++ b/electron/mcp/coordinator.ts @@ -2,10 +2,11 @@ // Manages task lifecycle independently of the SolidJS renderer, // using existing backend primitives (pty, git, tasks). -import { randomUUID, randomBytes } from 'crypto'; -import { execFile } from 'child_process'; +import { createHash, randomUUID, randomBytes } from 'crypto'; +import { execFile, spawnSync } from 'child_process'; +import { dirname, join } from 'path'; import { promisify } from 'util'; -import { unlinkSync, readFileSync, existsSync } from 'fs'; +import { mkdirSync, unlinkSync, readFileSync, existsSync } from 'fs'; import { unlink as fsUnlink } from 'fs/promises'; import { buildSubTaskMcpConfig, @@ -14,9 +15,15 @@ import { writeSubTaskMcpConfig, writeSubTaskMcpConfigSync, } from './config.js'; -import { buildMcpLaunchArgs, isCodexCommand, type ParallelCodeMcpConfig } from './agent-args.js'; +import { + buildMcpLaunchArgs, + isCodexCommand, + isKimiCommand, + type ParallelCodeMcpConfig, +} from './agent-args.js'; import { validateBranchName } from './validation.js'; import { atomicWriteFileSync } from './atomic.js'; +import { appendGitInfoExcludeBlocks } from '../ipc/git-exclude.js'; import { ReplayCache } from './replay-cache.js'; import { detectPreambleFiles, @@ -77,6 +84,7 @@ import type { ApiTaskDetail, ApiDiffResult, ApiLandSelfResult, + AutoDiscoveredMcpConfigState, LandSelfInput, LandingState, SubtaskVerification, @@ -135,6 +143,82 @@ const PREAMBLE_ARTIFACT_PATHS = new Set([ '.claude/settings.local.json', ]); const UNRESOLVED_LANDED_COMMIT = 'unresolved'; +const KIMI_AUTO_DISCOVERED_MCP_PATHS = ['.kimi-code/mcp.json', '.mcp.json'] as const; + +type McpJsonContent = Record & { + mcpServers?: Record; +}; + +type RestoreMcpConfigResult = { + status: 'none' | 'restored' | 'failed'; + managedEntry?: unknown; +}; + +function parseMcpJsonContent(configPath: string, raw: string): McpJsonContent { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + throw new Error(`${configPath} contains invalid JSON`); + } + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + throw new Error(`${configPath} must contain a JSON object`); + } + + const content = parsed as McpJsonContent; + const servers = content.mcpServers; + if ( + servers !== undefined && + (!servers || typeof servers !== 'object' || Array.isArray(servers)) + ) { + throw new Error(`${configPath} mcpServers must be a JSON object`); + } + return content; +} + +function readMcpJsonContent(configPath: string): McpJsonContent { + if (!existsSync(configPath)) return {}; + return parseMcpJsonContent(configPath, readFileSync(configPath, 'utf-8')); +} + +function mcpEntryFingerprint(value: unknown): string { + return createHash('sha256') + .update(JSON.stringify(value) ?? 'undefined') + .digest('hex'); +} + +function isTrackedGitPath(worktreePath: string, relativePath: string): boolean { + const result = spawnSync('git', ['ls-files', '--error-unmatch', '--', relativePath], { + cwd: worktreePath, + stdio: 'ignore', + timeout: 3000, + }); + if (result.error) { + throw new Error(`Unable to verify whether ${relativePath} is tracked: ${result.error.message}`); + } + if (result.status === 0) return true; + if (result.status === 1) return false; + throw new Error(`Unable to verify whether ${relativePath} is tracked`); +} + +function validateAutoDiscoveredMcpConfigState( + value: unknown, + worktreePath: string, +): AutoDiscoveredMcpConfigState | undefined { + if (!value || typeof value !== 'object' || Array.isArray(value)) return undefined; + const state = value as Record; + const allowedPaths = KIMI_AUTO_DISCOVERED_MCP_PATHS.map((path) => join(worktreePath, path)); + if (typeof state.path !== 'string' || !allowedPaths.includes(state.path)) return undefined; + if ( + typeof state.writtenParallelCodeFingerprint !== 'string' || + !/^[a-f0-9]{64}$/.test(state.writtenParallelCodeFingerprint) + ) + return undefined; + return { + path: state.path, + writtenParallelCodeFingerprint: state.writtenParallelCodeFingerprint, + }; +} function pasteDelayMs(text: string): number { const lines = text.split('\n').length; @@ -857,6 +941,14 @@ export class Coordinator { doneToken: task.doneToken, }); writeSubTaskMcpConfigSync(mcpConfigPath, mcpConfig); + try { + this.writeKimiAutoDiscoveredMcpConfig(task, mcpConfig); + } catch (err) { + logWarn('coordinator.kimi_mcp', 'failed to refresh Kimi child MCP config', { + taskId: task.id, + error: err instanceof Error ? err.message : String(err), + }); + } } } @@ -1368,6 +1460,7 @@ export class Coordinator { }; const agentCommand = opts.agentCommand ?? coordinatorState.spawnDefaults.command; + task.agentCommand = agentCommand; const dockerContainerName = this.coordinators.get(task.coordinatorTaskId)?.dockerContainerName ?? null; @@ -1408,6 +1501,7 @@ export class Coordinator { subTaskMcpConfigPath = configPath; task.mcpConfigPath = configPath; assertStillPresent(); + this.writeKimiAutoDiscoveredMcpConfig(task, mcpConfig); } const skipPermissions = opts.skipPermissions ?? coordinatorState.propagateSkipPermissions; @@ -1499,6 +1593,7 @@ export class Coordinator { signalDoneConsumed: task.signalDoneConsumed ?? false, baseBranch: task.baseBranch, mcpConfigPath: subTaskMcpConfigPath, + autoDiscoveredMcpConfig: task.autoDiscoveredMcpConfig, prompt: task.initialPrompt, preambleFileExistedBefore: task.preambleFileExistedBefore, agentCommand: agentCommand, @@ -2206,6 +2301,266 @@ export class Coordinator { this.clearAgentBuffers(task.agentId); } + private writeKimiAutoDiscoveredMcpConfig( + task: CoordinatedTask, + mcpConfig: ReturnType, + syncState = true, + ): void { + if (!task.agentCommand || !isKimiCommand(task.agentCommand)) return; + + const writtenParallelCode = mcpConfig.mcpServers['parallel-code']; + const priorState = task.autoDiscoveredMcpConfig; + type Candidate = { + relativePath: (typeof KIMI_AUTO_DISCOVERED_MCP_PATHS)[number]; + configPath: string; + tracked: boolean; + }; + const toCandidate = ( + relativePath: (typeof KIMI_AUTO_DISCOVERED_MCP_PATHS)[number], + ): Candidate => ({ + relativePath, + configPath: join(task.worktreePath, relativePath), + tracked: isTrackedGitPath(task.worktreePath, relativePath), + }); + let candidate: Candidate | undefined; + if (priorState) { + const relativePath = KIMI_AUTO_DISCOVERED_MCP_PATHS.find( + (path) => join(task.worktreePath, path) === priorState.path, + ); + if (relativePath) candidate = toCandidate(relativePath); + } else { + for (const relativePath of KIMI_AUTO_DISCOVERED_MCP_PATHS) { + const current = toCandidate(relativePath); + if (!current.tracked) { + candidate = current; + break; + } + } + } + if (!candidate) { + throw new Error( + 'Unable to create Kimi child MCP config: both .kimi-code/mcp.json and .mcp.json are tracked by Git.', + ); + } + if (candidate.tracked) { + throw new Error( + `Unable to create Kimi child MCP config: ${candidate.relativePath} is tracked by Git.`, + ); + } + + if (candidate.relativePath === '.mcp.json') { + // Kimi loads .kimi-code/mcp.json after .mcp.json. A server defined there + // would replace the task-scoped server written to the fallback path. + const preferredPath = join(task.worktreePath, KIMI_AUTO_DISCOVERED_MCP_PATHS[0]); + if (readMcpJsonContent(preferredPath).mcpServers?.['parallel-code'] !== undefined) { + throw new Error( + `Unable to create Kimi child MCP config: ${KIMI_AUTO_DISCOVERED_MCP_PATHS[0]} already defines mcpServers["parallel-code"] and would override .mcp.json.`, + ); + } + } + + const { configPath, relativePath } = candidate; + const content = readMcpJsonContent(configPath); + const existingParallelCode = content.mcpServers?.['parallel-code']; + const isManagedCandidate = priorState?.path === configPath; + if (existingParallelCode !== undefined && !isManagedCandidate) { + throw new Error( + `Unable to create Kimi child MCP config: ${relativePath} already defines mcpServers["parallel-code"].`, + ); + } + const servers = content.mcpServers ?? {}; + + if ( + priorState?.path === configPath && + existingParallelCode !== undefined && + mcpEntryFingerprint(servers['parallel-code']) !== priorState.writtenParallelCodeFingerprint + ) { + logWarn('coordinator.kimi_mcp', 'auto-discovered MCP config changed; refusing overwrite', { + taskId: task.id, + configPath, + }); + return; + } + + const relativeDir = relativePath.includes('/') + ? relativePath.slice(0, relativePath.lastIndexOf('/') + 1) + : ''; + // Leading slash anchors both patterns to the worktree root. Without it a + // slashless pattern such as `.mcp.json` matches at any depth and would hide + // a user's nested config from git status. + const configPattern = `/${relativePath}`; + const atomicTmpPattern = `/${relativeDir}.parallel-code-atomic-*.tmp`; + const excludePatterns = [ + { + marker: configPattern, + block: `# Parallel Code Kimi MCP config (contains ephemeral token)\n${configPattern}\n`, + }, + { + marker: atomicTmpPattern, + block: `${atomicTmpPattern}\n`, + }, + ]; + const result = appendGitInfoExcludeBlocks(task.worktreePath, excludePatterns, (err, markers) => + console.warn(`[MCP] Could not git-exclude child Kimi MCP paths ${markers.join(', ')}:`, err), + ); + if (result !== 'appended' && result !== 'present') { + throw new Error( + `Unable to git-exclude Kimi child MCP credential paths ${configPattern}, ${atomicTmpPattern}; refusing to write them.`, + ); + } + + content.mcpServers = { ...servers, 'parallel-code': writtenParallelCode }; + mkdirSync(dirname(configPath), { recursive: true }); + atomicWriteFileSync(configPath, JSON.stringify(content, null, 2), { mode: 0o600 }); + task.autoDiscoveredMcpConfig = { + path: configPath, + writtenParallelCodeFingerprint: mcpEntryFingerprint(writtenParallelCode), + }; + if (syncState) this.syncAutoDiscoveredMcpConfig(task); + } + + private syncAutoDiscoveredMcpConfig(task: CoordinatedTask): void { + this.notifyRenderer(IPC.MCP_TaskStateSync, { + taskId: task.id, + autoDiscoveredMcpConfig: task.autoDiscoveredMcpConfig ?? null, + }); + } + + private readManagedMcpEntryFromTaskConfig( + task: CoordinatedTask, + state: AutoDiscoveredMcpConfigState, + ): unknown { + if (!task.mcpConfigPath || !existsSync(task.mcpConfigPath)) return undefined; + try { + const entry = readMcpJsonContent(task.mcpConfigPath).mcpServers?.['parallel-code']; + return mcpEntryFingerprint(entry) === state.writtenParallelCodeFingerprint + ? entry + : undefined; + } catch (err) { + logWarn('coordinator.kimi_mcp', 'failed to read per-task MCP config for history check', { + taskId: task.id, + configPath: task.mcpConfigPath, + error: err instanceof Error ? err.message : String(err), + }); + return undefined; + } + } + + private restoreTaskAutoDiscoveredMcpConfig(task: CoordinatedTask): RestoreMcpConfigResult { + const state = task.autoDiscoveredMcpConfig; + if (!state) return { status: 'none' }; + + try { + const content = readMcpJsonContent(state.path); + const servers = content.mcpServers ?? {}; + const managedEntry = servers['parallel-code']; + if (managedEntry === undefined) { + const historicalManagedEntry = this.readManagedMcpEntryFromTaskConfig(task, state); + if (historicalManagedEntry === undefined) return { status: 'failed' }; + task.autoDiscoveredMcpConfig = undefined; + this.syncAutoDiscoveredMcpConfig(task); + return { status: 'restored', managedEntry: historicalManagedEntry }; + } + if (mcpEntryFingerprint(managedEntry) !== state.writtenParallelCodeFingerprint) + return { status: 'failed' }; + + delete servers['parallel-code']; + + const hasServers = Object.keys(servers).length > 0; + const hasOtherKeys = Object.keys(content).some((key) => key !== 'mcpServers'); + if (!hasServers && !hasOtherKeys) { + unlinkSync(state.path); + task.autoDiscoveredMcpConfig = undefined; + this.syncAutoDiscoveredMcpConfig(task); + return { status: 'restored', managedEntry }; + } + if (hasServers) content.mcpServers = servers; + else delete content.mcpServers; + atomicWriteFileSync(state.path, JSON.stringify(content, null, 2), { mode: 0o600 }); + task.autoDiscoveredMcpConfig = undefined; + this.syncAutoDiscoveredMcpConfig(task); + return { status: 'restored', managedEntry }; + } catch (err) { + logWarn('coordinator.kimi_mcp', 'failed to restore auto-discovered MCP config', { + taskId: task.id, + configPath: state.path, + error: err instanceof Error ? err.message : String(err), + }); + return { status: 'failed' }; + } + } + + private extractManagedMcpTokens(managedEntry: unknown): string[] { + if (!managedEntry || typeof managedEntry !== 'object' || Array.isArray(managedEntry)) return []; + const env = (managedEntry as { env?: unknown }).env; + if (!env || typeof env !== 'object' || Array.isArray(env)) return []; + return ['PARALLEL_CODE_MCP_TOKEN', 'PARALLEL_CODE_MCP_DONE_TOKEN'] + .map((key) => (env as Record)[key]) + .filter((value): value is string => typeof value === 'string' && value.length > 0); + } + + private async assertManagedMcpTokensAbsentFromGitHistory( + task: CoordinatedTask, + managedEntry: unknown, + ): Promise { + const tokens = this.extractManagedMcpTokens(managedEntry); + if (tokens.length === 0) { + throw new Error( + 'Unable to verify managed Kimi MCP tokens before landing or merge; refusing to continue.', + ); + } + const historyRange = task.baseBranch ? `${task.baseBranch}..HEAD` : 'HEAD'; + const result = await execAsync( + 'git', + [ + 'log', + historyRange, + '-m', + '--text', + '--no-textconv', + '-p', + '--format=', + '--', + '.mcp.json', + '.kimi-code/mcp.json', + ':(glob)**/.parallel-code-atomic-*.tmp', + ], + { cwd: task.worktreePath, maxBuffer: 8 * 1024 * 1024 }, + ); + const history = execStdout(result); + if (tokens.some((token) => history.includes(token))) { + throw new Error( + 'Managed Kimi MCP token was found in task Git history; refusing to land or merge until the token-bearing commit is removed.', + ); + } + } + + private kimiMcpRecoveryError(task: CoordinatedTask, operation: string): string { + const configPath = + task.autoDiscoveredMcpConfig?.path ?? join(task.worktreePath, '.kimi-code', 'mcp.json'); + return ( + `Unable to restore managed Kimi MCP config before ${operation}; refusing to validate, stage or merge a worktree that may contain ephemeral MCP tokens. ` + + `Inspect ${configPath}. If you edited mcpServers["parallel-code"], preserve any intentional changes securely outside the worktree, then remove only that entry and retry; keep other MCP servers intact. ` + + 'If the entry was not edited, check that the config is valid JSON and its directory is writable before retrying. Do not commit this config or its tokens.' + ); + } + + private refreshTaskMcpConfigAfterLandingFailure(task: CoordinatedTask): void { + try { + this.rewriteHydratedSubtaskMcpConfig( + task, + task.coordinatorTaskId, + task.mcpConfigPath, + task.agentCommand, + ); + } catch (err) { + logWarn('coordinator.kimi_mcp', 'failed to restore MCP config after landing failure', { + taskId: task.id, + error: err instanceof Error ? err.message : String(err), + }); + } + } + /** Best-effort removal of a task's per-sub-task MCP config file. */ private unlinkMcpConfigFile(path: string | undefined): void { if (!path) return; @@ -2217,6 +2572,7 @@ export class Coordinator { } private clearTaskMcpConfig(task: CoordinatedTask): void { + this.restoreTaskAutoDiscoveredMcpConfig(task); this.unlinkMcpConfigFile(task.mcpConfigPath); task.mcpConfigPath = undefined; } @@ -2332,16 +2688,46 @@ export class Coordinator { task.verification = input.verification; task.landingSummary = input.summary; + const restoreMcpConfig = this.restoreTaskAutoDiscoveredMcpConfig(task); + if (restoreMcpConfig.status === 'failed') { + // Re-arm before escalating. In the case that gets us here the managed + // entry is already gone from the worktree, so without this the child is + // left with no parallel-code server and cannot report the escalation + // back. The write refuses to touch an entry it does not own, so a + // fingerprint mismatch stays untouched; landing fails closed either way. + this.refreshTaskMcpConfigAfterLandingFailure(task); + const reason = this.kimiMcpRecoveryError(task, 'self-landing'); + this.escalateLanding(task, 'landing_escalated', reason); + throw new Error(reason); + } + if (restoreMcpConfig.managedEntry !== undefined) { + try { + await this.assertManagedMcpTokensAbsentFromGitHistory(task, restoreMcpConfig.managedEntry); + } catch (err) { + if (restoreMcpConfig.status === 'restored') + this.refreshTaskMcpConfigAfterLandingFailure(task); + const reason = err instanceof Error ? err.message : String(err); + this.escalateLanding(task, 'landing_escalated', reason); + throw err; + } + } + const shouldRefreshMcpConfig = restoreMcpConfig.status === 'restored'; try { await this.prepareCleanSelfLandingWorktree(task, () => this.assertOrchestrationEnabled(epoch), ); } catch (err) { + if (shouldRefreshMcpConfig) this.refreshTaskMcpConfigAfterLandingFailure(task); const reason = err instanceof Error ? err.message : String(err); this.escalateLanding(task, 'landing_escalated', reason); throw err; } - await this.verifyBeforeLanding(task); + try { + await this.verifyBeforeLanding(task); + } catch (err) { + if (shouldRefreshMcpConfig) this.refreshTaskMcpConfigAfterLandingFailure(task); + throw err; + } let mergeResult: { mainBranch: string; linesAdded: number; linesRemoved: number }; try { @@ -2349,6 +2735,7 @@ export class Coordinator { this.assertOrchestrationEnabled(epoch), ); } catch (err) { + if (shouldRefreshMcpConfig) this.refreshTaskMcpConfigAfterLandingFailure(task); const reason = err instanceof Error ? err.message : String(err); const state = reason.toLowerCase().includes('conflict') || reason.includes('Merge failed') @@ -2451,50 +2838,79 @@ export class Coordinator { if (!this.coordinators.has(task.coordinatorTaskId)) throw new Error('The parent is unavailable; integration is disabled.'); this.assertTaskCanBeMerged(task); - - // Strip injected preamble files before staging so they don't land in history, - // then auto-commit any uncommitted changes in the task worktree before merging. - if (task.worktreePath) { - await stripPreambleFromBranch(task); + const restoreMcpConfig = this.restoreTaskAutoDiscoveredMcpConfig(task); + if (restoreMcpConfig.status === 'failed') { + this.refreshTaskMcpConfigAfterLandingFailure(task); + throw new Error(this.kimiMcpRecoveryError(task, 'merge')); + } + if (restoreMcpConfig.managedEntry !== undefined) { try { this.assertOrchestrationEnabled(epoch); - await execAsync('git', ['add', '-A'], { cwd: task.worktreePath }); - this.assertOrchestrationEnabled(epoch); - await execAsync('git', ['commit', '-m', 'WIP: auto-commit before merge'], { - cwd: task.worktreePath, - }); - } catch { - // Commit failed — check if uncommitted changes still exist - const { stdout: statusOut } = await execAsync('git', ['status', '--porcelain'], { - cwd: task.worktreePath, - }); - if (statusOut.trim()) { - throw new Error( - `Auto-commit failed and the task worktree still has uncommitted changes. ` + - `Please commit or discard changes in ${task.worktreePath} before merging.`, - ); - } - // Nothing to commit — swallow silently + await this.assertManagedMcpTokensAbsentFromGitHistory(task, restoreMcpConfig.managedEntry); + } catch (err) { + if (restoreMcpConfig.status === 'restored') + this.refreshTaskMcpConfigAfterLandingFailure(task); + throw err; } } - // skipVerification is the coordinator's way past a failure the task cannot - // fix, such as a suite that is already red on the base branch. - if (!opts?.skipVerification) await this.verifyBeforeLanding(task); + const shouldRefreshMcpConfig = restoreMcpConfig.status === 'restored'; - const result = await this.runGitMerge(task, opts, () => this.assertOrchestrationEnabled(epoch)); + try { + // Strip injected preamble files before staging so they don't land in history, + // then auto-commit any uncommitted changes in the task worktree before merging. + if (task.worktreePath) { + await stripPreambleFromBranch(task); + try { + this.assertOrchestrationEnabled(epoch); + await execAsync('git', ['add', '-A'], { cwd: task.worktreePath }); + this.assertOrchestrationEnabled(epoch); + await execAsync('git', ['commit', '-m', 'WIP: auto-commit before merge'], { + cwd: task.worktreePath, + }); + } catch { + // Commit failed — check if uncommitted changes still exist + const { stdout: statusOut } = await execAsync('git', ['status', '--porcelain'], { + cwd: task.worktreePath, + }); + if (statusOut.trim()) { + throw new Error( + `Auto-commit failed and the task worktree still has uncommitted changes. ` + + `Please commit or discard changes in ${task.worktreePath} before merging.`, + ); + } + // Nothing to commit — swallow silently + } + } - if (opts?.cleanup) { - await this.cleanupTask(taskId, { + // Verify the cleaned, committed tree so the recorded HEAD/dirty state + // describes what will be merged, matching the upstream landing order. + // skipVerification remains the explicit escape hatch for base-branch failures. + this.assertOrchestrationEnabled(epoch); + if (!opts?.skipVerification) await this.verifyBeforeLanding(task); + + const result = await this.runGitMerge(task, opts, () => + this.assertOrchestrationEnabled(epoch), + ); + + if (opts?.cleanup) { + await this.cleanupTask(taskId, { + linesAdded: result.linesAdded, + linesRemoved: result.linesRemoved, + }); + } + if (this.tasks.has(taskId) && shouldRefreshMcpConfig) { + this.refreshTaskMcpConfigAfterLandingFailure(task); + } + + return { + mainBranch: result.mainBranch, linesAdded: result.linesAdded, linesRemoved: result.linesRemoved, - }); + }; + } catch (err) { + if (shouldRefreshMcpConfig) this.refreshTaskMcpConfigAfterLandingFailure(task); + throw err; } - - return { - mainBranch: result.mainBranch, - linesAdded: result.linesAdded, - linesRemoved: result.linesRemoved, - }; } async getReviewSnapshot(taskId: string): Promise<{ @@ -2545,34 +2961,52 @@ export class Coordinator { ) { throw new Error('The integration target changed or is unavailable. Review again.'); } - // Remove uncommitted runtime guidance only. Never stage or commit reviewed results. - await stripPreambleFromBranch(task); - if ((await this.statusPaths(task.worktreePath)).length) { - throw new Error( - 'The child has uncommitted changes. Commit the intended result and review again.', + const restoreMcpConfig = this.restoreTaskAutoDiscoveredMcpConfig(task); + if (restoreMcpConfig.status === 'failed') { + this.refreshTaskMcpConfigAfterLandingFailure(task); + throw new Error(this.kimiMcpRecoveryError(task, 'reviewed merge')); + } + try { + if (restoreMcpConfig.managedEntry !== undefined) { + await this.assertManagedMcpTokensAbsentFromGitHistory( + task, + restoreMcpConfig.managedEntry, + ); + } + // Remove uncommitted runtime guidance only. Never stage or commit reviewed results. + await stripPreambleFromBranch(task); + if ((await this.statusPaths(task.worktreePath)).length) { + throw new Error( + 'The child has uncommitted changes. Commit the intended result and review again.', + ); + } + await this.verifyBeforeLanding(task); + const result = await gitMergeTask( + task.projectRoot, + task.branchName, + false, + null, + false, + task.baseBranch, + task.worktreePath, + parent.worktreePath, + approval, ); + // Keep the integrated result visible; cleanup is a separate explicit action. + task.landingState = 'reviewed'; + this.syncLandingState(task); + this.suppressPendingNotificationForTask(task, true); + return { + mainBranch: result.main_branch, + linesAdded: result.lines_added, + linesRemoved: result.lines_removed, + }; + } catch (err) { + if (restoreMcpConfig.status === 'restored') { + this.refreshTaskMcpConfigAfterLandingFailure(task); + } + throw err; } - await this.verifyBeforeLanding(task); - const result = await gitMergeTask( - task.projectRoot, - task.branchName, - false, - null, - false, - task.baseBranch, - task.worktreePath, - parent.worktreePath, - approval, - ); - // Keep the integrated result visible; cleanup is a separate explicit action. - task.landingState = 'reviewed'; - this.syncLandingState(task); - this.suppressPendingNotificationForTask(task, true); - return { - mainBranch: result.main_branch, - linesAdded: result.lines_added, - linesRemoved: result.lines_removed, - }; }); } @@ -2759,12 +3193,16 @@ export class Coordinator { landingSummary?: string; landedMetadata?: CoordinatedTask['landedMetadata']; mcpConfigPath?: string; + autoDiscoveredMcpConfig?: AutoDiscoveredMcpConfigState; agentCommand?: string; preambleFileExistedBefore?: boolean; initialPrompt?: string; pendingPrompts?: string[]; assignedPromptDelivered?: boolean; - }): { mcpLaunchArgs?: string[] } { + }): { + mcpLaunchArgs?: string[]; + autoDiscoveredMcpConfig: AutoDiscoveredMcpConfigState | null; + } { const coordinatorState = this.coordinators.get(opts.coordinatorTaskId); if (!coordinatorState) { throw new Error(`coordinator ${opts.coordinatorTaskId} is not registered`); @@ -2781,14 +3219,38 @@ export class Coordinator { const existingTask = this.tasks.get(opts.id); if (existingTask) { - if (!this.orchestrationEnabled) this.discardAutomatedPrompts(existingTask); - if (safeMcpConfigPath) existingTask.mcpConfigPath = safeMcpConfigPath; + // Publish restored fields only after both credential files have been written. + const stagedTask = { ...existingTask }; + stagedTask.agentCommand = opts.agentCommand ?? stagedTask.agentCommand; + if (safeMcpConfigPath) stagedTask.mcpConfigPath = safeMcpConfigPath; + if (opts.autoDiscoveredMcpConfig !== undefined) { + const restoredState = validateAutoDiscoveredMcpConfigState( + opts.autoDiscoveredMcpConfig, + existingTask.worktreePath, + ); + if (restoredState) { + stagedTask.autoDiscoveredMcpConfig = restoredState; + } else if (existingTask.autoDiscoveredMcpConfig) { + logWarn( + 'coordinator.kimi_mcp', + 'ignored invalid persisted Kimi MCP state; preserving live state', + { taskId: existingTask.id }, + ); + } + } const mcpLaunchArgs = this.rewriteHydratedSubtaskMcpConfig( - existingTask, + stagedTask, opts.coordinatorTaskId, - safeMcpConfigPath ?? existingTask.mcpConfigPath, + stagedTask.mcpConfigPath, opts.agentCommand, + true, ); + const configStateChanged = + stagedTask.autoDiscoveredMcpConfig !== existingTask.autoDiscoveredMcpConfig; + Object.assign(existingTask, stagedTask); + if (!this.orchestrationEnabled) this.discardAutomatedPrompts(existingTask); + // Notification failures must not roll back just one of the committed files. + if (configStateChanged) this.syncAutoDiscoveredMcpConfig(existingTask); // A renderer reload may have missed the publication event. Send the live // record back, rather than letting an older saved report replace it. this.notifyRenderer(IPC.MCP_TaskStateSync, { @@ -2799,7 +3261,10 @@ export class Coordinator { signalDoneAt: existingTask.signalDoneAt?.toISOString() ?? null, signalDoneConsumed: existingTask.signalDoneConsumed ?? false, }); - return { mcpLaunchArgs }; + return { + mcpLaunchArgs, + autoDiscoveredMcpConfig: existingTask.autoDiscoveredMcpConfig ?? null, + }; } const completion = parseCompletionRecord(opts.completion); @@ -2834,6 +3299,11 @@ export class Coordinator { landingSummary: opts.landingSummary, landedMetadata: opts.landedMetadata, preambleFileExistedBefore: opts.preambleFileExistedBefore, + agentCommand: opts.agentCommand, + autoDiscoveredMcpConfig: validateAutoDiscoveredMcpConfigState( + opts.autoDiscoveredMcpConfig, + opts.worktreePath, + ), }; this.tasks.set(task.id, task); if (!this.orchestrationEnabled) this.discardAutomatedPrompts(task); @@ -2876,12 +3346,16 @@ export class Coordinator { } catch { /* agent not yet spawned — onPtyEvent('spawn') will subscribe when it starts */ } - return { mcpLaunchArgs }; + return { + mcpLaunchArgs, + autoDiscoveredMcpConfig: task.autoDiscoveredMcpConfig ?? null, + }; } catch (err) { // Clean up partial map entries so the agentId doesn't linger in state. this.clearAgentBuffers(agentId); this.subscribers.delete(agentId); this.clearPromptDeliveryState(task.id); + this.clearTaskMcpConfig(task); this.tasks.delete(task.id); throw err; } @@ -2931,6 +3405,7 @@ export class Coordinator { coordinatorTaskId: string, mcpConfigPath: string | undefined, agentCommand: string | undefined, + preserveExistingConfig = false, ): string[] | undefined { const serverInfo = this.coordinators.get(coordinatorTaskId)?.mcpServerInfo; if (!serverInfo) return undefined; @@ -2945,6 +3420,7 @@ export class Coordinator { taskId: task.id, doneToken: task.doneToken, }); + task.agentCommand = agentCommand ?? task.agentCommand ?? 'claude'; if (session && !mcpConfigPath) { mcpConfigPath = getSubTaskMcpConfigPath( this.coordinators.get(coordinatorTaskId)?.dockerContainerName, @@ -2953,10 +3429,39 @@ export class Coordinator { ); task.mcpConfigPath = mcpConfigPath; } - if (mcpConfigPath) { - writeSubTaskMcpConfigSync(mcpConfigPath, mcpConfig); + const launchArgs = this.buildTaskMcpLaunchArgs(task.agentCommand, mcpConfigPath, mcpConfig); + let previousConfig: string | undefined; + if (preserveExistingConfig && mcpConfigPath) { + try { + previousConfig = readFileSync(mcpConfigPath, 'utf8'); + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== 'ENOENT') throw err; + } + } + let configWritten = false; + try { + if (mcpConfigPath) { + writeSubTaskMcpConfigSync(mcpConfigPath, mcpConfig); + configWritten = true; + } + this.writeKimiAutoDiscoveredMcpConfig(task, mcpConfig, !preserveExistingConfig); + } catch (err) { + if (preserveExistingConfig && configWritten && mcpConfigPath) { + try { + if (previousConfig === undefined) unlinkSync(mcpConfigPath); + else atomicWriteFileSync(mcpConfigPath, previousConfig, { mode: 0o600 }); + } catch (restoreError) { + throw Object.assign( + new Error( + 'Task hydration failed and the previous per-task MCP config could not be restored.', + ), + { cause: err, restoreError }, + ); + } + } + throw err; } - return this.buildTaskMcpLaunchArgs(agentCommand ?? 'claude', mcpConfigPath, mcpConfig); + return launchArgs; } isRegisteredCoordinator(coordinatorTaskId: string): boolean { diff --git a/electron/mcp/dockerfile.test.ts b/electron/mcp/dockerfile.test.ts new file mode 100644 index 000000000..2f20a11e8 --- /dev/null +++ b/electron/mcp/dockerfile.test.ts @@ -0,0 +1,12 @@ +import { readFileSync } from 'fs'; +import { resolve } from 'path'; +import { describe, expect, it } from 'vitest'; + +describe('agent Dockerfile', () => { + it('pins Kimi Code below the workspace-trust-gated 0.33 line', () => { + const dockerfile = readFileSync(resolve(__dirname, '../../docker/Dockerfile'), 'utf8'); + + expect(dockerfile).toContain('# Keep Kimi below 0.33'); + expect(dockerfile).toContain('@moonshot-ai/kimi-code@0.32.0'); + }); +}); diff --git a/electron/mcp/preamble.test.ts b/electron/mcp/preamble.test.ts index 4539968f0..8c4f1122a 100644 --- a/electron/mcp/preamble.test.ts +++ b/electron/mcp/preamble.test.ts @@ -49,6 +49,55 @@ describe('sub-task preamble injection', () => { } }); + it.each(['kimi', '/usr/local/bin/kimi'])( + 'writes %s child preambles to AGENTS.md instead of Claude settings', + async (agentCommand) => { + const dir = mkdtempSync(join(tmpdir(), 'parallel-code-preamble-test-')); + const agentsPath = join(dir, 'AGENTS.md'); + const settingsPath = join(dir, '.claude', 'settings.local.json'); + const queue = new Map>(); + + try { + const injected = await injectSubTaskPreamble({ + worktreePath: dir, + agentCommand, + queue, + }); + + expect(injected).toMatchObject({ + filePath: agentsPath, + existedBefore: false, + restoreOnFailure: true, + }); + expect(readFileSync(agentsPath, 'utf8')).toContain(''); + expect(existsSync(settingsPath)).toBe(false); + await restoreSubTaskPreambleInjection(injected); + expect(existsSync(agentsPath)).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + ); + + it.each(['kimi-helper', '/opt/kimi/bin/claude', 'KIMI'])( + 'does not classify %s as the Kimi executable', + async (agentCommand) => { + const dir = mkdtempSync(join(tmpdir(), 'parallel-code-preamble-test-')); + try { + const injected = await injectSubTaskPreamble({ + worktreePath: dir, + agentCommand, + queue: new Map(), + }); + expect(injected.filePath).toBeUndefined(); + expect(existsSync(join(dir, '.claude', 'settings.local.json'))).toBe(false); + expect(existsSync(join(dir, 'AGENTS.md'))).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + ); + it('writes nothing for Claude, leaving existing settings untouched', async () => { const dir = mkdtempSync(join(tmpdir(), 'parallel-code-preamble-test-')); const settingsPath = join(dir, '.claude', 'settings.local.json'); @@ -84,43 +133,52 @@ describe('sub-task preamble injection', () => { } }); - it('does not follow or replace a symlinked instruction file', async () => { - const dir = mkdtempSync(join(tmpdir(), 'parallel-code-preamble-test-')); - try { - writeFileSync(join(dir, 'CLAUDE.md'), 'shared rules'); - symlinkSync('CLAUDE.md', join(dir, 'AGENTS.md')); - const injected = await injectSubTaskPreamble({ - worktreePath: dir, - agentCommand: 'codex', - queue: new Map(), - }); - expect(injected.filePath).toBeUndefined(); - expect(lstatSync(join(dir, 'AGENTS.md')).isSymbolicLink()).toBe(true); - expect(readFileSync(join(dir, 'CLAUDE.md'), 'utf8')).toBe('shared rules'); - await stripPreambleFromBranch({ worktreePath: dir, preambleFileExistedBefore: true }); - expect(lstatSync(join(dir, 'AGENTS.md')).isSymbolicLink()).toBe(true); - } finally { - rmSync(dir, { recursive: true, force: true }); - } - }); - - it('does not append a second block and keeps CRLF endings', async () => { - const dir = mkdtempSync(join(tmpdir(), 'parallel-code-preamble-test-')); - const agentsPath = join(dir, 'AGENTS.md'); - try { - writeFileSync(agentsPath, 'one\r\ntwo\r\n'); - const opts = { worktreePath: dir, agentCommand: 'codex', queue: new Map() }; - await injectSubTaskPreamble(opts); - const once = readFileSync(agentsPath, 'utf8'); - expect(once.replace(/\r\n/g, '')).not.toContain('\n'); - await injectSubTaskPreamble(opts); - expect(readFileSync(agentsPath, 'utf8')).toBe(once); - await stripPreambleFromBranch({ worktreePath: dir, preambleFileExistedBefore: true }); - expect(readFileSync(agentsPath, 'utf8')).toBe('one\r\ntwo\r\n'); - } finally { - rmSync(dir, { recursive: true, force: true }); - } - }); + it.each(['codex', 'kimi'])( + 'does not follow or replace a symlinked %s instruction file', + async (agentCommand) => { + const dir = mkdtempSync(join(tmpdir(), 'parallel-code-preamble-test-')); + try { + writeFileSync(join(dir, 'CLAUDE.md'), 'shared rules'); + symlinkSync('CLAUDE.md', join(dir, 'AGENTS.md')); + const injected = await injectSubTaskPreamble({ + worktreePath: dir, + agentCommand, + queue: new Map(), + }); + expect(injected.filePath).toBeUndefined(); + expect(injected.existedBefore).toBe(true); + expect(injected.restoreOnFailure).toBe(false); + expect(lstatSync(join(dir, 'AGENTS.md')).isSymbolicLink()).toBe(true); + expect(readFileSync(join(dir, 'CLAUDE.md'), 'utf8')).toBe('shared rules'); + await stripPreambleFromBranch({ worktreePath: dir, preambleFileExistedBefore: true }); + expect(lstatSync(join(dir, 'AGENTS.md')).isSymbolicLink()).toBe(true); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + ); + + it.each(['codex', 'kimi'])( + 'does not append a second %s block and keeps CRLF endings', + async (agentCommand) => { + const dir = mkdtempSync(join(tmpdir(), 'parallel-code-preamble-test-')); + const agentsPath = join(dir, 'AGENTS.md'); + try { + writeFileSync(agentsPath, 'one\r\ntwo\r\n'); + const opts = { worktreePath: dir, agentCommand, queue: new Map() }; + await injectSubTaskPreamble(opts); + const once = readFileSync(agentsPath, 'utf8'); + expect(once).toContain(SUB_TASK_MODE_PREAMBLE.replace(/\n/g, '\r\n')); + expect(once.replace(/\r\n/g, '')).not.toContain('\n'); + await injectSubTaskPreamble(opts); + expect(readFileSync(agentsPath, 'utf8')).toBe(once); + await stripPreambleFromBranch({ worktreePath: dir, preambleFileExistedBefore: true }); + expect(readFileSync(agentsPath, 'utf8')).toBe('one\r\ntwo\r\n'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + ); it('restore removes a file it created and tolerates it being gone', async () => { const dir = mkdtempSync(join(tmpdir(), 'parallel-code-preamble-test-')); diff --git a/electron/mcp/preamble.ts b/electron/mcp/preamble.ts index f180edd48..0e13e54d1 100644 --- a/electron/mcp/preamble.ts +++ b/electron/mcp/preamble.ts @@ -4,6 +4,7 @@ import { promisify } from 'util'; import { writeFileSync, readFileSync, existsSync, unlinkSync } from 'fs'; import { readFile as fsReadFile, unlink as fsUnlink, lstat as fsLstat } from 'fs/promises'; import { atomicWriteFile } from './atomic.js'; +import { isKimiCommand } from './agent-args.js'; import { info as logInfo, warn as logWarn } from '../log.js'; import { join } from 'path'; import os from 'os'; @@ -151,7 +152,11 @@ export async function injectSubTaskPreamble(args: { const preamble = args.integrationPolicy === 'review' ? REVIEW_SUB_TASK_MODE_PREAMBLE : SUB_TASK_MODE_PREAMBLE; const agentCmd = basename(args.agentCommand); - if (agentCmd.includes('codex') || agentCmd.includes('opencode')) { + if ( + agentCmd.includes('codex') || + agentCmd.includes('opencode') || + isKimiCommand(args.agentCommand) + ) { return injectMarkdownPreamble(args.queue, join(args.worktreePath, 'AGENTS.md'), preamble); } if (agentCmd.includes('gemini')) { diff --git a/electron/mcp/types.ts b/electron/mcp/types.ts index 9399b1875..71d1ae24f 100644 --- a/electron/mcp/types.ts +++ b/electron/mcp/types.ts @@ -1,4 +1,5 @@ -import type { VerificationRun } from '../ipc/shared-types.js'; +import type { AutoDiscoveredMcpConfigState, VerificationRun } from '../ipc/shared-types.js'; +export type { AutoDiscoveredMcpConfigState } from '../ipc/shared-types.js'; import type { ActivityEvidence } from '../agent-hooks/status.js'; import type { CompletionRecord, SubtaskVerification } from '../shared/completion-report.js'; export type { SubtaskVerification, SubtaskVerificationCheck } from '../shared/completion-report.js'; @@ -25,6 +26,8 @@ export interface CoordinatedTask { initialPrompt?: string; automationWriteInFlight?: boolean; mcpConfigPath?: string; // path to per-task tmp config, deleted on cleanup + autoDiscoveredMcpConfig?: AutoDiscoveredMcpConfigState; + agentCommand?: string; doneToken?: string; // per-task token; only the owning sub-task may call /done preambleFileExistedBefore?: boolean; // true if the preamble file existed before injection (even if empty) signalDoneAt?: Date; // set when sub-task explicitly calls signal_done diff --git a/electron/shared/agent-support.test.ts b/electron/shared/agent-support.test.ts new file mode 100644 index 000000000..164f5af88 --- /dev/null +++ b/electron/shared/agent-support.test.ts @@ -0,0 +1,15 @@ +import { describe, expect, it } from 'vitest'; +import { isAgentSupportedInMode } from './agent-support.js'; + +describe('isAgentSupportedInMode', () => { + it.each(['kimi', '/opt/bin/kimi'])('requires Docker for %s', (command) => { + expect(isAgentSupportedInMode(command)).toBe(false); + expect(isAgentSupportedInMode(command, false)).toBe(false); + expect(isAgentSupportedInMode(command, true)).toBe(true); + }); + + it.each(['claude', 'codex', 'gemini', '/bin/zsh', 'kimi-wrapper'])( + 'preserves native support for %s', + (command) => expect(isAgentSupportedInMode(command)).toBe(true), + ); +}); diff --git a/electron/shared/agent-support.ts b/electron/shared/agent-support.ts new file mode 100644 index 000000000..a794e67a1 --- /dev/null +++ b/electron/shared/agent-support.ts @@ -0,0 +1,4 @@ +/** Kimi's project MCP/trust flow is only supported by the pinned Docker image. */ +export function isAgentSupportedInMode(command: string, dockerMode = false): boolean { + return dockerMode || command.split('/').pop() !== 'kimi'; +} diff --git a/electron/shared/skip-permissions.test.ts b/electron/shared/skip-permissions.test.ts index 54969a247..1c8aa38af 100644 --- a/electron/shared/skip-permissions.test.ts +++ b/electron/shared/skip-permissions.test.ts @@ -6,6 +6,8 @@ describe('getSkipPermissionsArgs', () => { expect(getSkipPermissionsArgs('claude')).toEqual(['--dangerously-skip-permissions']); expect(getSkipPermissionsArgs('codex')).toEqual(['--dangerously-bypass-approvals-and-sandbox']); expect(getSkipPermissionsArgs('gemini')).toEqual(['--yolo']); + expect(getSkipPermissionsArgs('kimi')).toEqual(['--yolo']); + expect(getSkipPermissionsArgs('/usr/local/bin/kimi')).toEqual(['--yolo']); expect(getSkipPermissionsArgs('copilot')).toEqual(['--yolo']); expect(getSkipPermissionsArgs('agy')).toEqual(['--dangerously-skip-permissions']); }); diff --git a/electron/shared/skip-permissions.ts b/electron/shared/skip-permissions.ts index 0f10d3428..8a9c049ca 100644 --- a/electron/shared/skip-permissions.ts +++ b/electron/shared/skip-permissions.ts @@ -19,6 +19,7 @@ const SKIP_PERMISSIONS_ARGS = new Map([ ['claude', ['--dangerously-skip-permissions']], ['codex', ['--dangerously-bypass-approvals-and-sandbox']], ['gemini', ['--yolo']], + ['kimi', ['--yolo']], ['copilot', ['--yolo']], ['agy', ['--dangerously-skip-permissions']], ]); diff --git a/scripts/pre-commit.test.mjs b/scripts/pre-commit.test.mjs new file mode 100644 index 000000000..1aad61ad7 --- /dev/null +++ b/scripts/pre-commit.test.mjs @@ -0,0 +1,205 @@ +import { execFileSync, spawnSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import process from 'node:process'; +import { URL } from 'node:url'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +const hookSource = readFileSync(new URL('../.husky/pre-commit', import.meta.url), 'utf8'); +let fixture = ''; +let manifest; + +function git(args) { + return execFileSync('git', args, { cwd: fixture, encoding: 'utf8', stdio: 'pipe' }); +} + +function stageManifest() { + writeFileSync(join(fixture, 'package.json'), JSON.stringify(manifest)); + git(['add', 'package.json']); +} + +function runHook(env = {}) { + const result = spawnSync('sh', ['.husky/pre-commit'], { + cwd: fixture, + encoding: 'utf8', + env: { + ...process.env, + PATH: join(fixture, 'bin') + ':' + process.env.PATH, + HOOK_COMMAND_LOG: join(fixture, 'commands.log'), + NPX_EXIT_CODE: '0', + NPM_EXIT_CODE: '0', + ...env, + }, + }); + expect(result.error).toBeUndefined(); + return result; +} + +beforeEach(() => { + fixture = mkdtempSync(join(tmpdir(), 'parallel-code-pre-commit-')); + manifest = { + name: 'hook-fixture', + version: '1.0.0', + scripts: { check: 'old-check', postinstall: 'old-install' }, + dependencies: { example: '1.0.0' }, + }; + git(['init', '--quiet']); + // Fixture commits use their own ordinary, empty hook directory, never the real worktree hooks. + git(['config', 'core.hooksPath', '.git/hooks']); + writeFileSync(join(fixture, 'package.json'), JSON.stringify(manifest)); + writeFileSync(join(fixture, 'package-lock.json'), JSON.stringify({ lockfileVersion: 3 })); + git(['add', 'package.json', 'package-lock.json']); + git([ + '-c', + 'user.name=Hook Fixture', + '-c', + 'user.email=fixture@example.invalid', + '-c', + 'commit.gpgsign=false', + 'commit', + '--quiet', + '-m', + 'chore: seed hook fixture', + ]); + mkdirSync(join(fixture, '.husky')); + writeFileSync(join(fixture, '.husky', 'pre-commit'), hookSource); + mkdirSync(join(fixture, 'bin')); + for (const command of ['npx', 'npm']) { + const variable = command === 'npx' ? 'NPX_EXIT_CODE' : 'NPM_EXIT_CODE'; + writeFileSync( + join(fixture, 'bin', command), + '#!/bin/sh\nprintf "%s\\n" ' + + command + + ' >> "$HOOK_COMMAND_LOG"\nexit "$' + + variable + + '"\n', + { mode: 0o755 }, + ); + } +}); + +afterEach(() => { + if (fixture) rmSync(fixture, { recursive: true, force: true }); + fixture = ''; +}); + +describe('pre-commit lockfile protection', () => { + it('allows unchanged manifests and still runs both required checks', () => { + expect(runHook().status).toBe(0); + expect(readFileSync(join(fixture, 'commands.log'), 'utf8')).toBe('npx\nnpm\n'); + }); + + it('allows development-script-only changes without lockfile churn', () => { + manifest.scripts.check = 'new-check'; + manifest.scripts['test:changed'] = 'new-tests'; + stageManifest(); + expect(runHook().status).toBe(0); + }); + + it('compares JSON values rather than object key order', () => { + manifest = { + dependencies: manifest.dependencies, + scripts: { ...manifest.scripts, check: 'new' }, + version: manifest.version, + name: manifest.name, + }; + stageManifest(); + expect(runHook().status).toBe(0); + }); + + it.each([ + 'dependencies', + 'devDependencies', + 'optionalDependencies', + 'peerDependencies', + 'overrides', + ])('rejects changes to %s without a staged lockfile update', (field) => { + manifest[field] = { example: '2.0.0' }; + stageManifest(); + expect(runHook().status).not.toBe(0); + }); + + it('rejects package-version changes without a staged lockfile update', () => { + manifest.version = '2.0.0'; + stageManifest(); + expect(runHook().status).not.toBe(0); + }); + + it.each([ + 'preinstall', + 'install', + 'postinstall', + 'preprepare', + 'prepare', + 'postprepare', + 'prepublish', + ])('does not exempt the installation lifecycle script %s', (script) => { + manifest.scripts[script] = 'new-install'; + stageManifest(); + expect(runHook().status).not.toBe(0); + }); + + it('retains the existing paired manifest-and-lockfile update path', () => { + manifest.dependencies.example = '2.0.0'; + stageManifest(); + writeFileSync( + join(fixture, 'package-lock.json'), + JSON.stringify({ lockfileVersion: 3, version: '2' }), + ); + git(['add', 'package-lock.json']); + expect(runHook().status).toBe(0); + }); + + it('uses the staged manifest, not an unstaged working-tree repair', () => { + manifest.dependencies.example = '2.0.0'; + stageManifest(); + manifest.dependencies.example = '1.0.0'; + writeFileSync(join(fixture, 'package.json'), JSON.stringify(manifest)); + expect(runHook().status).not.toBe(0); + }); + + it('does not require a lock update for an unrelated unstaged dependency edit', () => { + manifest.scripts.check = 'new-check'; + stageManifest(); + manifest.dependencies.example = '2.0.0'; + writeFileSync(join(fixture, 'package.json'), JSON.stringify(manifest)); + expect(runHook().status).toBe(0); + }); + + it('rejects malformed staged manifests rather than treating them as script-only', () => { + writeFileSync(join(fixture, 'package.json'), '{ broken'); + git(['add', 'package.json']); + expect(runHook().status).not.toBe(0); + }); + + it('retains the lockfile gate for nested package manifests', () => { + mkdirSync(join(fixture, 'nested')); + writeFileSync( + join(fixture, 'nested', 'package.json'), + JSON.stringify({ dependencies: { example: '2.0.0' } }), + ); + git(['add', 'nested/package.json']); + expect(runHook().status).not.toBe(0); + }); + + it('rejects removal of the tracked lockfile even when it remains on disk', () => { + git(['rm', '--cached', '--quiet', 'package-lock.json']); + expect(runHook().status).not.toBe(0); + }); + + it('rejects an ignore rule hiding the lockfile, including an already tracked lockfile', () => { + writeFileSync(join(fixture, '.gitignore'), 'package-lock.json\n'); + git(['add', '.gitignore']); + expect(runHook().status).not.toBe(0); + }); + + it('stops when lint-staged fails', () => { + expect(runHook({ NPX_EXIT_CODE: '1' }).status).not.toBe(0); + expect(readFileSync(join(fixture, 'commands.log'), 'utf8')).toBe('npx\n'); + }); + + it('stops when npm run check fails', () => { + expect(runHook({ NPM_EXIT_CODE: '1' }).status).not.toBe(0); + }); +}); diff --git a/src/App.tsx b/src/App.tsx index 2a2ec09bd..a35a143fd 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -74,7 +74,7 @@ import { import { isGitHubUrl } from './lib/github-url'; import { getDeepActiveElement } from './lib/dom-focus'; import { HoldToQuit } from './components/HoldToQuit'; -import type { PersistedWindowState } from './store/types'; +import type { PersistedWindowState, Task } from './store/types'; import { initShortcuts, registerFromRegistry, @@ -494,7 +494,10 @@ function App() { if (!projectRoot) continue; markTaskMcpPending(task.id); hydratePromises.push( - invoke<{ mcpLaunchArgs?: string[] }>(IPC.MCP_HydrateCoordinatedTask, { + invoke<{ + mcpLaunchArgs?: string[]; + autoDiscoveredMcpConfig?: Task['autoDiscoveredMcpConfig'] | null; + }>(IPC.MCP_HydrateCoordinatedTask, { id: task.id, name: task.name, projectId: task.projectId, @@ -516,6 +519,7 @@ function App() { landingSummary: task.landingSummary, landedMetadata: task.landedMetadata, mcpConfigPath: task.mcpConfigPath, + autoDiscoveredMcpConfig: task.autoDiscoveredMcpConfig, agentCommand: store.agents[task.agentIds[0]]?.def.command ?? 'claude', preambleFileExistedBefore: task.preambleFileExistedBefore, initialPrompt: task.initialPrompt, diff --git a/src/components/AgentSelector.client.test.tsx b/src/components/AgentSelector.client.test.tsx new file mode 100644 index 000000000..5f20b107c --- /dev/null +++ b/src/components/AgentSelector.client.test.tsx @@ -0,0 +1,86 @@ +import { createSignal } from 'solid-js'; +import { render } from 'solid-js/web'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { AgentSelector } from './AgentSelector'; +import type { AgentDef } from '../ipc/types'; + +vi.mock('../store/store', () => ({ store: { themePreset: 'dark' } })); +vi.mock('../lib/theme', () => ({ theme: {} })); + +const agents: AgentDef[] = ['claude', 'kimi', 'codex'].map((command) => ({ + id: command, + command, + name: command, + args: [], + resume_args: [], + skip_permissions_args: [], + description: command, +})); +const disposers: Array<() => void> = []; +afterEach(() => { + disposers.splice(0).forEach((dispose) => dispose()); + document.body.replaceChildren(); +}); + +function mount(docker = false, selected = agents[0]) { + const [dockerMode, setDockerMode] = createSignal(docker); + const [selectedAgent, setSelectedAgent] = createSignal(selected); + const container = document.createElement('div'); + document.body.append(container); + disposers.push( + render( + () => ( + + ), + container, + ), + ); + return { container, selectedAgent, setDockerMode }; +} + +describe('AgentSelector Docker-only eligibility', () => { + it('hides Kimi from native selection and explains Docker support', () => { + const { container } = mount(); + expect( + Array.from(container.querySelectorAll('[role=radio]')).map((b) => b.textContent), + ).toEqual(['claude', 'codex']); + expect(container.textContent).toContain('Kimi Code is available in Docker mode only.'); + }); + + it('retains Kimi in Docker mode', () => { + const { container } = mount(true); + expect(container.querySelectorAll('[role=radio]')).toHaveLength(3); + expect(container.textContent).not.toContain('Docker mode only'); + }); + + it('switches away from Kimi when Docker is disabled', () => { + const { selectedAgent, setDockerMode, container } = mount(true, agents[1]); + setDockerMode(false); + expect(selectedAgent().command).toBe('claude'); + expect(container.querySelector('[aria-checked=true]')?.textContent).toBe('claude'); + }); + + it('skips hidden agents during keyboard navigation', () => { + const { selectedAgent, container } = mount(); + container + .querySelector('button') + ?.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true })); + expect(selectedAgent().command).toBe('codex'); + expect(document.activeElement?.textContent).toBe('codex'); + }); + + it('keeps keyboard focus correct after the Docker-only button is removed', () => { + const { selectedAgent, container, setDockerMode } = mount(true); + setDockerMode(false); + container + .querySelector('button') + ?.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true })); + expect(selectedAgent().command).toBe('codex'); + expect(document.activeElement?.textContent).toBe('codex'); + }); +}); diff --git a/src/components/AgentSelector.tsx b/src/components/AgentSelector.tsx index ebcd2b618..5ed16ef88 100644 --- a/src/components/AgentSelector.tsx +++ b/src/components/AgentSelector.tsx @@ -1,4 +1,5 @@ -import { For, Show } from 'solid-js'; +import { createEffect, For, Show } from 'solid-js'; +import { isAgentSupportedInMode } from '../../electron/shared/agent-support'; import { theme } from '../lib/theme'; import type { AgentDef } from '../ipc/types'; @@ -7,6 +8,7 @@ interface AgentSelectorProps { selectedAgent: AgentDef | null; onSelect: (agent: AgentDef) => void; wrap?: boolean; + dockerMode?: boolean; } /** @@ -14,11 +16,22 @@ interface AgentSelectorProps { * Only the selected agent is in the Tab order; Arrow keys move between agents. */ export function AgentSelector(props: AgentSelectorProps) { - const btnRefs: HTMLButtonElement[] = []; + const btnRefs = new Map(); const allowWrap = () => props.wrap ?? true; + const supportedAgents = () => + props.agents.filter((agent) => isAgentSupportedInMode(agent.command, props.dockerMode)); + + // Switching Docker off must not leave a hidden, unsupported agent selected. + createEffect(() => { + const selected = props.selectedAgent; + if (selected && !isAgentSupportedInMode(selected.command, props.dockerMode)) { + const fallback = supportedAgents()[0]; + if (fallback) props.onSelect(fallback); + } + }); function handleKeyDown(e: KeyboardEvent, idx: number) { - const agents = props.agents; + const agents = supportedAgents(); let nextIdx: number | null = null; if (e.key === 'ArrowRight' || e.key === 'ArrowDown') { @@ -31,7 +44,7 @@ export function AgentSelector(props: AgentSelectorProps) { if (nextIdx !== null) { props.onSelect(agents[nextIdx]); - btnRefs[nextIdx]?.focus(); + btnRefs.get(agents[nextIdx].id)?.focus(); } } @@ -58,12 +71,12 @@ export function AgentSelector(props: AgentSelectorProps) { 'padding-bottom': allowWrap() ? undefined : '2px', }} > - + {(agent, i) => { const isSelected = () => props.selectedAgent?.id === agent.id; return (