diff --git a/README.md b/README.md index 8baf8b1..48e2d55 100644 --- a/README.md +++ b/README.md @@ -290,16 +290,28 @@ Suite-level `withAspect` **before** hook that: 2. Sets `this.model` when provided; sets `this.image` from `options.image`, a registered `options.variant`, or the resolved agent 3. Asserts that Docker image already exists (`docker image inspect`) — it does **not** build. Build once in `globalSetup` with `buildAgentImage(...)` / `buildToolchainImage(...)` so parallel test files do not race +`timeoutMs` (optional) cancels a run that takes longer, see [Cancellation](#cancellation-and-timeouts). + Pair workspace lifecycle separately: `withAspect(a_workspace, cleanup_workspace)`. ## What `executing_the_agent` does 1. Requires `this.workspace`, `this.prompt`, and `this.agent` 2. Calls `this.agent.run(...)` with `this.image`: - - Cursor: `docker run` with credentials-only mount + `agent -p --force --output-format json [--model …] -- ` - - Claude: `docker run` with the workspace mount and credentials forwarded by env **name** (the value never appears on the host command line) or a read-only `.credentials.json` mount + `claude -p --output-format json --dangerously-skip-permissions [--model …] -- ` + - Cursor: `docker run` with credentials-only mount + `agent -p --force --output-format json [--model …]` with the prompt on stdin + - Claude: `docker run` with the workspace mount and credentials forwarded by env **name** (the value never appears on the host command line) or a read-only `.credentials.json` mount + `claude -p --output-format json --dangerously-skip-permissions [--model …]` with the prompt on stdin 3. Sets `this.agentResult` to the parsed JSON +## Cancellation and timeouts + +Every `docker run` gets a unique `--name`, and a run can be cancelled; cancelling force-removes the container so the agent stops working (and billing) at once. Three things cancel a run: + +- `agent({ timeoutMs })`: the run is cancelled after that many milliseconds and `executing_the_agent` throws `agent run exceeded ms`. +- The test finishing first, for example on Vitest's own timeout: the container is removed and no second error is reported. +- The process exiting while a run is still tracked: a best-effort `docker rm -f` on the way out. The signal handlers assume Vitest's default `forks` pool; under `threads` only the exit hook applies. + +The prompt is sent to the container on stdin rather than as a command-line argument, so there is no size limit (Linux caps one argument at 128 KB) and the prompt never appears in `ps` output on the host. + ## Exports | Export | Role | diff --git a/e2e/cancel.spec.ts b/e2e/cancel.spec.ts new file mode 100644 index 0000000..79bbdba --- /dev/null +++ b/e2e/cancel.spec.ts @@ -0,0 +1,80 @@ +import { execFileSync } from "node:child_process"; +import { describe, expect } from "vitest"; +import test from "vitest-gwt"; +import { BASE_IMAGE, runDocker } from "../src/index.js"; + +type Context = { + name: string; + controller: AbortController; + stdout: string; +}; + +function hasBaseImage(): boolean { + try { + execFileSync("docker", ["image", "inspect", BASE_IMAGE], { stdio: "ignore" }); + return true; + } catch { + return false; + } +} + +describe.skipIf(!hasBaseImage())("cancellation and stdin against real docker", () => { + test("an aborted run is rejected and its container is gone", { + given: { + a_named_container_and_a_controller_that_aborts_shortly, + }, + when: { + running_a_sleeping_container, + }, + then: { + expect_error: run_was_cancelled_and_container_is_gone, + }, + }); + + test("a 300 KB prompt reaches the container on stdin", { + when: { + piping_a_large_stdin_through_wc, + }, + then: { + byte_count_matches, + }, + }); +}); + +function a_named_container_and_a_controller_that_aborts_shortly(this: Context) { + this.name = `agent-gwt-e2e-cancel-${Date.now()}`; + this.controller = new AbortController(); + setTimeout(() => this.controller.abort(new Error("test timeout")), 1_000); +} + +async function running_a_sleeping_container(this: Context) { + await runDocker(["run", "--rm", "--name", this.name, BASE_IMAGE, "sleep", "60"], { + signal: this.controller.signal, + containerName: this.name, + }); +} + +async function run_was_cancelled_and_container_is_gone(this: Context, error: Error) { + expect(error.message).toContain("docker run cancelled: test timeout"); + const listed = await runDocker([ + "ps", + "-a", + "--filter", + `name=${this.name}`, + "--format", + "{{.Names}}", + ]); + expect(listed.stdout.trim()).toBe(""); +} + +async function piping_a_large_stdin_through_wc(this: Context) { + const prompt = "x".repeat(300 * 1024); + const result = await runDocker(["run", "--rm", "-i", BASE_IMAGE, "sh", "-c", "wc -c"], { + stdin: prompt, + }); + this.stdout = result.stdout; +} + +function byte_count_matches(this: Context) { + expect(this.stdout.trim()).toBe(String(300 * 1024)); +} diff --git a/src/agents/_containerName.ts b/src/agents/_containerName.ts new file mode 100644 index 0000000..d9a40b8 --- /dev/null +++ b/src/agents/_containerName.ts @@ -0,0 +1,6 @@ +import { randomBytes } from "node:crypto"; + +/** A unique `--name` per run, so the container can be found and force-removed. */ +export function containerName(agent: string): string { + return `agent-gwt-${agent}-${randomBytes(4).toString("hex")}`; +} diff --git a/src/agents/_containerRegistry.ts b/src/agents/_containerRegistry.ts new file mode 100644 index 0000000..f05f1d3 --- /dev/null +++ b/src/agents/_containerRegistry.ts @@ -0,0 +1,32 @@ +import { removeContainer } from "./_removeContainer.js"; + +const running = new Set(); +let hooksInstalled = false; + +/** Track a running container so process exit or a termination signal can force-remove it. */ +export function trackContainer(name: string): void { + running.add(name); + + if (!hooksInstalled) { + hooksInstalled = true; + process.once("exit", removeAll); + for (const signal of ["SIGTERM", "SIGINT"] as const) { + process.once(signal, () => { + removeAll(); + // re-raise so the process still terminates + process.kill(process.pid, signal); + }); + } + } +} + +export function untrackContainer(name: string): void { + running.delete(name); +} + +function removeAll(): void { + for (const container of running) { + removeContainer(container); + } + running.clear(); +} diff --git a/src/agents/_removeContainer.ts b/src/agents/_removeContainer.ts new file mode 100644 index 0000000..d4b7519 --- /dev/null +++ b/src/agents/_removeContainer.ts @@ -0,0 +1,6 @@ +import { spawnSync } from "node:child_process"; + +/** Synchronous so it also works from an exit handler and is not cut short by worker teardown. */ +export function removeContainer(name: string): void { + spawnSync("docker", ["rm", "-f", name], { stdio: "ignore" }); +} diff --git a/src/agents/_runProcess.spec.ts b/src/agents/_runProcess.spec.ts new file mode 100644 index 0000000..d91d1dd --- /dev/null +++ b/src/agents/_runProcess.spec.ts @@ -0,0 +1,93 @@ +import { describe, expect } from "vitest"; +import test from "vitest-gwt"; + +import { runProcess } from "./_runProcess.js"; +import type { DockerRunResult } from "./types.js"; + +type Context = { + result: DockerRunResult; + controller: AbortController; + abortCalls: number; +}; + +const ECHO_STDIN = + "process.stdin.on('data', (d) => process.stdout.write(d)); process.stdin.on('end', () => process.exit(0));"; +const HANG = "setTimeout(() => {}, 30_000);"; + +describe("runProcess", () => { + test("feeds stdin to the process and captures its output", { + when: { + running_an_echo_process_with_stdin, + }, + then: { + stdout_is_the_stdin, + exit_code_is_zero, + }, + }); + + test("kills the process, runs onAbort, and rejects when the signal aborts", { + given: { + a_controller_that_aborts_shortly, + }, + when: { + running_a_hanging_process, + }, + then: { + expect_error: error_says_cancelled_and_on_abort_ran, + }, + }); + + test("rejects at once when the signal is already aborted", { + given: { + an_already_aborted_controller, + }, + when: { + running_a_hanging_process, + }, + then: { + expect_error: error_says_cancelled_with_the_reason, + }, + }); +}); + +async function running_an_echo_process_with_stdin(this: Context) { + this.result = await runProcess(process.execPath, ["-e", ECHO_STDIN], { stdin: "hello, stdin" }); +} + +function stdout_is_the_stdin(this: Context) { + expect(this.result.stdout).toBe("hello, stdin"); +} + +function exit_code_is_zero(this: Context) { + expect(this.result.exitCode).toBe(0); +} + +function a_controller_that_aborts_shortly(this: Context) { + this.controller = new AbortController(); + this.abortCalls = 0; + setTimeout(() => this.controller.abort(new Error("took too long")), 10); +} + +function an_already_aborted_controller(this: Context) { + this.controller = new AbortController(); + this.abortCalls = 0; + this.controller.abort(new Error("took too long")); +} + +async function running_a_hanging_process(this: Context) { + await runProcess(process.execPath, ["-e", HANG], { + signal: this.controller.signal, + onAbort: () => { + this.abortCalls += 1; + }, + }); +} + +function error_says_cancelled_with_the_reason(this: Context, error: Error) { + expect(error.message).toContain("run cancelled: took too long"); +} + +function error_says_cancelled_and_on_abort_ran(this: Context, error: Error) { + error_says_cancelled_with_the_reason.call(this, error); + expect(this.abortCalls).toBe(1); +} diff --git a/src/agents/_runProcess.ts b/src/agents/_runProcess.ts new file mode 100644 index 0000000..1228135 --- /dev/null +++ b/src/agents/_runProcess.ts @@ -0,0 +1,84 @@ +import { spawn } from "node:child_process"; + +import type { DockerRunOptions, DockerRunResult } from "./types.js"; + +type RunProcessOptions = Omit & { + /** Called once on abort, before the process is killed. */ + onAbort?: () => void; +}; + +export function runProcess( + command: string, + args: string[], + options: RunProcessOptions = {}, +): Promise { + return new Promise((resolve, reject) => { + const { signal } = options; + + if (signal?.aborted === true) { + reject(cancelledError(command, signal.reason)); + return; + } + + const child = spawn(command, args, { + stdio: [options.stdin === undefined ? "ignore" : "pipe", "pipe", "pipe"], + env: { ...process.env, ...options.env }, + }); + + let stdout = ""; + let stderr = ""; + let cancelled: Error | undefined; + const inheritOutput = options.inheritOutput === true; + + child.stdout?.on("data", (chunk: Buffer | string) => { + const text = chunk.toString(); + stdout += text; + if (inheritOutput) { + process.stdout.write(text); + } + }); + child.stderr?.on("data", (chunk: Buffer | string) => { + const text = chunk.toString(); + stderr += text; + if (inheritOutput) { + process.stderr.write(text); + } + }); + + if (options.stdin !== undefined && child.stdin !== null) { + // Ignore EPIPE: the child may exit before draining stdin. + child.stdin.on("error", () => undefined); + child.stdin.end(options.stdin); + } + + const onAbort = () => { + cancelled = cancelledError(command, signal?.reason); + try { + options.onAbort?.(); + } finally { + child.kill("SIGKILL"); + } + }; + signal?.addEventListener("abort", onAbort, { once: true }); + + child.on("error", (error: Error) => { + signal?.removeEventListener("abort", onAbort); + const hint = command === "docker" ? " Is Docker installed and running?" : ""; + reject(new Error(`Failed to start ${command}: ${error.message}.${hint}`)); + }); + + child.on("close", (exitCode: number | null) => { + signal?.removeEventListener("abort", onAbort); + if (cancelled !== undefined) { + reject(cancelled); + return; + } + resolve({ exitCode, stdout, stderr }); + }); + }); +} + +function cancelledError(command: string, reason: unknown): Error { + const detail = reason instanceof Error ? reason.message : String(reason); + return new Error(`${command} run cancelled: ${detail}`); +} diff --git a/src/agents/claude/_buildDockerArgs.spec.ts b/src/agents/claude/_buildDockerArgs.spec.ts index e29f72f..7d42b93 100644 --- a/src/agents/claude/_buildDockerArgs.spec.ts +++ b/src/agents/claude/_buildDockerArgs.spec.ts @@ -21,7 +21,7 @@ const envFlagValues = (args: string[]) => args.filter((arg, i) => args[i - 1] == const volumeMounts = (args: string[]) => args.filter((arg, i) => args[i - 1] === "-v"); describe("buildClaudeDockerArgs", () => { - test("runs as host user with an OAuth token forwarded by name only", { + test("runs as host user, named, with stdin open and an OAuth token forwarded by name only", { given: { oauth_token_credentials, }, @@ -93,8 +93,8 @@ function credentials_file_credentials(this: Context) { function building_docker_args(this: Context) { this.args = buildClaudeDockerArgs({ workspace: "/tmp/.agents-gwt/ws-abc", - prompt: "Create a README", image: "agent-gwt/claude-code:local", + containerName: "agent-gwt-claude-test", credentials: this.credentials, uid: 1000, gid: 1000, @@ -104,8 +104,8 @@ function building_docker_args(this: Context) { function building_docker_args_with_model(this: Context) { this.args = buildClaudeDockerArgs({ workspace: "/tmp/.agents-gwt/ws-abc", - prompt: "Create a README", image: "agent-gwt/claude-code:local", + containerName: "agent-gwt-claude-test", credentials: this.credentials, uid: 1000, gid: 1000, @@ -174,13 +174,12 @@ function invokes_claude_headless_with_json_output(this: Context) { expect(this.args).toContain("--output-format"); expect(this.args).toContain("json"); expect(this.args).toContain("--dangerously-skip-permissions"); - expect(this.args.at(-2)).toBe("--"); - expect(this.args.at(-1)).toBe("Create a README"); + expect(this.args).toContain("-i"); + expect(this.args[this.args.indexOf("--name") + 1]).toBe("agent-gwt-claude-test"); } function includes_model_flag(this: Context) { const modelIndex = this.args.indexOf("--model"); expect(modelIndex).toBeGreaterThan(-1); expect(this.args[modelIndex + 1]).toBe("sonnet"); - expect(this.args.indexOf("--")).toBeGreaterThan(modelIndex); } diff --git a/src/agents/claude/_buildDockerArgs.ts b/src/agents/claude/_buildDockerArgs.ts index bf21dac..b40e0c0 100644 --- a/src/agents/claude/_buildDockerArgs.ts +++ b/src/agents/claude/_buildDockerArgs.ts @@ -7,8 +7,8 @@ import { CLAUDE_CONTAINER_CREDENTIALS_PATH } from "./constants.js"; export function buildClaudeDockerArgs(options: { workspace: string; - prompt: string; image: string; + containerName?: string; credentials: ClaudeCredentials; uid: number; gid: number; @@ -20,8 +20,6 @@ export function buildClaudeDockerArgs(options: { claudeArgs.push("--model", options.model); } - claudeArgs.push("--", options.prompt); - const volumes: DockerVolumeMount[] = [ { host: options.workspace, container: CONTAINER_WORKSPACE }, ]; @@ -39,6 +37,8 @@ export function buildClaudeDockerArgs(options: { uid: options.uid, gid: options.gid, workdir: CONTAINER_WORKSPACE, + ...(options.containerName !== undefined ? { name: options.containerName } : {}), + interactive: true, env: { HOME: CONTAINER_HOME }, // Names only; the values reach the container through the docker CLI's own environment. envPassthrough: Object.keys(credentialsEnv(options.credentials)), diff --git a/src/agents/claude/run.spec.ts b/src/agents/claude/run.spec.ts index 8add4b5..7782cb6 100644 --- a/src/agents/claude/run.spec.ts +++ b/src/agents/claude/run.spec.ts @@ -14,6 +14,7 @@ type Context = { dockerRunner: DockerRunner; lastArgs: string[]; lastRunOptions: DockerRunOptions | undefined; + signal: AbortSignal | undefined; }; describe("runClaudeInDocker", () => { @@ -45,6 +46,20 @@ describe("runClaudeInDocker", () => { }, }); + test("forwards an abort signal to the docker runner", { + given: { + successful_docker_runner, + oauth_token_credentials, + an_abort_signal, + }, + when: { + running_claude_in_docker, + }, + then: { + docker_runner_received_the_signal, + }, + }); + test("throws when docker exits non-zero", { given: { failing_docker_runner, @@ -140,6 +155,7 @@ async function running_claude_in_docker(this: Context) { credentials: this.credentials, uid: 1000, gid: 1000, + ...(this.signal !== undefined ? { signal: this.signal } : {}), }, this.dockerRunner, ); @@ -164,7 +180,12 @@ function docker_runner_received_api_key_env(this: Context) { function docker_runner_received_claude_args(this: Context) { expect(this.lastArgs).toContain("claude"); - expect(this.lastArgs.at(-1)).toBe("hi"); + expect(this.lastArgs).not.toContain("hi"); + expect(this.lastRunOptions?.stdin).toBe("hi"); + expect(this.lastRunOptions?.containerName).toMatch(/^agent-gwt-claude-[0-9a-f]{8}$/); + expect(this.lastArgs[this.lastArgs.indexOf("--name") + 1]).toBe( + this.lastRunOptions?.containerName, + ); } function error_includes_exit_code(this: Context, error: Error) { @@ -179,3 +200,11 @@ function error_includes_claude_message(this: Context, error: Error) { expect(error.message).toContain("Invalid API key"); expect(error.message).toContain("error_during_execution"); } + +function an_abort_signal(this: Context) { + this.signal = new AbortController().signal; +} + +function docker_runner_received_the_signal(this: Context) { + expect(this.lastRunOptions?.signal).toBe(this.signal); +} diff --git a/src/agents/claude/run.ts b/src/agents/claude/run.ts index 6083178..c52c880 100644 --- a/src/agents/claude/run.ts +++ b/src/agents/claude/run.ts @@ -2,6 +2,7 @@ import { runDocker } from "../docker.js"; import { parseAgentJsonOutput } from "../parse-result.js"; import { agentRunError } from "../run-error.js"; import type { AgentRunBindingsOptions, DockerRunner } from "../types.js"; +import { containerName } from "../_containerName.js"; import { buildClaudeDockerArgs } from "./_buildDockerArgs.js"; import { credentialsEnv } from "./_credentialsEnv.js"; import { type ClaudeCredentials, resolveClaudeCredentials } from "./_resolveCredentials.js"; @@ -37,17 +38,23 @@ export async function runClaudeInDocker( const uid = options.uid ?? process.getuid?.() ?? 0; const gid = options.gid ?? process.getgid?.() ?? 0; + const name = containerName("claude"); const args = buildClaudeDockerArgs({ workspace: options.workspace, - prompt: options.prompt, image: options.image, + containerName: name, credentials, uid, gid, ...(options.model !== undefined ? { model: options.model } : {}), }); - const result = await dockerRunner(args, { env: credentialsEnv(credentials) }); + const result = await dockerRunner(args, { + env: credentialsEnv(credentials), + stdin: options.prompt, + containerName: name, + ...(options.signal !== undefined ? { signal: options.signal } : {}), + }); if (result.exitCode !== 0) { throw agentRunError({ diff --git a/src/agents/cursor/_buildDockerArgs.spec.ts b/src/agents/cursor/_buildDockerArgs.spec.ts index 011378c..176dedf 100644 --- a/src/agents/cursor/_buildDockerArgs.spec.ts +++ b/src/agents/cursor/_buildDockerArgs.spec.ts @@ -37,8 +37,8 @@ describe("buildDockerArgs", () => { function building_docker_args(this: Context) { this.args = buildDockerArgs({ workspace: "/tmp/.agents-gwt/ws-abc", - prompt: "Create a README", image: "agent-gwt/cursor-cli:local", + containerName: "agent-gwt-cursor-test", authFile: "/home/dev/.config/cursor/auth.json", uid: 1000, gid: 1000, @@ -48,8 +48,8 @@ function building_docker_args(this: Context) { function building_docker_args_with_model(this: Context) { this.args = buildDockerArgs({ workspace: "/tmp/.agents-gwt/ws-abc", - prompt: "Create a README", image: "agent-gwt/cursor-cli:local", + containerName: "agent-gwt-cursor-test", authFile: "/home/dev/.config/cursor/auth.json", uid: 1000, gid: 1000, @@ -89,12 +89,12 @@ function invokes_agent_with_json_output(this: Context) { expect(this.args).toContain("--output-format"); expect(this.args).toContain("json"); expect(this.args).toContain("--force"); - expect(this.args.at(-1)).toBe("Create a README"); + expect(this.args).toContain("-i"); + expect(this.args[this.args.indexOf("--name") + 1]).toBe("agent-gwt-cursor-test"); } function includes_model_flag(this: Context) { const modelIndex = this.args.indexOf("--model"); expect(modelIndex).toBeGreaterThan(-1); expect(this.args[modelIndex + 1]).toBe("composer-2"); - expect(this.args.indexOf("--")).toBeGreaterThan(modelIndex); } diff --git a/src/agents/cursor/_buildDockerArgs.ts b/src/agents/cursor/_buildDockerArgs.ts index cb752c3..ee2cb0f 100644 --- a/src/agents/cursor/_buildDockerArgs.ts +++ b/src/agents/cursor/_buildDockerArgs.ts @@ -4,8 +4,8 @@ import { CONTAINER_AUTH_PATH } from "./constants.js"; export function buildDockerArgs(options: { workspace: string; - prompt: string; image: string; + containerName?: string; authFile: string; uid: number; gid: number; @@ -17,13 +17,13 @@ export function buildDockerArgs(options: { agentArgs.push("--model", options.model); } - agentArgs.push("--", options.prompt); - return buildDockerRunArgs({ image: options.image, uid: options.uid, gid: options.gid, workdir: CONTAINER_WORKSPACE, + ...(options.containerName !== undefined ? { name: options.containerName } : {}), + interactive: true, env: { HOME: CONTAINER_HOME }, volumes: [ { host: options.workspace, container: CONTAINER_WORKSPACE }, diff --git a/src/agents/cursor/run.spec.ts b/src/agents/cursor/run.spec.ts index 4929da1..e699eaa 100644 --- a/src/agents/cursor/run.spec.ts +++ b/src/agents/cursor/run.spec.ts @@ -3,12 +3,14 @@ import test from "vitest-gwt"; import { join } from "node:path"; import { runCursorInDocker } from "./run.js"; -import type { DockerRunner } from "../types.js"; +import type { DockerRunOptions, DockerRunner } from "../types.js"; type Context = { result: unknown; dockerRunner: DockerRunner; authFile: string; + lastArgs: string[]; + lastRunOptions: DockerRunOptions | undefined; }; describe("runCursorInDocker", () => { @@ -22,6 +24,7 @@ describe("runCursorInDocker", () => { }, then: { agent_result_is_parsed, + prompt_went_to_stdin_of_a_named_container, }, }); @@ -40,11 +43,11 @@ describe("runCursorInDocker", () => { }); function successful_docker_runner(this: Context) { - this.dockerRunner = async () => ({ - exitCode: 0, - stdout: '{"ok":true}', - stderr: "", - }); + this.dockerRunner = async (args, options) => { + this.lastArgs = args; + this.lastRunOptions = options; + return { exitCode: 0, stdout: '{"ok":true}', stderr: "" }; + }; } function failing_docker_runner(this: Context) { @@ -80,3 +83,12 @@ function agent_result_is_parsed(this: Context) { function error_includes_exit_code(this: Context, error: Error) { expect(error.message).toContain("exited with code 1"); } + +function prompt_went_to_stdin_of_a_named_container(this: Context) { + expect(this.lastArgs).not.toContain("hi"); + expect(this.lastRunOptions?.stdin).toBe("hi"); + expect(this.lastRunOptions?.containerName).toMatch(/^agent-gwt-cursor-[0-9a-f]{8}$/); + expect(this.lastArgs[this.lastArgs.indexOf("--name") + 1]).toBe( + this.lastRunOptions?.containerName, + ); +} diff --git a/src/agents/cursor/run.ts b/src/agents/cursor/run.ts index 2a6425e..7570e3f 100644 --- a/src/agents/cursor/run.ts +++ b/src/agents/cursor/run.ts @@ -6,6 +6,7 @@ import { runDocker } from "../docker.js"; import { parseAgentJsonOutput } from "../parse-result.js"; import { agentRunError } from "../run-error.js"; import type { DockerRunner, AgentRunBindingsOptions } from "../types.js"; +import { containerName } from "../_containerName.js"; import { buildDockerArgs } from "./_buildDockerArgs.js"; import { defaultHostAuthFile } from "./constants.js"; @@ -31,17 +32,22 @@ export async function runCursorInDocker( ); } + const name = containerName("cursor"); const args = buildDockerArgs({ workspace: options.workspace, - prompt: options.prompt, image: options.image, + containerName: name, authFile, uid, gid, ...(options.model !== undefined ? { model: options.model } : {}), }); - const result = await dockerRunner(args); + const result = await dockerRunner(args, { + stdin: options.prompt, + containerName: name, + ...(options.signal !== undefined ? { signal: options.signal } : {}), + }); if (result.exitCode !== 0) { throw agentRunError({ agent: "Cursor", name: "cursor", image: options.image, result }); diff --git a/src/agents/docker.spec.ts b/src/agents/docker.spec.ts index e643770..a9e6df6 100644 --- a/src/agents/docker.spec.ts +++ b/src/agents/docker.spec.ts @@ -38,6 +38,16 @@ describe("buildDockerRunArgs", () => { passthrough_env_is_name_only, }, }); + + test("names the container and keeps stdin open when asked", { + when: { + building_args_with_name_and_stdin, + }, + then: { + names_the_container, + keeps_stdin_open, + }, + }); }); function building_args(this: Context) { @@ -112,3 +122,23 @@ function passthrough_env_is_name_only(this: Context) { const envValues = this.args.filter((arg, i) => this.args[i - 1] === "-e"); expect(envValues).toEqual(["HOME=/home/agent", "SECRET_TOKEN"]); } + +function building_args_with_name_and_stdin(this: Context) { + this.args = buildDockerRunArgs({ + image: "example:local", + uid: 1, + gid: 1, + workdir: "/workspace", + name: "agent-gwt-test-1234", + interactive: true, + command: ["true"], + }); +} + +function names_the_container(this: Context) { + expect(this.args[this.args.indexOf("--name") + 1]).toBe("agent-gwt-test-1234"); +} + +function keeps_stdin_open(this: Context) { + expect(this.args).toContain("-i"); +} diff --git a/src/agents/docker.ts b/src/agents/docker.ts index 635e1b2..e21ba47 100644 --- a/src/agents/docker.ts +++ b/src/agents/docker.ts @@ -1,5 +1,6 @@ -import { spawn } from "node:child_process"; - +import { trackContainer, untrackContainer } from "./_containerRegistry.js"; +import { removeContainer } from "./_removeContainer.js"; +import { runProcess } from "./_runProcess.js"; import type { BuildDockerRunArgsOptions, DockerRunResult, DockerRunner } from "./types.js"; export type { @@ -10,46 +11,35 @@ export type { DockerVolumeMount, } from "./types.js"; -export const runDocker: DockerRunner = (args, options = {}) => - new Promise((resolve, reject) => { - const child = spawn("docker", args, { - stdio: ["ignore", "pipe", "pipe"], - env: { ...process.env, ...options.env }, - }); - - let stdout = ""; - let stderr = ""; - const inheritOutput = options.inheritOutput === true; +export const runDocker: DockerRunner = async (args, options = {}) => { + const { containerName, ...processOptions } = options; - child.stdout.on("data", (chunk: Buffer | string) => { - const text = chunk.toString(); - stdout += text; - if (inheritOutput) { - process.stdout.write(text); - } - }); - child.stderr.on("data", (chunk: Buffer | string) => { - const text = chunk.toString(); - stderr += text; - if (inheritOutput) { - process.stderr.write(text); - } - }); - - child.on("error", (error: Error) => { - reject( - new Error(`Failed to start docker: ${error.message}. Is Docker installed and running?`), - ); - }); + if (containerName === undefined) { + return runProcess("docker", args, processOptions); + } - child.on("close", (exitCode: number | null) => { - resolve({ exitCode, stdout, stderr }); + trackContainer(containerName); + try { + return await runProcess("docker", args, { + ...processOptions, + onAbort: () => removeContainer(containerName), }); - }); + } finally { + untrackContainer(containerName); + } +}; export function buildDockerRunArgs(options: BuildDockerRunArgsOptions): string[] { const args = ["run", "--rm", "--user", `${options.uid}:${options.gid}`]; + if (options.name !== undefined) { + args.push("--name", options.name); + } + + if (options.interactive === true) { + args.push("-i"); + } + if (options.env !== undefined) { for (const [key, value] of Object.entries(options.env)) { args.push("-e", `${key}=${value}`); diff --git a/src/agents/types.ts b/src/agents/types.ts index 82c8ee7..9afe226 100644 --- a/src/agents/types.ts +++ b/src/agents/types.ts @@ -6,6 +6,8 @@ export type RunAgentOptions = { model?: string; /** Override the agent's default image (e.g. a toolchain-extended tag). */ image?: string; + /** Aborting it force-removes the container and rejects the run. */ + signal?: AbortSignal; }; export type AgentRunBindingsOptions = RunAgentOptions & { @@ -21,6 +23,8 @@ export type Agent = { export type AgentOptions = { model?: string; + /** Cancel a run, and remove its container, after this many milliseconds. */ + timeoutMs?: number; /** * Named toolchain registered via `buildToolchainImage(variant, ...)`. * Mutually exclusive with `image`. @@ -45,6 +49,12 @@ export type DockerRunOptions = { * appearing on the host command line. */ env?: Record; + /** Written to the process's stdin, then closed. Pair with `interactive` on `docker run`. */ + stdin?: string; + /** Aborting it kills the process, force-removes `containerName` when set, and rejects. */ + signal?: AbortSignal; + /** The `--name` given to `docker run`, so an abort or a process exit can `docker rm -f` it. */ + containerName?: string; }; export type DockerRunner = (args: string[], options?: DockerRunOptions) => Promise; @@ -65,6 +75,10 @@ export type BuildDockerRunArgsOptions = { env?: Record; /** `-e NAME` — value is read from the docker CLI's own environment, never on argv. */ envPassthrough?: string[]; + /** `--name`, so the container can be force-removed on cancellation. */ + name?: string; + /** `-i`, keep stdin open so the prompt can be piped in. */ + interactive?: boolean; volumes?: DockerVolumeMount[]; }; diff --git a/src/given/agent.spec.ts b/src/given/agent.spec.ts index bfce72e..41b31d5 100644 --- a/src/given/agent.spec.ts +++ b/src/given/agent.spec.ts @@ -88,6 +88,18 @@ describe("agent", () => { }, }); + test("puts timeoutMs on the context", { + given: { + stub_ensure_docker_image, + }, + when: { + applying_agent: agent({ name: "cursor", timeoutMs: 1234 }), + }, + then: { + timeout_is: timeout_is(1234), + }, + }); + test("resolves the claude agent by name", { given: { stub_ensure_docker_image, @@ -196,3 +208,9 @@ function error_mentions_unknown_variant(this: Context) { function error_mentions_mutual_exclusion(this: Context) { expect(this.error?.message).toContain("cannot set both image and variant"); } + +function timeout_is(timeoutMs: number) { + return function (this: Context) { + expect(this.timeoutMs).toBe(timeoutMs); + }; +} diff --git a/src/given/agent.ts b/src/given/agent.ts index 501a4af..83f3a16 100644 --- a/src/given/agent.ts +++ b/src/given/agent.ts @@ -19,6 +19,10 @@ export function agent(options: ConfigureAgentOptions) { this.model = options.model; } + if (options.timeoutMs !== undefined) { + this.timeoutMs = options.timeoutMs; + } + await ensureDockerImage(this.image); }; } diff --git a/src/types.ts b/src/types.ts index b51ae19..182de59 100644 --- a/src/types.ts +++ b/src/types.ts @@ -11,4 +11,5 @@ export type AgentContext = { agent: Agent; image: string; model?: string; + timeoutMs?: number; }; diff --git a/src/when/executing_the_agent.spec.ts b/src/when/executing_the_agent.spec.ts index 69b9f3c..17917f2 100644 --- a/src/when/executing_the_agent.spec.ts +++ b/src/when/executing_the_agent.spec.ts @@ -46,6 +46,18 @@ describe("executing_the_agent", () => { }, }); + test("cancels the run after timeoutMs and reports it", { + given: { + workspace_prompt_and_a_slow_agent_with_a_timeout, + }, + when: { + executing_the_agent, + }, + then: { + expect_error: error_reports_the_timeout, + }, + }); + test("requires a prompt", { given: { workspace_and_agent_only, @@ -125,6 +137,7 @@ function agent_was_called_with_workspace_and_prompt(this: Context) { workspace: "/tmp/.agents-gwt/ws-test", prompt: "Create a README", image: "agent-gwt/test:local", + signal: expect.any(AbortSignal), }); } @@ -134,6 +147,7 @@ function agent_was_called_with_model(this: Context) { prompt: "Create a README", image: "agent-gwt/test:local", model: "composer-2", + signal: expect.any(AbortSignal), }); } @@ -148,3 +162,23 @@ function error_requires_prompt(this: Context, error: Error) { function error_requires_agent(this: Context, error: Error) { expect(error.message).toContain("this.agent"); } + +function workspace_prompt_and_a_slow_agent_with_a_timeout(this: Context) { + this.workspace = "/tmp/.agents-gwt/ws-test"; + this.prompt = "Create a README"; + this.image = "agent-gwt/test:local"; + this.timeoutMs = 10; + this.agent = { + image: "agent-gwt/test:local", + ensureImage: async () => undefined, + buildImage: async () => undefined, + run: ({ signal }) => + new Promise((_resolve, reject) => { + signal?.addEventListener("abort", () => reject(signal.reason as Error)); + }), + }; +} + +function error_reports_the_timeout(this: Context, error: Error) { + expect(error.message).toContain("exceeded 10 ms"); +} diff --git a/src/when/executing_the_agent.ts b/src/when/executing_the_agent.ts index 13839fe..e1b2864 100644 --- a/src/when/executing_the_agent.ts +++ b/src/when/executing_the_agent.ts @@ -15,10 +15,38 @@ export async function executing_the_agent(this: AgentContext): Promise { ); } - this.agentResult = await this.agent.run({ - workspace: this.workspace, - prompt: this.prompt, - image: this.image, - ...(this.model !== undefined ? { model: this.model } : {}), - }); + const controller = new AbortController(); + await abortWhenTestFinishes(controller); + + const timeoutMs = this.timeoutMs; + const timer = + timeoutMs === undefined + ? undefined + : setTimeout(() => { + controller.abort(new Error(`agent run exceeded ${timeoutMs} ms (agent({ timeoutMs }))`)); + }, timeoutMs); + + try { + this.agentResult = await this.agent.run({ + workspace: this.workspace, + prompt: this.prompt, + image: this.image, + signal: controller.signal, + ...(this.model !== undefined ? { model: this.model } : {}), + }); + } finally { + clearTimeout(timer); + } +} + +/** Under vitest, abort when the test finishes first (typically its own timeout); other runners skip this. */ +async function abortWhenTestFinishes(controller: AbortController): Promise { + try { + const { onTestFinished } = (await import("vitest")) as { + onTestFinished?: (fn: () => void) => void; + }; + onTestFinished?.(() => controller.abort(new Error("the test finished before the agent did"))); + } catch { + // vitest absent, or not inside a running test + } }