diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f79df6b..7309953 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -38,3 +38,18 @@ jobs: - name: Test run: pnpm test + + timeout-cleanup: + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [macos-latest, windows-latest] + steps: + - uses: actions/checkout@v6 + - uses: pnpm/action-setup@v4 + - uses: actions/setup-node@v6 + with: + node-version: 24 + cache: pnpm + - run: pnpm i --frozen-lockfile + - run: pnpm vitest run test/invoke.test.ts -t "deadline cleanup" diff --git a/README.md b/README.md index ff018e4..c5b1206 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ console.log(claude.hooks); // [{ path: ".claude/hooks/", scope: "project", ... } console.log(claude.invocationModes); // advisor and full agent modes, no read-only mode const codex = getHarness("codex"); -await codex.invoke("Review this patch", { readOnly: true }); +await codex.invoke("Review this patch", { readOnly: true, timeoutMs: 60_000 }); const pi = getHarness("pi"); const { models } = await pi.listModels({ search: "gpt-5.4" }); @@ -89,6 +89,10 @@ import type { ClaudeSessionEntry, CodexThread, GeminiConversationRecord } from " Primary references: [Antigravity prompting](https://antigravity.google/docs/cli/prompting/), [Antigravity changelog](https://github.com/google-antigravity/antigravity-cli/blob/main/CHANGELOG.md), [Gemini CLI tools](https://github.com/google-gemini/gemini-cli/blob/main/docs/reference/tools.md), [Gemini CLI video request](https://github.com/google-gemini/gemini-cli/issues/27194), [OpenCode attachments](https://opencode.ai/v2/docs/attachments/), [Cursor prompting](https://cursor.com/docs/agent/prompting), and [Copilot CLI voice input](https://docs.github.com/en/copilot/how-tos/copilot-cli/use-copilot-cli/voice-input). +`invoke()` and `listModels()` accept `timeoutMs`. Unset or `0` means no deadline. On Linux and macOS, timed commands run in their own process group: the deadline sends `SIGTERM`, then `SIGKILL` after 500 ms, even if the root has already exited. Windows uses `taskkill /T /F` immediately, with a 2 s budget for that command. Cleanup failures reject the call. Timed results retain captured output with `timedOut: true` and `exitCode: null`. + +This is command cleanup, not a sandbox. Descendants that leave the POSIX process group, or outlive an already exited root on Windows, cannot be reliably reached by these mechanisms. Inherited output pipes do not extend the wait after cleanup. Scheduling and OS delays can exceed the stated budgets. + Each agent is a concrete subclass of the abstract `Harness` class. Custom subclasses can be added with `registerHarness`. Every harness exposes config paths, session locations, instruction files, skills dirs, hooks, commands, persistence formats, capabilities (MCP, vision, audio, video, tools, streaming), detection rules, a normalized non-interactive invocation (`harness.invoke(prompt, { model })`) where the CLI has a headless mode, native model listing (`harness.listModels()`) where the CLI supports it, and its MCP server config files (`listMcpServers`/`addMcpServer`/`removeMcpServer` normalize the dialects; writes rewrite JSON and surgically edit TOML with comments preserved). `syncMcpServers` treats `~/.config/agntn/mcp.jsonc` (JSONC, XDG-aware) as the single source of truth and resets every harness's user-scope MCP config to exactly that list; a top-level `"excludes": ["codex"]` array opts individual harnesses out of the sync (their own servers stay, master-listed names are withdrawn), and `~`/`${HOME}` in commands, args, and env values expand to absolute paths at sync time (harnesses spawn MCP servers without a shell). `syncAgentsFiles` links every harness's global instructions file (CLAUDE.md/AGENTS.md/GEMINI.md) to one master file as symlinks, so an edit made through any harness lands in the single physical copy; `~/.config/agntn/agents.jsonc` sets the `source`, `companions`, and `excludes`, diverged regular files are backed up and relinked, and check mode reports without writing. Companion paths are relative to the source directory and are linked at the same relative path beside each harness target. ```jsonc diff --git a/src/harness.ts b/src/harness.ts index c1e1dfc..f717fcf 100644 --- a/src/harness.ts +++ b/src/harness.ts @@ -1,5 +1,6 @@ import { existsSync } from "node:fs"; -import { execFileSync, spawn } from "node:child_process"; +import { execFile, execFileSync, spawn } from "node:child_process"; +import { setTimeout as delay } from "node:timers/promises"; import { join } from "node:path"; import type { HarnessDetection, @@ -132,6 +133,36 @@ function buildInvocationArgs( return args; } +function signalProcessGroup(pid: number, signal: NodeJS.Signals): void { + try { + process.kill(-pid, signal); + } catch (error) { + if (!(error instanceof Error && "code" in error && error.code === "ESRCH")) throw error; + } +} + +/** + * Keep escalation alive even when the root exits before its descendants. + * @param pid - Root of the command's process group or Windows tree. + */ +async function terminateCommand(pid: number): Promise { + if (process.platform === "win32") { + await new Promise((resolve, reject) => { + execFile( + join(process.env.SystemRoot ?? "C:\\Windows", "System32", "taskkill.exe"), + ["/PID", String(pid), "/T", "/F"], + { windowsHide: true, timeout: 2000, killSignal: "SIGKILL" }, + (error) => (error ? reject(error) : resolve()), + ); + }); + return; + } + + signalProcessGroup(pid, "SIGTERM"); + await delay(500); + signalProcessGroup(pid, "SIGKILL"); +} + function executeCommand( command: string, args: readonly string[], @@ -142,6 +173,7 @@ function executeCommand( cwd: options.cwd, env: options.env ? { ...process.env, ...options.env } : process.env, stdio: ["ignore", "pipe", "pipe"], + detached: process.platform !== "win32" && Boolean(options.timeoutMs), }); let stdout = ""; @@ -150,7 +182,19 @@ function executeCommand( const timer = options.timeoutMs ? setTimeout(() => { timedOut = true; - child.kill("SIGTERM"); + if (child.pid === undefined) return; + void terminateCommand(child.pid).then( + () => { + child.stdout.destroy(); + child.stderr.destroy(); + finish(null); + }, + (error: unknown) => { + child.stdout.destroy(); + child.stderr.destroy(); + reject(error); + }, + ); }, options.timeoutMs) : undefined; @@ -168,6 +212,10 @@ function executeCommand( }); child.on("close", (code) => { if (timer) clearTimeout(timer); + if (!timedOut) finish(code); + }); + + function finish(code: number | null): void { resolve({ command, args: [...args], @@ -176,7 +224,7 @@ function executeCommand( exitCode: timedOut ? null : code, timedOut, }); - }); + } }); } diff --git a/src/types.ts b/src/types.ts index 97643a1..bcab609 100644 --- a/src/types.ts +++ b/src/types.ts @@ -100,7 +100,7 @@ export interface ListModelsOptions { search?: string; cwd?: string; env?: Record; - /** Kill the model-listing command after this many milliseconds; unset means no timeout. */ + /** Milliseconds before cleanup starts. Unset or 0 disables the deadline. */ timeoutMs?: number; } @@ -113,7 +113,7 @@ export interface InvokeOptions { tools?: boolean; /** Require native enforcement of read-only tool access. Implies `tools: true`. */ readOnly?: boolean; - /** Kill the harness after this many milliseconds; unset means no timeout. */ + /** Milliseconds before cleanup starts. Unset or 0 disables the deadline. */ timeoutMs?: number; /** Use the harness's structured (JSON) output mode instead of plain text. */ structured?: boolean; diff --git a/test/invoke.test.ts b/test/invoke.test.ts index 9446ebb..a7bf74e 100644 --- a/test/invoke.test.ts +++ b/test/invoke.test.ts @@ -1,4 +1,7 @@ import { describe, expect, it } from "vitest"; +import { mkdtemp, readFile, readdir, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { getHarness, registerHarness } from "../src/index.ts"; import { Harness } from "../src/harness.ts"; import Cursor from "../src/harnesses/cursor.ts"; @@ -27,7 +30,7 @@ class FakeCursor extends Harness { readonly skills: Harness["skills"] = []; readonly commands: Harness["commands"] = []; readonly hooks: Harness["hooks"] = []; - readonly mcpConfigs: Harness["mcpConfigs"] = []; + override readonly mcpConfigs: Harness["mcpConfigs"] = []; readonly detection = { envVars: [], projectMarkers: [] }; readonly invocation: Harness["invocation"] = { args: ["-e", "console.log('echo:' + process.argv[1]); process.exitCode = 0", "{prompt}"], @@ -193,6 +196,140 @@ describe("normalized invocation", () => { }); }); +describe.each(["invoke", "listModels"] as const)("%s deadline cleanup", (operation) => { + it("preserves output and exit status when the command finishes before its deadline", async () => { + const args = ["-e", "console.log('out'); console.error('err'); process.exitCode = 3"]; + const fake = new (class extends FakeCursor { + override readonly binaries = [process.execPath]; + override readonly invocation: Harness["invocation"] = { args, level: "inferred" }; + override readonly modelListing: Harness["modelListing"] = { args, level: "inferred" }; + })(); + const options = { timeoutMs: 1000, tools: true }; + const result = await (operation === "invoke" + ? fake.invoke("x", options) + : fake.listModels(options)); + expect(result).toMatchObject({ + stdout: "out\n", + stderr: "err\n", + exitCode: 3, + timedOut: false, + }); + }); + + it("rejects a spawn failure without waiting for its deadline", async () => { + const fake = new (class extends FakeCursor { + override readonly binaries = [join(tmpdir(), "agntn-missing-harness-binary")]; + override readonly modelListing: Harness["modelListing"] = { args: [], level: "inferred" }; + })(); + const options = { timeoutMs: 1000 }; + await expect( + operation === "invoke" ? fake.invoke("x", options) : fake.listModels(options), + ).rejects.toMatchObject({ code: "ENOENT" }); + }); + + it.each(["ignore", "grace", "inherit", "ignore-stdio"] as const)( + "cleans up the %s fixture within the deadline and cleanup budget", + async (mode) => { + const directory = await mkdtemp(join(tmpdir(), "harness-timeout-")); + const leaf = ` + const { writeFileSync } = require('node:fs'); + const { join } = require('node:path'); + process.on('SIGTERM', () => {}); + writeFileSync(join(process.argv[1], 'leaf'), String(process.pid)); + setTimeout(() => process.exit(0), 5000); + `; + const script = ` + const { writeFileSync } = require('node:fs'); + const { join } = require('node:path'); + writeFileSync(join(process.argv[1], 'root'), String(process.pid)); + if (process.argv[2] === 'ignore' || process.argv[2] === 'grace') { + process.on('SIGTERM', () => { + if (process.argv[2] === 'grace') { + setTimeout(() => { console.log('cleaned'); process.exit(0); }, 100); + } + }); + console.log('ready'); + } else { + require('node:child_process').spawn(process.execPath, ['-e', ${JSON.stringify(leaf)}, process.argv[1]], { + stdio: process.argv[2] === 'inherit' ? 'inherit' : 'ignore' + }); + } + setTimeout(() => process.exit(0), 5000); + `; + const args = ["-e", script, directory, mode]; + const fake = new (class extends FakeCursor { + override readonly binaries = [process.execPath]; + override readonly invocation: Harness["invocation"] = { args, level: "inferred" }; + override readonly modelListing: Harness["modelListing"] = { args, level: "inferred" }; + })(); + + try { + const start = performance.now(); + const options = { timeoutMs: 1000, tools: true }; + const result = await (operation === "invoke" + ? fake.invoke("x", options) + : fake.listModels(options)); + + expect(result.timedOut).toBe(true); + expect(result.exitCode).toBeNull(); + expect(performance.now() - start).toBeLessThan(3000); + const rootOnly = mode === "ignore" || mode === "grace"; + if (rootOnly) { + expect(result.stdout).toBe( + mode === "grace" && process.platform !== "win32" ? "ready\ncleaned\n" : "ready\n", + ); + } + const names = await readdir(directory); + expect(names.sort()).toEqual(rootOnly ? ["root"] : ["leaf", "root"]); + for (const name of names) { + const pid = Number(await readFile(join(directory, name), "utf8")); + await expect.poll(() => processIsRunning(pid), { timeout: 1000 }).toBe(false); + } + } finally { + await cleanupFixture(directory); + } + }, + 10000, + ); +}); + +async function cleanupFixture(directory: string): Promise { + for (const name of await readdir(directory)) { + const pid = Number(await readFile(join(directory, name), "utf8")); + try { + process.kill(pid, "SIGKILL"); + } catch (error) { + if (!(error instanceof Error && "code" in error && error.code === "ESRCH")) throw error; + } + } + await rm(directory, { recursive: true, force: true }); +} + +/** + * Linux may retain an orphan's PID as a zombie after it has stopped executing. + * @param pid - PID written by a fixture process. + * @returns {Promise} Whether the process can still execute. + */ +async function processIsRunning(pid: number): Promise { + try { + process.kill(pid, 0); + if (process.platform === "linux") { + const stat = await readFile(`/proc/${pid}/stat`, "utf8"); + return stat.slice(stat.lastIndexOf(")") + 2, stat.lastIndexOf(")") + 3) !== "Z"; + } + return true; + } catch (error) { + if ( + error instanceof Error && + "code" in error && + ["ESRCH", "ENOENT"].includes(String(error.code)) + ) { + return false; + } + throw error; + } +} + describe("harness metadata for agents", () => { it("exposes invocation modes through the harness API", () => { expect(getHarness("claude").invocationModes).toEqual({