diff --git a/evals/terminal-bench/README.md b/evals/terminal-bench/README.md index 5f08056d4..6e16ca110 100644 --- a/evals/terminal-bench/README.md +++ b/evals/terminal-bench/README.md @@ -61,12 +61,12 @@ The task container must provide Debian/Ubuntu Linux with glibc, root access, Bas Linux x86_64 and aarch64 have separate pinned Node archive checksums. The installer fails explicitly on unsupported platforms and refuses to reuse an existing installation. -[pins.toml](src/diffex_terminal_bench/pins.toml) records Node `22.23.2`, Diffex `0.2.7`, the public npm registry, archive checksums, npm integrity, and a 600-second installation deadline. +[pins.toml](src/diffex_terminal_bench/pins.toml) records Node `22.23.2`, Diffex `0.3.0`, the public npm registry, archive checksums, npm integrity, and a 600-second installation deadline. The deadline includes apt prerequisites, Node download, npm installation, and all three CLI checks. GNU `timeout` terminates the installer process group at the deadline and kills it after a ten-second grace period. Harbor's command transport allows another five seconds to collect the result. -Setup installs Node under `/opt/diffex-terminal-bench/node` and uses `npm install --ignore-scripts --save-exact --registry=https://registry.npmjs.org/ @diffexai/diffex@0.2.7` with a private prefix. +Setup installs Node under `/opt/diffex-terminal-bench/node` and uses `npm install --ignore-scripts --save-exact --registry=https://registry.npmjs.org/ @diffexai/diffex@0.3.0` with a private prefix. An isolated home and npm cache prevent host or task npm settings from changing installation. Node archive SHA-256, registry metadata, installed package version, and npm lockfile integrity must match the checked-in pins. The `/usr/local/bin/diffex` wrapper always invokes the pinned Node binary. @@ -132,7 +132,7 @@ Run `npm run check` from the repository root after changing code. ### Recorded validation The following historical smoke records used Diffex 0.2.2. -They do not qualify the updated Diffex 0.2.6 / Terminal-Bench 2.1 configuration. +They do not qualify the updated Diffex 0.3.0 / Terminal-Bench 2.1 configuration. On 2026-09-14, both pinned Ubuntu images passed through Harbor 0.23.0 on Docker Desktop with an arm64 host. @@ -250,11 +250,11 @@ Startup network updates are disabled with the supported offline setting so the p The `context-only` project policy retains ancestor `AGENTS.md`/`CLAUDE.md` context and disables project trust, discovered extensions, skills, prompt templates, and themes. It does not pass `--no-context-files` or replace the system prompt. -The pinned npm `0.2.6` release includes `dist/AGENTS.md`, which matches the adapter's checked-in release instructions. +The pinned npm `0.3.0` release includes `dist/AGENTS.md`, which matches the adapter's checked-in release instructions. The adapter retains that packaged context without adding a global duplicate. The effective system prompt must contain the release instructions, and their SHA-256 is recorded. -The checked-in `benchmark.toml` and `execution.yaml` enable subagents, adding `spawn_agent`, `agent_action`, and `wait_for_agents` to the explicit tool allowlist. +The checked-in `benchmark.toml` and `execution.yaml` enable subagents, adding `spawn_agent`, `agent_action`, and `agent_status` to the explicit tool allowlist. Diffex decides whether to call those tools; enabling them does not require a child on every task. Children share their trial's sandbox, and the pinned CLI owns child scheduling and its default concurrency limit. The staged runner's `-n/--concurrency` controls simultaneous trials and sandboxes separately. @@ -304,7 +304,7 @@ Tool errors alone do not fail execution: the assistant can inspect and recover f `DiffexExecutionError` subclasses Harbor's `NonZeroAgentExitCodeError`, so infrastructure failures retain exception information rather than becoming ordinary zero-reward results. The adapter never sets or infers verifier reward. A completed assistant that produces an incorrect solution returns normally and can earn zero from the verifier. -The pinned `0.2.6` release returns nonzero for terminal assistant errors and aborts in JSON mode as well as text mode. +The pinned `0.3.0` release returns nonzero for terminal assistant errors and aborts in JSON mode as well as text mode. The adapter also parses native events to classify provider failures and reject incomplete output independently of process exit status. The launcher writes stdout and stderr directly to files and retains native sessions during execution. diff --git a/evals/terminal-bench/RELEASE_GATE.md b/evals/terminal-bench/RELEASE_GATE.md index 8008f9cfe..c96ca2f7a 100644 --- a/evals/terminal-bench/RELEASE_GATE.md +++ b/evals/terminal-bench/RELEASE_GATE.md @@ -26,9 +26,9 @@ Each invocation extracts a fresh copy of the verified archive; it does not reuse The preparation command writes `results/release-gate/dataset.json` without creating sandboxes or calling a model. Harbor 0.23.0 and Daytona SDK 0.210.0 are locked in the benchmark's private Python environment. -Node 22.23.2, Diffex 0.2.6, npm integrity, registry, and Node archive checksums come from [pins.toml](src/diffex_terminal_bench/pins.toml). +Node 22.23.2, Diffex 0.3.0, npm integrity, registry, and Node archive checksums come from [pins.toml](src/diffex_terminal_bench/pins.toml). The default is `openai/gpt-5.6-luna`, `max` thinking, tools `read,bash,edit,write,update_plan`, enabled subagents, disabled evolution, and `context-only` project resources. -Enabled subagents add `spawn_agent`, `agent_action`, and `wait_for_agents` to the model's tools; Diffex chooses whether to use them. +Enabled subagents add `spawn_agent`, `agent_action`, and `agent_status` to the model's tools; Diffex chooses whether to use them. Children execute inside the parent trial's sandbox and their sessions and usage remain in the trial evidence. Change and commit the configuration before starting a new qualification sequence. Every subsequent stage must match the exact recorded identity, including the adapter commit and dependency lock. diff --git a/evals/terminal-bench/src/diffex_terminal_bench/execution.py b/evals/terminal-bench/src/diffex_terminal_bench/execution.py index 9ae82dc95..10506fa86 100644 --- a/evals/terminal-bench/src/diffex_terminal_bench/execution.py +++ b/evals/terminal-bench/src/diffex_terminal_bench/execution.py @@ -17,7 +17,7 @@ "mistral": ("MISTRAL_API_KEY",), } BUILTIN_TOOLS = ("read", "bash", "edit", "write", "update_plan", "grep", "find", "ls") -SUBAGENT_TOOLS = ("spawn_agent", "agent_action", "wait_for_agents") +SUBAGENT_TOOLS = ("spawn_agent", "agent_action", "agent_status") class ExecutionOptions(InstalledAgentOptions): diff --git a/evals/terminal-bench/src/diffex_terminal_bench/pins.toml b/evals/terminal-bench/src/diffex_terminal_bench/pins.toml index 186c5913d..2d7df9d72 100644 --- a/evals/terminal-bench/src/diffex_terminal_bench/pins.toml +++ b/evals/terminal-bench/src/diffex_terminal_bench/pins.toml @@ -1,8 +1,8 @@ harbor_version = "0.23.0" node_version = "22.23.2" -diffex_version = "0.2.7" +diffex_version = "0.3.0" npm_registry = "https://registry.npmjs.org/" -diffex_integrity = "sha512-+LXbkWtgLZ/AMxN3uKnUFpCeG1iw1mH4DMKhBx1SjKYmmvZw7NWFCiLdlMFSFrHTt6N5IE9YV8FIjefzxx36eg==" +diffex_integrity = "sha512-MeD6G7az/b/SSj4yXrtN3bgRB4tPFwR26pVToPuvBDOTr8VU0ZtKNz0f/GLt5d4X4dx5d4/8FQzWn16LpYhSyw==" # Must match the instructions shipped by diffex_version, not the current checkout. release_agents_sha256 = "6f194cd7f04d80e08cd3c8b22096de33ba5dbb2ba42176567c466579f8ec26db" setup_timeout_sec = 600 diff --git a/evals/terminal-bench/tests/test_execution.py b/evals/terminal-bench/tests/test_execution.py index 270a3183d..257a9fbe1 100644 --- a/evals/terminal-bench/tests/test_execution.py +++ b/evals/terminal-bench/tests/test_execution.py @@ -112,7 +112,8 @@ async def upload(source, target): agent = DiffexAgent(logs_dir=self.logs_dir, model_name="openai/gpt-5", subagents="enabled", extra_env={"OPENAI_API_KEY": "test-only"}) await agent.run("Task", environment, AgentContext()) - self.assertTrue({"spawn_agent", "agent_action", "wait_for_agents"}.issubset(configs[0]["tools"])) + self.assertEqual(configs[0]["tools"], ["read", "bash", "edit", "write", "update_plan", + "spawn_agent", "agent_action", "agent_status"]) self.assertTrue(any("chown -R 1000" in call.kwargs["command"] for call in environment.exec.call_args_list)) async def test_checked_in_eval_policies_expose_subagent_tools(self): @@ -134,7 +135,7 @@ async def upload(source, target): await agent.run("Task", environment, AgentContext()) self.assertEqual(requested[0]["subagents"], "enabled") self.assertEqual(requested[0]["tools"], ["read", "bash", "edit", "write", "update_plan", - "spawn_agent", "agent_action", "wait_for_agents"]) + "spawn_agent", "agent_action", "agent_status"]) async def test_missing_credentials_and_ambiguous_models_fail_before_exec(self): with patch.dict(os.environ, {}, clear=True): diff --git a/packages/coding-agent/docs/background-processes.md b/packages/coding-agent/docs/background-processes.md new file mode 100644 index 000000000..88d0a41cc --- /dev/null +++ b/packages/coding-agent/docs/background-processes.md @@ -0,0 +1,75 @@ +# Background process lifetime + +Local shell commands normally belong to their Diffex session. +Abort, disposal, print-mode exit, and normal subagent completion terminate surviving background jobs, including jobs started with `nohup`. + +An environment supervisor can explicitly take responsibility for successful background jobs on Linux and macOS: + +```sh +diffex --print \ + --background-process-lifetime=environment \ + --background-process-ownership-dir=/run/trial/diffex-shell-ownership \ + "Start the task service" +``` + +The default lifetime is `session`. +Environment lifetime requires an explicit ownership directory and applies to all built-in local bash executions, direct session bash commands, and child sessions in that invocation. +Relative CLI directory paths are resolved against the initial working directory, including after session replacement. +Custom or remote `BashOperations` implementations manage their own process lifecycle. + +A shell process group becomes environment-owned only when its launching shell exits with code `0` before cancellation or timeout. +Handoff happens at shell exit, before the next provider request or final assistant response. +Failed commands and active commands remain session-owned. +Once handed off, background processes survive later provider errors, interruption, child failure, session replacement, and disposal. +The failed or interrupted invocation still retains its failure outcome. + +The environment supervisor must terminate transferred processes when the environment is destroyed. +Services should redirect their output to files: environment mode bounds inherited stdout/stderr draining to 100 milliseconds after shell exit so a continuously logging service cannot hold the CLI open. + +## SDK + +```ts +const { session } = await createAgentSession({ + backgroundProcessPolicy: { + lifetime: "environment", + ownershipDirectory: "/run/trial/diffex-shell-ownership", + }, +}); +``` + +The same option is available on `AgentSessionConfig` and `createAgentSessionFromServices`. +SDK ownership directories must be absolute. +Runtime factories should capture this invocation-level policy and pass it to each replacement session. +Child sessions inherit the parent policy, including children restored within the invocation. + +## Supervisor handoff records + +Use a separate ownership directory for each environment execution. +Diffex writes one versioned JSON file per shell execution, named `.json`. +The execution ID also appears in the inherited `DIFFEX_SHELL_EXECUTION_ID` environment variable. +Records contain `version: 1`, `executionId`, `processGroupId`, `supervisorPid`, `owner`, `state`, and a snapshot of live `members`. +Each member contains its PID, process group ID, and process start identity. +Linux identities combine the boot ID and `/proc//stat` start ticks; macOS identities use `ps lstart` in UTC and the C locale. + +The initial record is `owner: "session", state: "active"`. +Successful handoff replaces it with `owner: "environment", state: "background"`. +Confirmed termination of an untransferred group records `owner: "session", state: "terminated"`. +Member snapshots are evidence at publication time, not a continuously updated process inventory. + +Each replacement is flushed to a temporary file and atomically renamed before the process group is removed from session and global shutdown tracking. +Only the completed `.json` file establishes ownership; ignore `.tmp` files. +The atomic rename is the handoff commit point, so a supervisor can recover ownership after a CLI crash or SIGKILL without depending on session-history flushing or a successful final response. +Publication failure leaves ownership with Diffex and fails the shell operation. +Publication and in-memory release contain no asynchronous boundary, so ordinary cancellation cannot interleave with the handoff. + +A supervisor must validate current process identities before signaling or preserving a recorded group and follow inherited execution markers for newly created descendants. +Never signal a numeric PID or group ID from a stale record without checking identity. +Missing, invalid, or ambiguous records are not proof of an environment handoff. +Records are a cooperation protocol within the trusted trial environment, not a security boundary against task code that can modify that environment. + +## Terminal-Bench integration + +This option provides the Diffex ownership boundary described by process-lifetime issue 1. +The adapter must enable it for every task once its pinned Diffex release supports these flags and its pre-verifier cleanup consumes the ownership records. +The separate adapter cleanup change in issue 2 is still required: marker-based cleanup that kills all trial processes will also kill transferred services. +Final environment deletion remains responsible for removing task services after verification. diff --git a/packages/coding-agent/src/cli/args.ts b/packages/coding-agent/src/cli/args.ts index 664d67432..8f6f03044 100644 --- a/packages/coding-agent/src/cli/args.ts +++ b/packages/coding-agent/src/cli/args.ts @@ -11,6 +11,8 @@ import type { TuiMode } from "../core/settings-manager.ts"; export type Mode = "text" | "json" | "rpc"; export interface Args { + backgroundProcessLifetime?: "session" | "environment"; + backgroundProcessOwnershipDir?: string; provider?: string; model?: string; apiKey?: string; @@ -82,6 +84,25 @@ export function parseArgs(args: string[]): Args { if (mode === "text" || mode === "json" || mode === "rpc") { result.mode = mode; } + } else if (arg === "--background-process-lifetime" || arg.startsWith("--background-process-lifetime=")) { + const value = arg.includes("=") ? arg.slice(arg.indexOf("=") + 1) : args[i + 1]; + if (!arg.includes("=") && value !== undefined && !value.startsWith("-")) i++; + if (value === "session" || value === "environment") result.backgroundProcessLifetime = value; + else + result.diagnostics.push({ + type: "error", + message: "--background-process-lifetime requires session or environment", + }); + } else if ( + arg === "--background-process-ownership-dir" || + arg.startsWith("--background-process-ownership-dir=") + ) { + const value = arg.includes("=") ? arg.slice(arg.indexOf("=") + 1) : args[i + 1]; + if (value && !value.startsWith("-")) { + result.backgroundProcessOwnershipDir = value; + if (!arg.includes("=")) i++; + } else + result.diagnostics.push({ type: "error", message: "--background-process-ownership-dir requires a path" }); } else if (arg === "--continue" || arg === "-c") { result.continue = true; } else if (arg === "--resume" || arg === "-r") { @@ -222,6 +243,18 @@ export function parseArgs(args: string[]): Args { } } + if (result.backgroundProcessLifetime === "environment" && !result.backgroundProcessOwnershipDir) { + result.diagnostics.push({ + type: "error", + message: "Environment lifetime requires --background-process-ownership-dir", + }); + } + if (result.backgroundProcessOwnershipDir && result.backgroundProcessLifetime !== "environment") { + result.diagnostics.push({ + type: "error", + message: "--background-process-ownership-dir requires environment lifetime", + }); + } return result; } @@ -259,6 +292,10 @@ ${chalk.bold("Options:")} --append-system-prompt Append text or file contents to the system prompt (can be used multiple times) --mode Output mode: text (default), json, or rpc --print, -p Non-interactive mode: process prompt and exit + --background-process-lifetime + Background shell ownership (default: session) + --background-process-ownership-dir + Supervisor handoff records (required for environment lifetime) --continue, -c Continue previous session --resume, -r Select a session to resume --session Use specific session file or partial UUID diff --git a/packages/coding-agent/src/core/agent-session-services.ts b/packages/coding-agent/src/core/agent-session-services.ts index 51c3c1878..01aea6ccc 100644 --- a/packages/coding-agent/src/core/agent-session-services.ts +++ b/packages/coding-agent/src/core/agent-session-services.ts @@ -59,6 +59,7 @@ export interface CreateAgentSessionServicesOptions { * have been resolved against those services. */ export interface CreateAgentSessionFromServicesOptions { + backgroundProcessPolicy?: CreateAgentSessionOptions["backgroundProcessPolicy"]; services: AgentSessionServices; sessionManager: SessionManager; sessionStartEvent?: SessionStartEvent; @@ -240,6 +241,7 @@ export async function createInternalAgentSessionFromServices( registerSubagentRuntimeDiagnosticSink(options.sessionManager, options.services.diagnostics); return createInternalAgentSession( { + backgroundProcessPolicy: options.backgroundProcessPolicy, cwd: options.services.cwd, agentDir: options.services.agentDir, modelRuntime: options.services.modelRuntime, diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index a65508259..667307dd1 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -57,7 +57,7 @@ import type { HarnessWorkspaceIdentity, } from "@diffexai/diffex-harness-state"; import { getThemeByName, theme } from "../modes/interactive/theme/theme.ts"; -import { OwnedShellProcesses } from "../utils/owned-shell-processes.ts"; +import { type BackgroundProcessPolicy, OwnedShellProcesses } from "../utils/owned-shell-processes.ts"; import { canonicalizePath, resolvePath } from "../utils/paths.ts"; import { sleep } from "../utils/sleep.ts"; import { normalizeToolResultImages } from "../utils/tool-result-images.ts"; @@ -303,6 +303,8 @@ function withoutDeletedHeaders(headers: ProviderHeaders | undefined): Record(); private _pendingBashMessages: BashExecutionMessage[] = []; private readonly _ownedOperations = new Set>(); - private readonly _shellProcesses = new OwnedShellProcesses(); + private readonly _shellProcesses: OwnedShellProcesses; private readonly _bashOperations = new Set>(); // Extension system @@ -583,6 +585,7 @@ export class AgentSession { private _disposePromise: Promise | undefined; constructor(config: AgentSessionConfig) { + this._shellProcesses = new OwnedShellProcesses(config.backgroundProcessPolicy); this.agent = config.agent; this.sessionManager = config.sessionManager; this.settingsManager = config.settingsManager; @@ -3089,6 +3092,10 @@ export class AgentSession { await this._shellProcesses.terminateAll(); } + get backgroundProcessPolicy(): BackgroundProcessPolicy { + return this._shellProcesses.policy; + } + get ownedShellProcessCount(): number { return this._shellProcesses.count; } diff --git a/packages/coding-agent/src/core/sdk.ts b/packages/coding-agent/src/core/sdk.ts index 66415dfdd..ceea86385 100644 --- a/packages/coding-agent/src/core/sdk.ts +++ b/packages/coding-agent/src/core/sdk.ts @@ -12,6 +12,7 @@ import { clampThinkingLevel, type Message, type Model, streamSimple } from "@dif import type { HarnessStateStore, HarnessVersionBundle } from "@diffexai/diffex-harness-state"; import { EvolutionMemoryWriter, FileEvolutionPersistence } from "#evolution"; import { getAgentDir, VERSION } from "../config.ts"; +import type { BackgroundProcessPolicy } from "../utils/owned-shell-processes.ts"; import { resolvePath } from "../utils/paths.ts"; import { AgentSession } from "./agent-session.ts"; import type { AgentSessionLifecycleObserver } from "./agent-session-lifecycle.ts"; @@ -81,6 +82,8 @@ export interface SubagentOptions extends SubagentSupervisionOptions { } export interface CreateAgentSessionOptions { + /** Local shell background lifetime. Defaults to session-owned cleanup. */ + backgroundProcessPolicy?: BackgroundProcessPolicy; /** Working directory for project-local discovery. Default: process.cwd() */ cwd?: string; /** Global config directory. Default: ~/.diffex/agent */ @@ -665,6 +668,7 @@ export async function createInternalAgentSession( } const session = new AgentSession({ + backgroundProcessPolicy: options.backgroundProcessPolicy, agent, sessionManager, settingsManager, diff --git a/packages/coding-agent/src/core/subagents/services.ts b/packages/coding-agent/src/core/subagents/services.ts index 3f39ff7a4..241dad2b2 100644 --- a/packages/coding-agent/src/core/subagents/services.ts +++ b/packages/coding-agent/src/core/subagents/services.ts @@ -2,6 +2,7 @@ import type { TelemetryContext, ThinkingLevel } from "@diffexai/diffex-agent-cor import type { Model } from "@diffexai/diffex-ai"; import { clampThinkingLevel } from "@diffexai/diffex-ai/compat"; import type { HarnessStateStore, HarnessVersionBundle } from "@diffexai/diffex-harness-state"; +import type { BackgroundProcessPolicy } from "../../utils/owned-shell-processes.ts"; import type { AgentSession } from "../agent-session.ts"; import { type AgentSessionServices, @@ -32,6 +33,7 @@ export interface CreateSubagentServicesOptions { } export interface SubagentSessionConfig { + readonly backgroundProcessPolicy?: BackgroundProcessPolicy; readonly cwd: string; readonly agentDir: string; readonly modelRuntime: ModelRuntime; @@ -105,6 +107,7 @@ export function captureSubagentSessionConfig(parent: AgentSession, agentDir: str ); return Object.freeze({ + backgroundProcessPolicy: parent.backgroundProcessPolicy, cwd: parent.sessionManager.getCwd(), agentDir, modelRuntime: parent.modelRuntime, @@ -218,6 +221,7 @@ export async function createSubagentSession( const { session } = await createInternalAgentSessionFromServices( { services, + backgroundProcessPolicy: config.backgroundProcessPolicy, sessionManager, model: config.model, thinkingLevel: config.thinkingLevel, diff --git a/packages/coding-agent/src/core/tools/bash.ts b/packages/coding-agent/src/core/tools/bash.ts index bf528fad4..78acc713b 100644 --- a/packages/coding-agent/src/core/tools/bash.ts +++ b/packages/coding-agent/src/core/tools/bash.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "node:crypto"; import { constants } from "node:fs"; import { access as fsAccess } from "node:fs/promises"; import type { AgentTool } from "@diffexai/diffex-agent-core"; @@ -16,6 +17,7 @@ import { trackDetachedChildPid, untrackDetachedChildPid, } from "../../utils/shell.ts"; +import { SHELL_EXECUTION_ID_ENV } from "../../utils/shell-process-identity.ts"; import type { ExtensionContext, ToolDefinition, ToolRenderResultOptions } from "../extensions/types.ts"; import { OutputAccumulator } from "./output-accumulator.ts"; import { getTextOutput, invalidArgText, str } from "./render-utils.ts"; @@ -104,10 +106,11 @@ export function createLocalBashOperations(options?: { signal?.throwIfAborted(); const commandFromStdin = shellConfig.commandTransport === "stdin"; + const executionId = randomUUID(); const child = spawn(shellConfig.shell, commandFromStdin ? shellConfig.args : [...shellConfig.args, command], { cwd, detached: process.platform !== "win32", - env: env ?? getShellEnv(), + env: { ...(env ?? getShellEnv()), [SHELL_EXECUTION_ID_ENV]: executionId }, stdio: [commandFromStdin ? "pipe" : "ignore", "pipe", "pipe"], windowsHide: true, }); @@ -115,22 +118,44 @@ export function createLocalBashOperations(options?: { child.stdin?.on("error", () => {}); child.stdin?.end(command); } - if (child.pid) { - if (options?.owner) options.owner.track(child.pid); - else trackDetachedChildPid(child.pid); - } let timedOut = false; + const environmentLifetime = options?.owner?.policy.lifetime === "environment"; + let completed = false; + let completionError: unknown; let timeoutHandle: NodeJS.Timeout | undefined; const onAbort = () => { - if (child.pid) killProcessTree(child.pid); + if (child.pid) { + if (options?.owner) options.owner.terminate(child.pid); + else killProcessTree(child.pid); + } + }; + const onExit = (code: number | null) => { + completed = true; + if (environmentLifetime && timeoutHandle) clearTimeout(timeoutHandle); + try { + if (child.pid) options?.owner?.complete(child.pid, code === 0 && !timedOut && !signal?.aborted); + } catch (error) { + completionError = error; + try { + onAbort(); + } catch (cleanupError) { + completionError = new AggregateError([error, cleanupError], "Shell ownership handoff failed"); + } + } }; + child.once("exit", onExit); try { + if (child.pid) { + if (options?.owner) options.owner.track(child.pid, executionId); + else trackDetachedChildPid(child.pid); + } // Set timeout if provided. if (timeoutMs !== undefined) { timeoutHandle = setTimeout(() => { + if (environmentLifetime && completed) return; timedOut = true; - if (child.pid) killProcessTree(child.pid); + onAbort(); }, timeoutMs); } // Stream stdout and stderr. @@ -143,7 +168,10 @@ export function createLocalBashOperations(options?: { } // Handle shell spawn errors and wait for the process to terminate without hanging // on inherited stdio handles held by detached descendants. - const exitCode = await waitForChildProcess(child); + const exitCode = await waitForChildProcess(child, { + maxPostExitMs: environmentLifetime ? 100 : undefined, + }); + if (completionError) throw completionError; if (signal?.aborted) { throw new Error("aborted"); } @@ -152,6 +180,7 @@ export function createLocalBashOperations(options?: { } return { exitCode }; } finally { + child.removeListener("exit", onExit); if (child.pid) { if (options?.owner) options.owner.releaseIfExited(child.pid); else untrackDetachedChildPid(child.pid); diff --git a/packages/coding-agent/src/index.ts b/packages/coding-agent/src/index.ts index 265a14411..4faec9167 100644 --- a/packages/coding-agent/src/index.ts +++ b/packages/coding-agent/src/index.ts @@ -433,5 +433,6 @@ export { copyToClipboard } from "./utils/clipboard.ts"; export { parseFrontmatter, stripFrontmatter } from "./utils/frontmatter.ts"; export { convertToPng } from "./utils/image-convert.ts"; export { formatDimensionNote, type ResizedImage, resizeImage } from "./utils/image-resize.ts"; +export type { BackgroundProcessPolicy, ShellOwnershipRecord } from "./utils/owned-shell-processes.ts"; // Shell utilities export { getShellConfig } from "./utils/shell.ts"; diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index ec17055e4..bb7c5ccfc 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -547,6 +547,10 @@ export async function main(args: string[], options?: MainOptions) { const trustPromptMode: AppMode = parsed.help || parsed.listModels !== undefined ? "print" : appMode; const projectTrustByCwd = new Map(); + const backgroundProcessPolicy: CreateAgentSessionOptions["backgroundProcessPolicy"] = + parsed.backgroundProcessLifetime === "environment" + ? { lifetime: "environment", ownershipDirectory: resolvePath(parsed.backgroundProcessOwnershipDir!, cwd) } + : { lifetime: "session" }; const resolvedExtensionPaths = resolveCliPaths(cwd, parsed.extensions); const resolvedSkillPaths = resolveCliPaths(cwd, parsed.skills); const resolvedPromptTemplatePaths = resolveCliPaths(cwd, parsed.promptTemplates); @@ -664,6 +668,7 @@ export async function main(args: string[], options?: MainOptions) { } const created = await createAgentSessionFromServices({ + backgroundProcessPolicy, services, sessionManager, sessionStartEvent, diff --git a/packages/coding-agent/src/modes/print-mode.ts b/packages/coding-agent/src/modes/print-mode.ts index 01a2ca236..63110597e 100644 --- a/packages/coding-agent/src/modes/print-mode.ts +++ b/packages/coding-agent/src/modes/print-mode.ts @@ -36,20 +36,20 @@ export async function runPrintMode(runtimeHost: AgentSessionRuntime, options: Pr let session = runtimeHost.session; let unsubscribe: (() => void) | undefined; let unsubscribeBackpressure: (() => void) | undefined; - let disposed = false; + let disposePromise: Promise | undefined; let terminationStatus: "completed" | "failed" | "aborted" = "completed"; const signalCleanupHandlers: Array<() => void> = []; const disposeRuntime = async (status = terminationStatus): Promise => { - if (disposed) return; - disposed = true; + if (disposePromise) return disposePromise; unsubscribe?.(); unsubscribeBackpressure?.(); - await runtimeHost.dispose(status); + disposePromise = runtimeHost.dispose(status); + return disposePromise; }; const registerSignalHandlers = (): void => { - const signals: NodeJS.Signals[] = ["SIGTERM"]; + const signals: NodeJS.Signals[] = ["SIGTERM", "SIGINT"]; if (process.platform !== "win32") { signals.push("SIGHUP"); } @@ -59,7 +59,7 @@ export async function runPrintMode(runtimeHost: AgentSessionRuntime, options: Pr killTrackedDetachedChildren(); terminationStatus = "aborted"; void disposeRuntime("aborted").finally(() => { - process.exit(signal === "SIGHUP" ? 129 : 143); + process.exit(signal === "SIGHUP" ? 129 : signal === "SIGINT" ? 130 : 143); }); }; process.on(signal, handler); @@ -161,11 +161,12 @@ export async function runPrintMode(runtimeHost: AgentSessionRuntime, options: Pr console.error(error instanceof Error ? error.message : String(error)); return 1; } finally { - for (const cleanup of signalCleanupHandlers) { - cleanup(); + try { + if (exitCode !== 0) terminationStatus = "failed"; + await disposeRuntime(); + await flushRawStdout(); + } finally { + for (const cleanup of signalCleanupHandlers) cleanup(); } - if (exitCode !== 0) terminationStatus = "failed"; - await disposeRuntime(); - await flushRawStdout(); } } diff --git a/packages/coding-agent/src/utils/child-process.ts b/packages/coding-agent/src/utils/child-process.ts index 777224a4b..305aab66e 100644 --- a/packages/coding-agent/src/utils/child-process.ts +++ b/packages/coding-agent/src/utils/child-process.ts @@ -45,17 +45,21 @@ export function spawnProcessSync( * the grace timer is re-armed on every chunk, so an actively writing descendant keeps * us reading, while a quiet inherited handle (e.g. a Windows daemonized descendant * that never lets `close` fire) still releases us after the grace elapses. + * Callers handing background jobs to an environment can explicitly bound draining + * with maxPostExitMs so ongoing service output cannot block session disposal. */ -export function waitForChildProcess(child: ChildProcess): Promise { +export function waitForChildProcess(child: ChildProcess, options?: { maxPostExitMs?: number }): Promise { return new Promise((resolve, reject) => { let settled = false; let exited = false; let exitCode: number | null = null; let postExitTimer: NodeJS.Timeout | undefined; + let postExitDeadline: NodeJS.Timeout | undefined; let stdoutEnded = child.stdout === null; let stderrEnded = child.stderr === null; const cleanup = () => { + if (postExitDeadline) clearTimeout(postExitDeadline); if (postExitTimer) { clearTimeout(postExitTimer); postExitTimer = undefined; @@ -118,6 +122,9 @@ export function waitForChildProcess(child: ChildProcess): Promise exitCode = code; maybeFinalizeAfterExit(); if (!settled) { + if (options?.maxPostExitMs !== undefined) { + postExitDeadline = setTimeout(() => finalize(exitCode), options.maxPostExitMs); + } armIdleTimer(); } }; diff --git a/packages/coding-agent/src/utils/owned-shell-processes.ts b/packages/coding-agent/src/utils/owned-shell-processes.ts index 61419dde9..fadb3af52 100644 --- a/packages/coding-agent/src/utils/owned-shell-processes.ts +++ b/packages/coding-agent/src/utils/owned-shell-processes.ts @@ -1,63 +1,189 @@ -import { spawnSync } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { closeSync, fsyncSync, mkdirSync, openSync, renameSync, unlinkSync, writeFileSync } from "node:fs"; +import { isAbsolute, join } from "node:path"; import { setTimeout as delay } from "node:timers/promises"; import { killProcessTree, trackDetachedChildPid, untrackDetachedChildPid } from "./shell.ts"; +import { hasShellExecutionToken, readShellProcessGroup, type ShellProcessIdentity } from "./shell-process-identity.ts"; -function isAlive(pid: number): boolean { - try { - process.kill(process.platform === "win32" ? pid : -pid, 0); - return true; - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ESRCH") return false; - if ((error as NodeJS.ErrnoException).code === "EPERM") return true; - throw error; - } +export type BackgroundProcessPolicy = + | { readonly lifetime: "session" } + | { readonly lifetime: "environment"; readonly ownershipDirectory: string }; + +export interface ShellOwnershipRecord { + readonly version: 1; + readonly executionId: string; + readonly owner: "session" | "environment"; + readonly state: "active" | "background" | "terminated"; + readonly processGroupId: number; + readonly supervisorPid: number; + readonly members: readonly ShellProcessIdentity[]; +} + +interface Execution { + readonly id: string; + state: "active" | "background" | "terminating"; + members: readonly ShellProcessIdentity[]; } -/** A session owns the process groups it launches, including background jobs. */ +/** Session ownership ends only after a successful, durably published handoff. */ export class OwnedShellProcesses { - readonly #pids = new Set(); + readonly policy: BackgroundProcessPolicy; + readonly #executions = new Map(); - track(pid: number): void { - this.#pids.add(pid); - trackDetachedChildPid(pid); + constructor(policy: BackgroundProcessPolicy = { lifetime: "session" }) { + if (policy.lifetime !== "session" && policy.lifetime !== "environment") { + throw new Error("Invalid background process lifetime"); + } + if (policy.lifetime === "environment") { + if (process.platform !== "linux" && process.platform !== "darwin") { + throw new Error("Environment background process ownership requires Linux or macOS"); + } + if (!policy.ownershipDirectory || !isAbsolute(policy.ownershipDirectory)) { + throw new Error("Environment background process ownership requires an absolute ownershipDirectory"); + } + mkdirSync(policy.ownershipDirectory, { recursive: true, mode: 0o700 }); + } + this.policy = Object.freeze({ ...policy }); + } + + track(pid: number, executionId: string): void { + const execution: Execution = { id: executionId, state: "active", members: [] }; + this.#executions.set(pid, execution); + trackDetachedChildPid(pid, () => this.terminate(pid)); + try { + execution.members = process.platform === "win32" ? [] : readShellProcessGroup(pid); + this.#publish(pid, execution, "session", "active"); + } catch (error) { + this.terminate(pid); + throw error; + } + } + + /** Called synchronously on shell exit, before output draining or another event can cancel it. */ + complete(pid: number, successful: boolean): void { + const execution = this.#executions.get(pid); + if (!execution || execution.state !== "active") return; + // This callback belongs to the ChildProcess just reaped by Node. Capture the + // surviving group now; later cleanup must validate these process identities. + execution.members = process.platform === "win32" ? [] : readShellProcessGroup(pid); + execution.state = "background"; + if (!successful || this.policy.lifetime === "session") return; + const members = this.#members(pid, execution); + if (members.length === 0) { + this.releaseIfExited(pid); + return; + } + execution.members = members; + // No await between publication and removing both cleanup registrations. + // Atomic rename is the ownership commit point, including after a forced CLI death. + this.#publish(pid, execution, "environment", "background"); + this.#executions.delete(pid); + untrackDetachedChildPid(pid); + } + + #publish( + pid: number, + execution: Execution, + owner: ShellOwnershipRecord["owner"], + state: ShellOwnershipRecord["state"], + ): void { + if (this.policy.lifetime !== "environment") return; + const record: ShellOwnershipRecord = { + version: 1, + executionId: execution.id, + owner, + state, + processGroupId: pid, + supervisorPid: process.pid, + members: execution.members, + }; + const path = join(this.policy.ownershipDirectory, `${execution.id}.json`); + const temporary = `${path}.${randomUUID()}.tmp`; + const fd = openSync(temporary, "wx", 0o600); + try { + try { + writeFileSync(fd, `${JSON.stringify(record)}\n`); + fsyncSync(fd); + } finally { + closeSync(fd); + } + renameSync(temporary, path); + } finally { + try { + unlinkSync(temporary); + } catch {} + } + } + + #members(pid: number, execution: Execution): ShellProcessIdentity[] { + const members = readShellProcessGroup(pid); + if (members.length === 0) return []; + const known = members.some((member) => + execution.members.some((previous) => previous.pid === member.pid && previous.startTime === member.startTime), + ); + if (!known && !members.some((member) => hasShellExecutionToken(member.pid, execution.id))) { + throw new Error(`Cannot verify ownership of shell process group ${pid}`); + } + return members; + } + + terminate(pid: number): void { + const execution = this.#executions.get(pid); + if (!execution) return; + if (execution.state === "active" && process.platform !== "win32") { + // The launching ChildProcess has not delivered its exit event yet. + execution.members = readShellProcessGroup(pid); + } + execution.state = "terminating"; + if (process.platform === "win32") { + killProcessTree(pid); + } else if (this.#members(pid, execution).length > 0) { + // Never fall back to a recycled leader PID when its group has disappeared. + try { + process.kill(-pid, "SIGKILL"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; + } + } } releaseIfExited(pid: number): void { - if (!isAlive(pid)) { - this.#pids.delete(pid); + const execution = this.#executions.get(pid); + if (!execution) return; + let alive: boolean; + if (process.platform === "win32") { + try { + process.kill(pid, 0); + alive = true; + } catch { + alive = false; + } + } else { + alive = this.#members(pid, execution).length > 0; + } + if (!alive) { + this.#publish(pid, execution, "session", "terminated"); + this.#executions.delete(pid); untrackDetachedChildPid(pid); } } get count(): number { - for (const pid of this.#pids) this.releaseIfExited(pid); - return this.#pids.size; + for (const pid of this.#executions.keys()) this.releaseIfExited(pid); + return this.#executions.size; } async terminateAll(): Promise { - const pids = [...this.#pids]; - for (const pid of pids) killProcessTree(pid); + const pids = [...this.#executions.keys()]; + for (const pid of pids) this.terminate(pid); const deadline = performance.now() + 5_000; for (const pid of pids) { - while (isAlive(pid)) { - // Linux PID 1 may leave orphaned zombies unreaped. Zombies are terminated, - // but kill(0) still sees their process group until the parent reaps them. - if (process.platform !== "win32") { - const result = spawnSync("ps", ["-eo", "pgid=,stat="], { encoding: "utf8", timeout: 1_000 }); - if ( - result.status === 0 && - !result.stdout.split("\n").some((line) => { - const [group, state] = line.trim().split(/\s+/); - return Number(group) === pid && state && !state.startsWith("Z"); - }) - ) - break; - } + while (this.#executions.has(pid)) { + this.releaseIfExited(pid); + if (!this.#executions.has(pid)) break; if (performance.now() >= deadline) throw new Error(`Shell process group ${pid} did not terminate`); await delay(25); } - this.#pids.delete(pid); - untrackDetachedChildPid(pid); } } } diff --git a/packages/coding-agent/src/utils/shell-process-identity.ts b/packages/coding-agent/src/utils/shell-process-identity.ts new file mode 100644 index 000000000..b1915b901 --- /dev/null +++ b/packages/coding-agent/src/utils/shell-process-identity.ts @@ -0,0 +1,65 @@ +import { spawnSync } from "node:child_process"; +import { readdirSync, readFileSync } from "node:fs"; + +export const SHELL_EXECUTION_ID_ENV = "DIFFEX_SHELL_EXECUTION_ID"; + +export interface ShellProcessIdentity { + readonly pid: number; + readonly processGroupId: number; + /** Linux boot ID and start ticks, or macOS ps lstart in UTC and the C locale. */ + readonly startTime: string; +} + +/** Read live group members without treating orphaned zombies as running work. */ +export function readShellProcessGroup(processGroupId: number): ShellProcessIdentity[] { + if (process.platform === "linux") { + const bootId = readFileSync("/proc/sys/kernel/random/boot_id", "utf8").trim(); + const members: ShellProcessIdentity[] = []; + for (const name of readdirSync("/proc")) { + if (!/^\d+$/.test(name)) continue; + try { + const stat = readFileSync(`/proc/${name}/stat`, "utf8"); + const fields = stat + .slice(stat.lastIndexOf(")") + 2) + .trim() + .split(/\s+/); + if (Number(fields[2]) !== processGroupId || fields[0] === "Z") continue; + members.push({ pid: Number(name), processGroupId, startTime: `${bootId}:${fields[19]}` }); + } catch (error) { + if (["ENOENT", "ESRCH"].includes((error as NodeJS.ErrnoException).code ?? "")) continue; + throw error; + } + } + return members; + } + const result = spawnSync("ps", ["-axo", "pid=,pgid=,stat=,lstart="], { + encoding: "utf8", + timeout: 1_000, + env: { ...process.env, LC_ALL: "C", TZ: "UTC" }, + }); + if (result.status !== 0) throw new Error("Cannot inspect shell process groups", { cause: result.error }); + return result.stdout.split("\n").flatMap((line) => { + const [pid, group, state, ...started] = line.trim().split(/\s+/); + return Number(group) === processGroupId && state && !state.startsWith("Z") + ? [{ pid: Number(pid), processGroupId, startTime: started.join(" ") }] + : []; + }); +} + +/** The launch token prevents a recycled group ID from authorizing a signal. */ +export function hasShellExecutionToken(pid: number, executionId: string): boolean { + try { + const token = `${SHELL_EXECUTION_ID_ENV}=${executionId}`; + if (process.platform === "linux") { + return readFileSync(`/proc/${pid}/environ`, "utf8").split("\0").includes(token); + } + const result = spawnSync("ps", ["-Eww", "-p", String(pid), "-o", "command="], { + encoding: "utf8", + timeout: 1_000, + maxBuffer: 4 * 1024 * 1024, + }); + return result.status === 0 && result.stdout.split(/\s+/).includes(token); + } catch { + return false; + } +} diff --git a/packages/coding-agent/src/utils/shell.ts b/packages/coding-agent/src/utils/shell.ts index 2cafa595e..978798094 100644 --- a/packages/coding-agent/src/utils/shell.ts +++ b/packages/coding-agent/src/utils/shell.ts @@ -177,10 +177,10 @@ export function sanitizeBinaryOutput(str: string): string { * Detached child processes must be tracked so they can be killed on parent * shutdown signals (SIGHUP/SIGTERM). */ -const trackedDetachedChildPids = new Set(); +const trackedDetachedChildPids = new Map void>(); -export function trackDetachedChildPid(pid: number): void { - trackedDetachedChildPids.add(pid); +export function trackDetachedChildPid(pid: number, terminate = () => killProcessTree(pid)): void { + trackedDetachedChildPids.set(pid, terminate); } export function untrackDetachedChildPid(pid: number): void { @@ -188,8 +188,8 @@ export function untrackDetachedChildPid(pid: number): void { } export function killTrackedDetachedChildren(): void { - for (const pid of trackedDetachedChildPids) { - killProcessTree(pid); + for (const terminate of trackedDetachedChildPids.values()) { + terminate(); } trackedDetachedChildPids.clear(); } diff --git a/packages/coding-agent/test/background-process-cli.test.ts b/packages/coding-agent/test/background-process-cli.test.ts new file mode 100644 index 000000000..b1c86fae8 --- /dev/null +++ b/packages/coding-agent/test/background-process-cli.test.ts @@ -0,0 +1,241 @@ +import { type ChildProcess, spawn } from "node:child_process"; +import { once } from "node:events"; +import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { createServer } from "node:http"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { describe, expect, it, vi } from "vitest"; +import { parseArgs } from "../src/cli/args.ts"; +import type { ShellOwnershipRecord } from "../src/utils/owned-shell-processes.ts"; +import { readShellProcessGroup } from "../src/utils/shell-process-identity.ts"; +import { createBackgroundService, queryBackgroundService } from "./background-service.ts"; + +describe("background lifetime CLI validation", () => { + it("requires an explicit policy and handoff destination", () => { + expect(parseArgs(["--background-process-lifetime=environment"]).diagnostics).not.toEqual([]); + expect(parseArgs(["--background-process-lifetime", "invalid"]).diagnostics).not.toEqual([]); + expect(parseArgs(["--background-process-ownership-dir", "/tmp/ownership"]).diagnostics).not.toEqual([]); + expect( + parseArgs(["--background-process-lifetime=environment", "--background-process-ownership-dir=/tmp/ownership"]) + .diagnostics, + ).toEqual([]); + expect(parseArgs(["--background-process-lifetime", "session"]).diagnostics).toEqual([]); + }); +}); + +describe.skipIf(process.platform !== "linux" && process.platform !== "darwin").each(["text", "json"] as const)( + "%s CLI background services", + (mode) => { + it.each([ + "default", + "success", + "provider-error", + "SIGTERM", + "SIGHUP", + "SIGINT", + "SIGKILL", + "active-SIGTERM", + "noisy", + ])( + "handles %s", + async (scenario) => { + const directory = mkdtempSync(join(tmpdir(), "diffex-background-cli-")); + const ownershipDirectory = join(directory, "ownership"); + const service = createBackgroundService(directory, { noisy: scenario === "noisy" }); + let cli: ChildProcess | undefined; + let requests = 0; + let reachableBeforeFinal = false; + let observedRecords: ShellOwnershipRecord[] = []; + let providerError: unknown; + const server = createServer(async (_request, response) => { + requests++; + try { + let delta: Record; + let finishReason = "stop"; + if (requests === 1 || (requests === 2 && scenario === "active-SIGTERM")) { + const command = + requests === 1 + ? service.command + : "echo $$ > active.pgid; sleep 60 & echo $! > active.pid; wait"; + delta = { + role: "assistant", + tool_calls: [ + { + index: 0, + id: `launch-${requests}`, + type: "function", + function: { name: "bash", arguments: JSON.stringify({ command }) }, + }, + ], + }; + finishReason = "tool_calls"; + } else { + reachableBeforeFinal = (await queryBackgroundService(service.address().port)) === "service-ok"; + if (scenario !== "default") { + observedRecords = readdirSync(ownershipDirectory) + .filter((file) => file.endsWith(".json")) + .map((file) => JSON.parse(readFileSync(join(ownershipDirectory, file), "utf8"))); + } + if (scenario === "provider-error") { + response.writeHead(401, { "Content-Type": "application/json" }); + response.end(JSON.stringify({ error: { message: "Controlled provider failure" } })); + return; + } + if (scenario.startsWith("SIG")) { + cli!.kill(scenario as NodeJS.Signals); + return; + } + delta = { role: "assistant", content: "service-ready" }; + } + response.writeHead(200, { "Content-Type": "text/event-stream" }); + response.write( + `data: ${JSON.stringify({ id: "fixture", object: "chat.completion.chunk", choices: [{ index: 0, delta, finish_reason: null }] })}\n\n`, + ); + response.write( + `data: ${JSON.stringify({ id: "fixture", object: "chat.completion.chunk", choices: [{ index: 0, delta: {}, finish_reason: finishReason }] })}\n\n`, + ); + response.end("data: [DONE]\n\n"); + } catch (error) { + providerError = error; + response.writeHead(500); + response.end(); + } + }); + try { + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("Missing mock provider address"); + writeFileSync( + join(directory, "settings.json"), + JSON.stringify({ evolution: { enabled: false }, retry: { enabled: false } }), + ); + writeFileSync( + join(directory, "models.json"), + JSON.stringify({ + providers: { + fixture: { + baseUrl: `http://127.0.0.1:${address.port}/v1`, + api: "openai-completions", + apiKey: "fixture-only", + models: [ + { + id: "controlled", + name: "controlled", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 8192, + maxTokens: 1024, + }, + ], + }, + }, + }), + ); + cli = spawn( + process.execPath, + [ + `--import=${import.meta.resolve("tsx")}`, + resolve(__dirname, "../src/cli.ts"), + "--mode", + mode, + "--provider", + "fixture", + "--model", + "controlled", + "--thinking", + "off", + "--session", + join(directory, "root.jsonl"), + "--no-extensions", + "--no-skills", + "--no-context-files", + "--no-prompt-templates", + "--no-themes", + "--no-approve", + ...(scenario === "default" + ? [] + : [ + "--background-process-lifetime=environment", + "--background-process-ownership-dir", + ownershipDirectory, + ]), + "--print", + "Launch the service", + ], + { + cwd: directory, + env: { + PATH: process.env.PATH, + HOME: directory, + DIFFEX_CODING_AGENT_DIR: directory, + DIFFEX_OFFLINE: "1", + TSX_TSCONFIG_PATH: resolve(__dirname, "../../../tsconfig.json"), + }, + stdio: ["ignore", "pipe", "pipe"], + timeout: 15_000, + }, + ); + let stdout = ""; + let stderr = ""; + cli.stdout!.on("data", (chunk) => { + stdout += chunk.toString(); + }); + cli.stderr!.on("data", (chunk) => { + stderr += chunk.toString(); + }); + const closed = once(cli, "close"); + if (scenario === "active-SIGTERM") { + await vi.waitFor( + () => expect(Number(readFileSync(join(directory, "active.pid"), "utf8"))).toBeGreaterThan(0), + { timeout: 10_000 }, + ); + reachableBeforeFinal = (await queryBackgroundService(service.address().port)) === "service-ok"; + cli.kill("SIGTERM"); + } + const [code, signal] = await closed; + expect(providerError).toBeUndefined(); + expect(reachableBeforeFinal).toBe(true); + if (scenario === "SIGKILL") expect(signal).toBe("SIGKILL"); + else if (scenario.startsWith("SIG") || scenario === "active-SIGTERM") { + expect(code, stderr).toBe(scenario === "SIGHUP" ? 129 : scenario === "SIGINT" ? 130 : 143); + } else { + expect(code, stderr).toBe(scenario === "provider-error" ? 1 : 0); + if (scenario !== "provider-error") expect(stdout).toContain("service-ready"); + expect(readFileSync(join(directory, "root.jsonl"), "utf8")).toContain('"role":"assistant"'); + } + if (scenario === "default") { + await expect(queryBackgroundService(service.address().port)).rejects.toThrow(); + } else { + await expect(queryBackgroundService(service.address().port)).resolves.toBe("service-ok"); + if (scenario !== "active-SIGTERM") { + expect(observedRecords).toHaveLength(1); + expect(observedRecords[0]).toMatchObject({ owner: "environment", state: "background" }); + expect(observedRecords[0].members.some((member) => member.pid === service.address().pid)).toBe( + true, + ); + expect(readShellProcessGroup(observedRecords[0].processGroupId)).toEqual( + observedRecords[0].members, + ); + } else { + expect( + readShellProcessGroup(Number(readFileSync(join(directory, "active.pgid"), "utf8"))), + ).toEqual([]); + } + } + } finally { + if (cli && cli.exitCode === null && cli.signalCode === null) { + cli.kill("SIGKILL"); + await once(cli, "close"); + } + service.cleanup(); + server.closeAllConnections(); + await new Promise((done) => server.close(() => done())); + rmSync(directory, { recursive: true, force: true }); + } + }, + 20_000, + ); + }, +); diff --git a/packages/coding-agent/test/background-service.ts b/packages/coding-agent/test/background-service.ts new file mode 100644 index 000000000..b0dff7c0b --- /dev/null +++ b/packages/coding-agent/test/background-service.ts @@ -0,0 +1,51 @@ +import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { connect } from "node:net"; +import { join } from "node:path"; + +export interface ServiceAddress { + pid: number; + port: number; +} + +export function createBackgroundService(directory: string, options?: { noisy?: boolean }) { + const source = join(directory, "service.cjs"); + const addressFile = join(directory, "service.json"); + writeFileSync( + source, + ` +const fs = require("node:fs"); +const server = require("node:net").createServer(socket => socket.end("service-ok")); +process.stdout.on("error", () => {}); +server.listen(0, "127.0.0.1", () => fs.writeFileSync(${JSON.stringify(addressFile)}, JSON.stringify({ pid: process.pid, port: server.address().port }))); +${options?.noisy ? 'setInterval(() => process.stdout.write("background output\\n"), 10);' : ""} +`, + ); + const quote = (value: string) => `'${value.replace(/'/g, `'"'"'`)}'`; + return { + command: `nohup ${quote(process.execPath)} ${quote(source)} ${options?.noisy ? "" : ">/dev/null 2>&1"} & while [ ! -f ${quote(addressFile)} ]; do sleep 0.01; done`, + address(): ServiceAddress { + return JSON.parse(readFileSync(addressFile, "utf8")); + }, + cleanup() { + if (!existsSync(addressFile)) return; + const address: ServiceAddress = JSON.parse(readFileSync(addressFile, "utf8")); + try { + process.kill(address.pid, "SIGKILL"); + } catch {} + }, + }; +} + +export function queryBackgroundService(port: number): Promise { + return new Promise((resolve, reject) => { + const socket = connect(port, "127.0.0.1"); + let response = ""; + socket.setEncoding("utf8"); + socket.setTimeout(2_000, () => socket.destroy(new Error("Service query timed out"))); + socket.on("data", (chunk) => { + response += chunk; + }); + socket.on("end", () => resolve(response)); + socket.on("error", reject); + }); +} diff --git a/packages/coding-agent/test/shell-ownership.test.ts b/packages/coding-agent/test/shell-ownership.test.ts new file mode 100644 index 000000000..64f347d9c --- /dev/null +++ b/packages/coding-agent/test/shell-ownership.test.ts @@ -0,0 +1,159 @@ +import { randomUUID } from "node:crypto"; +import { mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it, vi } from "vitest"; +import { createLocalBashOperations } from "../src/core/tools/bash.ts"; +import { OwnedShellProcesses, type ShellOwnershipRecord } from "../src/utils/owned-shell-processes.ts"; +import { killTrackedDetachedChildren } from "../src/utils/shell.ts"; +import * as identity from "../src/utils/shell-process-identity.ts"; + +describe.skipIf(process.platform !== "linux" && process.platform !== "darwin")("shell ownership handoff", () => { + it.each(["session", "environment"] as const)("keeps %s cleanup semantics", async (lifetime) => { + const directory = mkdtempSync(join(tmpdir(), "diffex-shell-ownership-")); + const owner = new OwnedShellProcesses( + lifetime === "session" ? { lifetime } : { lifetime, ownershipDirectory: directory }, + ); + let pid: number | undefined; + try { + await createLocalBashOperations({ owner }).exec( + "echo $$ > group.pid; sleep 60 >/dev/null 2>&1 & echo $! > child.pid", + directory, + { + onData() {}, + }, + ); + pid = Number(readFileSync(join(directory, "child.pid"), "utf8")); + expect(owner.count).toBe(lifetime === "environment" ? 0 : 1); + if (lifetime === "environment") { + const records = readdirSync(directory).filter((path) => path.endsWith(".json")); + expect(records).toHaveLength(1); + const record: ShellOwnershipRecord = JSON.parse(readFileSync(join(directory, records[0]), "utf8")); + expect(record).toMatchObject({ version: 1, owner: "environment", state: "background" }); + expect(record.members.some((member) => member.pid === pid && member.startTime.length > 0)).toBe(true); + } + killTrackedDetachedChildren(); + await owner.terminateAll(); + await owner.terminateAll(); + if (lifetime === "environment") expect(() => process.kill(pid!, 0)).not.toThrow(); + else { + const group = Number(readFileSync(join(directory, "group.pid"), "utf8")); + await vi.waitFor(() => expect(identity.readShellProcessGroup(group)).toHaveLength(0)); + } + } finally { + if (pid) { + try { + process.kill(pid, "SIGKILL"); + } catch {} + } + await owner.terminateAll(); + rmSync(directory, { recursive: true, force: true }); + } + }); + + it.each(["failure", "timeout", "abort"])("retains ownership after %s", async (scenario) => { + const directory = mkdtempSync(join(tmpdir(), "diffex-shell-cancel-")); + const owner = new OwnedShellProcesses({ lifetime: "environment", ownershipDirectory: directory }); + const controller = new AbortController(); + let pid: number | undefined; + try { + const result = createLocalBashOperations({ owner }).exec( + `sleep 60 >/dev/null 2>&1 & echo $! > child.pid; echo ready; ${scenario === "failure" ? "exit 1" : "wait"}`, + directory, + { + onData() { + if (scenario === "abort") controller.abort(); + }, + signal: controller.signal, + timeout: scenario === "timeout" ? 0.2 : undefined, + }, + ); + if (scenario === "failure") expect((await result).exitCode).toBe(1); + else await expect(result).rejects.toThrow(scenario === "abort" ? "aborted" : "timeout"); + pid = Number(readFileSync(join(directory, "child.pid"), "utf8")); + const records = readdirSync(directory).filter((path) => path.endsWith(".json")); + expect(records).toHaveLength(1); + expect(JSON.parse(readFileSync(join(directory, records[0]), "utf8")).owner).toBe("session"); + await owner.terminateAll(); + expect(owner.count).toBe(0); + } finally { + controller.abort(); + if (pid) { + try { + process.kill(pid, "SIGKILL"); + } catch {} + } + await owner.terminateAll(); + rmSync(directory, { recursive: true, force: true }); + } + }); + + it.each(["cancellation-first", "handoff-first", "publication-failure"])( + "retains exactly one owner when %s", + async (scenario) => { + const directory = mkdtempSync(join(tmpdir(), "diffex-shell-race-")); + const owner = new OwnedShellProcesses({ lifetime: "environment", ownershipDirectory: directory }); + const executionId = randomUUID(); + const path = join(directory, `${executionId}.json`); + const pid = 123457; + const read = vi + .spyOn(identity, "readShellProcessGroup") + .mockReturnValue([{ pid, processGroupId: pid, startTime: "original" }]); + const kill = vi.spyOn(process, "kill").mockReturnValue(true); + try { + owner.track(pid, executionId); + if (scenario === "cancellation-first") owner.terminate(pid); + if (scenario === "publication-failure") { + rmSync(path); + mkdirSync(path); + expect(() => owner.complete(pid, true)).toThrow(); + expect(readdirSync(directory).some((name) => name.endsWith(".tmp"))).toBe(false); + rmSync(path, { recursive: true }); + } else { + owner.complete(pid, true); + owner.complete(pid, true); + expect(JSON.parse(readFileSync(path, "utf8")).owner).toBe( + scenario === "handoff-first" ? "environment" : "session", + ); + } + owner.terminate(pid); + killTrackedDetachedChildren(); + if (scenario === "handoff-first") { + expect(owner.count).toBe(0); + expect(kill).not.toHaveBeenCalled(); + } else { + expect(owner.count).toBe(1); + expect(kill).toHaveBeenCalledWith(-pid, "SIGKILL"); + } + } finally { + read.mockReturnValue([]); + await owner.terminateAll(); + read.mockRestore(); + kill.mockRestore(); + rmSync(directory, { recursive: true, force: true }); + } + }, + ); + + it("refuses to signal a recycled process group", async () => { + const owner = new OwnedShellProcesses(); + const read = vi + .spyOn(identity, "readShellProcessGroup") + .mockReturnValue([{ pid: 123456, processGroupId: 123456, startTime: "old" }]); + const token = vi.spyOn(identity, "hasShellExecutionToken").mockReturnValue(false); + const kill = vi.spyOn(process, "kill"); + try { + owner.track(123456, randomUUID()); + owner.complete(123456, true); + read.mockReturnValue([{ pid: 123456, processGroupId: 123456, startTime: "new" }]); + expect(() => owner.terminate(123456)).toThrow("Cannot verify ownership"); + expect(kill).not.toHaveBeenCalled(); + } finally { + read.mockReturnValue([]); + await owner.terminateAll(); + read.mockRestore(); + token.mockRestore(); + kill.mockRestore(); + } + }); +}); diff --git a/packages/coding-agent/test/suite/harness.ts b/packages/coding-agent/test/suite/harness.ts index f1f2fd71a..f7e346198 100644 --- a/packages/coding-agent/test/suite/harness.ts +++ b/packages/coding-agent/test/suite/harness.ts @@ -26,6 +26,7 @@ import { SessionManager } from "../../src/core/session-manager.ts"; import type { Settings } from "../../src/core/settings-manager.ts"; import { SettingsManager } from "../../src/core/settings-manager.ts"; import type { InlineExtension, ResourceLoader } from "../../src/index.ts"; +import type { BackgroundProcessPolicy } from "../../src/utils/owned-shell-processes.ts"; import { type CreateTestExtensionsResultInput, createTestExtensionsResult, @@ -64,6 +65,7 @@ export function getAssistantTexts(harness: Harness): string[] { } export interface HarnessOptions { + backgroundProcessPolicy?: BackgroundProcessPolicy; models?: FauxModelDefinition[]; settings?: Partial; systemPrompt?: string; @@ -191,6 +193,7 @@ export async function createHarness(options: HarnessOptions = {}): Promise { + it("preserves direct session bash services across repeated disposal", async () => { + const directory = mkdtempSync(join(tmpdir(), "diffex-direct-service-")); + const service = createBackgroundService(directory, { noisy: true }); + const harness = await createHarness({ + backgroundProcessPolicy: { lifetime: "environment", ownershipDirectory: join(directory, "ownership") }, + }); + try { + expect((await harness.session.executeBash(service.command)).exitCode).toBe(0); + expect(harness.session.ownedShellProcessCount).toBe(0); + await harness.session.abort(); + await harness.session.dispose(); + await harness.session.dispose(); + await expect(queryBackgroundService(service.address().port)).resolves.toBe("service-ok"); + } finally { + service.cleanup(); + await harness.cleanup(); + rmSync(directory, { recursive: true, force: true }); + } + }); + + it.each(["success", "failure", "deadline"])("inherits environment ownership through child %s", async (outcome) => { + const directory = mkdtempSync(join(tmpdir(), "diffex-child-service-")); + const ownershipDirectory = join(directory, "ownership"); + const service = createBackgroundService(directory); + const fixture = await createSubagentTestHarness(1, { + sessionManager: SessionManager.inMemory(directory), + backgroundProcessPolicy: { lifetime: "environment", ownershipDirectory }, + }); + const waiting = deferred(); + let statusAtFailure: string | undefined; + try { + fixture.harness.setResponses([ + fauxAssistantMessage(fauxToolCall("bash", { command: service.command }), { stopReason: "toolUse" }), + async (_context, options) => { + await expect(queryBackgroundService(service.address().port)).resolves.toBe("service-ok"); + if (outcome === "deadline") { + const cancelled = deferred(); + options?.signal?.addEventListener("abort", () => cancelled.resolve(), { once: true }); + waiting.resolve(); + await cancelled.promise; + return fauxAssistantMessage("deadline", { stopReason: "aborted" }); + } + return fauxAssistantMessage(outcome, { + stopReason: outcome === "failure" ? "error" : "stop", + errorMessage: outcome === "failure" ? "Controlled failure" : undefined, + }); + }, + ]); + if (outcome === "deadline") vi.useFakeTimers(); + fixture.controller.spawn("worker", "Launch the service", fixture.createSession); + if (outcome === "deadline") { + await waiting.promise; + await vi.advanceTimersByTimeAsync(15 * 60_000); + vi.useRealTimers(); + } + const expected = outcome === "success" ? "idle" : outcome === "failure" ? "failed" : "interrupted"; + await waitForStatus(fixture.controller, "worker", expected); + statusAtFailure = fixture.controller.snapshot().agents[0]?.status; + const records: ShellOwnershipRecord[] = readdirSync(ownershipDirectory) + .filter((file) => file.endsWith(".json")) + .map((file) => JSON.parse(readFileSync(join(ownershipDirectory, file), "utf8"))); + expect(records).toHaveLength(1); + expect(records[0]).toMatchObject({ owner: "environment", state: "background" }); + await expect(queryBackgroundService(service.address().port)).resolves.toBe("service-ok"); + await fixture.controller.shutdown(); + await fixture.harness.session.dispose(); + await expect(queryBackgroundService(service.address().port)).resolves.toBe("service-ok"); + expect(statusAtFailure).toBe(expected); + } finally { + vi.useRealTimers(); + await fixture.cleanup(); + service.cleanup(); + rmSync(directory, { recursive: true, force: true }); + } + }); + it("preserves a service across runtime replacement and invalidates the old session", async () => { + const directory = mkdtempSync(join(tmpdir(), "diffex-replace-service-")); + const policy = { lifetime: "environment", ownershipDirectory: join(directory, "ownership") } as const; + const service = createBackgroundService(directory); + const harness = await createHarness(); + const services = await createAgentSessionServices({ + cwd: directory, + agentDir: directory, + modelRuntime: harness.session.modelRuntime, + settingsManager: harness.settingsManager, + resourceLoaderOptions: { + noExtensions: true, + noSkills: true, + noThemes: true, + noPromptTemplates: true, + noContextFiles: true, + }, + }); + const runtime = await createAgentSessionRuntime( + async ({ sessionManager, sessionStartEvent }) => ({ + ...(await createAgentSessionFromServices({ + services, + sessionManager, + sessionStartEvent, + model: harness.getModel(), + backgroundProcessPolicy: policy, + })), + services, + diagnostics: [], + }), + { cwd: directory, agentDir: directory, sessionManager: SessionManager.inMemory(directory) }, + ); + try { + expect((await runtime.session.executeBash(service.command)).exitCode).toBe(0); + const oldSession = runtime.session; + await runtime.newSession(); + expect(runtime.session).not.toBe(oldSession); + expect(runtime.session.backgroundProcessPolicy).toEqual(policy); + await expect(oldSession.executeBash("echo stale")).rejects.toThrow("disposed"); + await expect(queryBackgroundService(service.address().port)).resolves.toBe("service-ok"); + await runtime.dispose(); + await runtime.dispose(); + await expect(queryBackgroundService(service.address().port)).resolves.toBe("service-ok"); + } finally { + service.cleanup(); + await runtime.dispose(); + await harness.cleanup(); + rmSync(directory, { recursive: true, force: true }); + } + }); +});