diff --git a/README.md b/README.md index af4e9bf..a75d602 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,7 @@ The only prerequisite is the underlying coding agent you want to use: - `acpx claude` -> Claude Code: https://claude.ai/code - `acpx gemini` -> Gemini CLI: https://github.com/google/gemini-cli - `acpx opencode` -> OpenCode: https://opencode.ai +- `acpx kiro` -> Kiro: https://kiro.dev - `acpx pi` -> Pi Coding Agent: https://github.com/mariozechner/pi ## Usage examples @@ -275,6 +276,7 @@ Built-ins: | `gemini` | native | [Gemini CLI](https://github.com/google/gemini-cli) | | `opencode` | native | [OpenCode](https://opencode.ai) | | `pi` | [pi-acp](https://github.com/svkozak/pi-acp) | [Pi Coding Agent](https://github.com/mariozechner/pi) | +| `kiro` | native | [Kiro](https://kiro.dev) | Use `--agent` as an escape hatch for custom ACP servers: diff --git a/docs/CLI.md b/docs/CLI.md index d94c382..fe7e598 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -42,7 +42,7 @@ acpx [global_options] sessions [list | new [--name ] | ensure [--n `` can be: -- built-in friendly name: `codex`, `claude`, `gemini`, `opencode`, `pi` +- built-in friendly name: `codex`, `claude`, `gemini`, `opencode`, `pi`, `kiro` - unknown token (treated as raw command) - overridden by `--agent ` escape hatch diff --git a/src/agent-registry.ts b/src/agent-registry.ts index c022c36..749993c 100644 --- a/src/agent-registry.ts +++ b/src/agent-registry.ts @@ -4,6 +4,7 @@ export const AGENT_REGISTRY: Record = { gemini: "gemini", opencode: "npx -y opencode-ai acp", pi: "npx pi-acp", + kiro: "kiro-cli acp", }; export const DEFAULT_AGENT_NAME = "codex"; diff --git a/src/client.ts b/src/client.ts index 9aaabf8..8d9b7cb 100644 --- a/src/client.ts +++ b/src/client.ts @@ -418,6 +418,7 @@ export class AcpClient { cwd: this.options.cwd, env: buildAgentEnvironment(this.options.authCredentials), stdio: ["pipe", "pipe", "pipe"], + detached: true, }); try { @@ -809,9 +810,20 @@ export class AcpClient { let exited = await waitForChildExit(child, AGENT_CLOSE_AFTER_STDIN_END_MS); if (!exited && isChildProcessRunning(child)) { try { - child.kill("SIGTERM"); + // Kill the entire process group (negative pgid) to ensure child processes + // spawned by wrapper agents (e.g. kiro-cli → kiro-cli-chat) are also terminated. + // Falls back to child.kill() if process group kill is not supported. + if (child.pid != null) { + process.kill(-child.pid, "SIGTERM"); + } else { + child.kill("SIGTERM"); + } } catch { - // best effort + try { + child.kill("SIGTERM"); + } catch { + // best effort + } } exited = await waitForChildExit(child, AGENT_CLOSE_TERM_GRACE_MS); } @@ -821,9 +833,17 @@ export class AcpClient { `agent did not exit after ${AGENT_CLOSE_TERM_GRACE_MS}ms; forcing SIGKILL`, ); try { - child.kill("SIGKILL"); + if (child.pid != null) { + process.kill(-child.pid, "SIGKILL"); + } else { + child.kill("SIGKILL"); + } } catch { - // best effort + try { + child.kill("SIGKILL"); + } catch { + // best effort + } } exited = await waitForChildExit(child, AGENT_CLOSE_KILL_GRACE_MS); } diff --git a/src/session-runtime.ts b/src/session-runtime.ts index 32eae5a..3d88a3e 100644 --- a/src/session-runtime.ts +++ b/src/session-runtime.ts @@ -201,6 +201,7 @@ type RunSessionPromptOptions = { onClientAvailable?: (controller: ActiveSessionController) => void; onClientClosed?: () => void; onPromptActive?: () => Promise | void; + onAgentPid?: (pid: number) => void; }; type ActiveSessionController = QueueOwnerActiveSessionController; @@ -291,6 +292,7 @@ async function runQueuedTask( onClientAvailable?: (controller: ActiveSessionController) => void; onClientClosed?: () => void; onPromptActive?: () => Promise | void; + onAgentPid?: (pid: number) => void; }, ): Promise { const outputFormatter = task.waitForCompletion @@ -314,6 +316,7 @@ async function runQueuedTask( onClientAvailable: options.onClientAvailable, onClientClosed: options.onClientClosed, onPromptActive: options.onPromptActive, + onAgentPid: options.onAgentPid, }); if (task.waitForCompletion) { @@ -450,6 +453,9 @@ async function runSessionPrompt( }, onConnectedRecord: (connectedRecord) => { connectedRecord.lastPromptAt = isoNow(); + if (connectedRecord.pid != null) { + options.onAgentPid?.(connectedRecord.pid); + } }, onSessionIdResolved: (sessionId) => { activeSessionIdForControl = sessionId; @@ -747,6 +753,7 @@ export async function runSessionQueueOwner( } let owner: SessionQueueOwner | undefined; + let lastAgentPid: number | undefined; const ttlMs = normalizeQueueOwnerTtlMs(options.ttlMs); const taskPollTimeoutMs = ttlMs === 0 ? undefined : ttlMs; const initialTaskPollTimeoutMs = @@ -859,13 +866,23 @@ export async function runSessionQueueOwner( authCredentials: options.authCredentials, authPolicy: options.authPolicy, suppressSdkConsoleErrors: options.suppressSdkConsoleErrors, - onClientAvailable: setActiveController, + onClientAvailable: (controller) => { + setActiveController(controller); + }, onClientClosed: clearActiveController, onPromptActive: async () => { turnController.markPromptActive(); await applyPendingCancel(); }, + onAgentPid: (pid) => { + lastAgentPid = pid; + }, }); + // Track the agent pid after each task so we can kill it when the queue owner exits + const record = await resolveSessionRecord(options.sessionId).catch(() => null); + if (record?.pid != null) { + lastAgentPid = record.pid; + } }); } } finally { @@ -874,6 +891,15 @@ export async function runSessionQueueOwner( await owner.close(); } await releaseQueueOwnerLease(lease); + // Kill the agent process if it is still alive (e.g. kiro-cli which does not self-exit) + if (lastAgentPid != null && isProcessAlive(lastAgentPid)) { + await terminateProcess(lastAgentPid).catch(() => {}); + if (options.verbose) { + process.stderr.write( + `[acpx] killed agent pid ${lastAgentPid} on queue owner exit for session ${options.sessionId}\n`, + ); + } + } if (options.verbose) { process.stderr.write( `[acpx] queue owner stopped for session ${options.sessionId}\n`, diff --git a/test/agent-registry.test.ts b/test/agent-registry.test.ts index 45dbd62..97472e6 100644 --- a/test/agent-registry.test.ts +++ b/test/agent-registry.test.ts @@ -12,6 +12,7 @@ test("resolveAgentCommand maps known agents to commands", () => { ["claude", "npx -y @zed-industries/claude-agent-acp"], ["gemini", "gemini"], ["opencode", "npx -y opencode-ai acp"], + ["kiro", "kiro-cli acp"], ["pi", "npx pi-acp"], ]); @@ -24,12 +25,12 @@ test("resolveAgentCommand returns raw value for unknown agents", () => { assert.equal(resolveAgentCommand("custom-acp-server"), "custom-acp-server"); }); -test("listBuiltInAgents returns exactly all 5 registered agent names", () => { +test("listBuiltInAgents returns exactly all 6 registered agent names", () => { const agents = listBuiltInAgents(); - assert.equal(agents.length, 5); + assert.equal(agents.length, 6); assert.deepEqual( new Set(agents), - new Set(["codex", "claude", "gemini", "opencode", "pi"]), + new Set(["codex", "claude", "gemini", "opencode", "kiro", "pi"]), ); }); diff --git a/test/process-group-kill.test.ts b/test/process-group-kill.test.ts new file mode 100644 index 0000000..e05ca00 --- /dev/null +++ b/test/process-group-kill.test.ts @@ -0,0 +1,117 @@ +/** + * Verifies that spawning with detached:true and killing via process.kill(-pgid) + * terminates the entire process group, including grandchild processes. + * + * This test demonstrates the fix for orphan kiro-cli-chat processes: + * - WITHOUT the fix: only the parent (kiro-cli wrapper) is killed; child survives + * - WITH the fix: entire process group is killed; no orphans + */ +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import test from "node:test"; + +function isRunning(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +test("process group kill terminates parent and grandchild (no orphan)", async () => { + // Spawn a parent shell that forks a grandchild (simulates kiro-cli → kiro-cli-chat) + // Parent prints its PID, then forks a long-running grandchild, then sleeps. + const parent = spawn( + "bash", + [ + "-c", + // Print parent pid, fork grandchild sleep, wait + 'echo "PARENT=$$"; sleep 60 & echo "CHILD=$!"; wait', + ], + { detached: true, stdio: ["pipe", "pipe", "pipe"] }, + ); + + // Collect stdout to get parent and child PIDs + let output = ""; + parent.stdout.on("data", (chunk: Buffer) => { + output += chunk.toString(); + }); + + // Wait until both PIDs are printed + await new Promise((resolve) => { + const interval = setInterval(() => { + if (output.includes("PARENT=") && output.includes("CHILD=")) { + clearInterval(interval); + resolve(); + } + }, 50); + }); + + const parentPid = parseInt(output.match(/PARENT=(\d+)/)![1]); + const childPid = parseInt(output.match(/CHILD=(\d+)/)![1]); + + assert.ok(isRunning(parentPid), "parent should be running before kill"); + assert.ok(isRunning(childPid), "grandchild should be running before kill"); + + // Kill entire process group (the fix) + assert.ok(parent.pid != null); + process.kill(-parent.pid, "SIGTERM"); + + // Give processes time to terminate + await sleep(200); + + assert.ok(!isRunning(parentPid), "parent should be dead after process group kill"); + assert.ok( + !isRunning(childPid), + "grandchild should be dead after process group kill (no orphan)", + ); +}); + +test("killing only parent pid leaves grandchild as orphan (demonstrates the bug)", async () => { + const parent = spawn( + "bash", + ["-c", 'echo "PARENT=$$"; sleep 60 & echo "CHILD=$!"; wait'], + { detached: true, stdio: ["pipe", "pipe", "pipe"] }, + ); + + let output = ""; + parent.stdout.on("data", (chunk: Buffer) => { + output += chunk.toString(); + }); + + await new Promise((resolve) => { + const interval = setInterval(() => { + if (output.includes("PARENT=") && output.includes("CHILD=")) { + clearInterval(interval); + resolve(); + } + }, 50); + }); + + const parentPid = parseInt(output.match(/PARENT=(\d+)/)![1]); + const childPid = parseInt(output.match(/CHILD=(\d+)/)![1]); + + // Kill only the parent (old behavior — no process group kill) + process.kill(parentPid, "SIGTERM"); + + await sleep(200); + + assert.ok(!isRunning(parentPid), "parent should be dead"); + // Grandchild survives as orphan — this is the bug the fix addresses + assert.ok( + isRunning(childPid), + "grandchild survives as orphan without process group kill", + ); + + // Cleanup orphan + try { + process.kill(childPid, "SIGKILL"); + } catch { + // already gone + } +});