Skip to content

Commit 893555c

Browse files
committed
Centralize agent task cleanup into one signal-safe coordinator
The executor previously carried four independent cleanup sites for the private runtime source root and the runner workspace seed snapshot: mid-run awaits, an inline catch-path duplicate, a partial exit hook, and signal handlers that cleaned only one of the two resources and exited 128 for every signal. Replace them with a single idempotent module-level coordinator that claims both roots exactly once and chains repeat invocations onto the in-flight cleanup instead of racing it. The top-level normal and failure paths route through one finally; SIGINT/SIGTERM/SIGHUP await the same coordinator before exiting with the conventional 128+signum code; the process exit hook keeps a bounded synchronous best-effort fallback for abrupt exits. Error reporting and redaction are preserved, and stderr messages are now redacted while the private root is still known. A deterministic interruption test starts the executor in a child process, pauses it immediately after seed snapshot creation through a narrowly scoped NODE_ENV=test marker-file hook that is inert in production, delivers each signal, and asserts the temp seed snapshot directory is removed, the conventional exit code is set, and no runner workspace content survives. A normal run in the same harness proves routine cleanup still passes.
1 parent 22aceec commit 893555c

5 files changed

Lines changed: 170 additions & 35 deletions

File tree

.github/scripts/run-agent-task/execute-native-agent-task.mjs

Lines changed: 50 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,41 @@ const secretValues = ["OPENAI_API_KEY", "MODEL_PROVIDER_SECRET_1", "MODEL_PROVID
2020
let privateRuntimeSourceRoot = ""
2121
let privateRuntimeSourceRootForSanitization = ""
2222
let runnerWorkspaceSeedSnapshot
23+
24+
const SIGNAL_EXIT_CODES = { SIGINT: 130, SIGTERM: 143, SIGHUP: 129 }
25+
let materializedSourceCleanup
26+
function claimMaterializedSourcePaths() {
27+
const paths = []
28+
if (runnerWorkspaceSeedSnapshot) {
29+
paths.push(runnerWorkspaceSeedSnapshot.source)
30+
runnerWorkspaceSeedSnapshot = undefined
31+
}
32+
if (privateRuntimeSourceRoot) {
33+
paths.push(privateRuntimeSourceRoot)
34+
privateRuntimeSourceRoot = ""
35+
}
36+
return paths
37+
}
38+
// Single idempotent cleanup coordinator for the private runtime source
39+
// materialization root and the runner workspace seed snapshot. Every
40+
// completion path (normal, failure, signal) awaits this coordinator; repeat
41+
// invocations chain onto the in-flight cleanup instead of racing it.
42+
function cleanupMaterializedSources() {
43+
materializedSourceCleanup = (materializedSourceCleanup ?? Promise.resolve())
44+
.then(() => Promise.all(claimMaterializedSourcePaths().map((path) => rm(path, { recursive: true, force: true }))))
45+
return materializedSourceCleanup
46+
}
47+
process.once("exit", () => {
48+
// Bounded synchronous best-effort fallback for abrupt exits; on every
49+
// awaited path the coordinator has already claimed these roots.
50+
for (const path of claimMaterializedSourcePaths()) {
51+
try { rmSync(path, { recursive: true, force: true, maxRetries: 0 }) } catch { /* best effort */ }
52+
}
53+
})
54+
for (const signal of ["SIGINT", "SIGTERM", "SIGHUP"]) {
55+
process.once(signal, () => { cleanupMaterializedSources().finally(() => process.exit(SIGNAL_EXIT_CODES[signal])) })
56+
}
57+
2358
function redact(value) {
2459
if (typeof value === "string") return secretValues.reduce((output, secret) => output.split(secret).join("[REDACTED]"), value)
2560
if (Array.isArray(value)) return value.map(redact)
@@ -220,6 +255,17 @@ async function testRuntimeFixtures(externalPackageSource) {
220255
return fixtures
221256
}
222257

258+
// Test-only interruption hook: inert in production (requires NODE_ENV=test and
259+
// an explicit marker path). Publishes the seed snapshot location, then holds a
260+
// bounded window so a harness can deliver a termination signal.
261+
async function testPauseAfterSeedSnapshot(seedSnapshot) {
262+
if (process.env.NODE_ENV !== "test") return
263+
const markerPath = string(process.env.WP_CODEBOX_TEST_SEED_SNAPSHOT_PAUSE_FILE)
264+
if (!markerPath) return
265+
await writeFile(markerPath, `${JSON.stringify({ schema: "wp-codebox/test-seed-snapshot-pause/v1", seed_snapshot_source: seedSnapshot?.source ?? "" })}\n`)
266+
await new Promise((resolvePause) => setTimeout(resolvePause, 120_000))
267+
}
268+
223269
function runtimeSourceFixtureRoot(value) {
224270
const paths = []
225271
const collect = (entry) => {
@@ -307,24 +353,6 @@ const runtimeInputPath = join(workspace, ".codebox", "native-agent-task-input.js
307353
const resultPath = join(workspace, ".codebox", "agent-task-workflow-result.json")
308354
const controlledCodeboxPath = resolve(requestPath, "..")
309355
const nativeResultPath = join(controlledCodeboxPath, "native-agent-task-result.json")
310-
let cleaningPrivateRuntimeSources = false
311-
async function cleanupPrivateRuntimeSources() {
312-
if (cleaningPrivateRuntimeSources || !privateRuntimeSourceRoot) return
313-
cleaningPrivateRuntimeSources = true
314-
const root = privateRuntimeSourceRoot
315-
privateRuntimeSourceRoot = ""
316-
await rm(root, { recursive: true, force: true })
317-
}
318-
async function cleanupRunnerWorkspaceSeedSnapshot() {
319-
if (!runnerWorkspaceSeedSnapshot) return
320-
const source = runnerWorkspaceSeedSnapshot.source
321-
runnerWorkspaceSeedSnapshot = undefined
322-
await rm(source, { recursive: true, force: true })
323-
}
324-
process.once("exit", () => { if (privateRuntimeSourceRoot) rmSync(privateRuntimeSourceRoot, { recursive: true, force: true }) })
325-
for (const signal of ["SIGINT", "SIGTERM", "SIGHUP"]) {
326-
process.once(signal, () => { cleanupPrivateRuntimeSources().finally(() => process.exit(128)) })
327-
}
328356
const runnerWorkspaceTools = [
329357
"workspace_read", "workspace_ls", "workspace_grep", "workspace_write", "workspace_edit", "workspace_apply_patch",
330358
"workspace_show", "workspace_git_status", "workspace_git_diff",
@@ -346,6 +374,7 @@ if (accessError) {
346374
}
347375

348376
if (request.runner_workspace?.enabled) runnerWorkspaceSeedSnapshot = await createRunnerWorkspaceSeedSnapshot(workspace)
377+
await testPauseAfterSeedSnapshot(runnerWorkspaceSeedSnapshot)
349378

350379
const testFixtures = await testRuntimeFixtures(externalPackageSource)
351380
const skipMaterialization = process.env.NODE_ENV === "test" && (process.env.WP_CODEBOX_TEST_SKIP_MATERIALIZATION === "true" || Boolean(testFixtures.materializedPackage || testFixtures.runtimeSourceInputs))
@@ -430,7 +459,6 @@ let execution = { code: 0, stdout: "", stderr: "", stdout_truncated: false, stde
430459
if (request.run_agent && !request.dry_run) {
431460
execution = await command("node", [codeboxCliPath, "agent-task-run", "--input-file", executionInputPath, "--result-file", nativeResultPath], workspace, agentEnvironment())
432461
}
433-
await cleanupRunnerWorkspaceSeedSnapshot()
434462

435463
// Public package bytes are embedded in the runtime recipe and consumed only by
436464
// the Playground bootstrap before the agent's tools are resolved.
@@ -455,7 +483,6 @@ if (execution.code === 0 && runtimeResult.success === true && request.runner_wor
455483
downstreamFailure = { stage: "apply", message: bounded(error instanceof Error ? error.message : String(error), MAX_OUTPUT_CHARS) }
456484
}
457485
}
458-
await cleanupPrivateRuntimeSources()
459486
runtimeResult = sanitizeRuntimeSourceValue(runtimeResult, runtimeSourceOutputRoots)
460487
privateRuntimeSourceRootForSanitization = runtimeSourceOutputRoots
461488
assertNoRuntimeSourcePaths(runtimeResult, privateRuntimeSourceRootForSanitization)
@@ -565,20 +592,9 @@ if (!success) process.exitCode = 1
565592
try {
566593
await executeNativeAgentTask()
567594
} catch (error) {
568-
try {
569-
await writeNormalizedFailure(error)
570-
} finally {
571-
if (runnerWorkspaceSeedSnapshot) {
572-
const source = runnerWorkspaceSeedSnapshot.source
573-
runnerWorkspaceSeedSnapshot = undefined
574-
await rm(source, { recursive: true, force: true })
575-
}
576-
if (privateRuntimeSourceRoot) {
577-
const root = privateRuntimeSourceRoot
578-
privateRuntimeSourceRoot = ""
579-
await rm(root, { recursive: true, force: true })
580-
}
581-
}
595+
await writeNormalizedFailure(error)
582596
console.error(bounded(error instanceof Error ? error.message : String(error), MAX_OUTPUT_CHARS))
583597
process.exitCode = 1
598+
} finally {
599+
await cleanupMaterializedSources()
584600
}

.github/workflows/agent-task-contracts.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ on:
1818
- "tests/runtime-sources-materialization.test.ts"
1919
- "tests/runtime-sources-playground-integration.test.ts"
2020
- "tests/execute-native-agent-task-playground-e2e.test.ts"
21+
- "tests/execute-native-agent-task-interruption.test.mjs"
2122
- "tests/redaction.test.ts"
2223
- "tests/production-boundary-enforcement.test.ts"
2324
- "tests/runtime-tool-policy.test.ts"
@@ -38,6 +39,7 @@ on:
3839
- "tests/runtime-sources-materialization.test.ts"
3940
- "tests/runtime-sources-playground-integration.test.ts"
4041
- "tests/execute-native-agent-task-playground-e2e.test.ts"
42+
- "tests/execute-native-agent-task-interruption.test.mjs"
4143
- "tests/redaction.test.ts"
4244
- "tests/production-boundary-enforcement.test.ts"
4345
- "tests/runtime-tool-policy.test.ts"
@@ -58,6 +60,7 @@ jobs:
5860
- run: npm run test:agent-task-contracts
5961
- run: npm run test:runtime-sources-playground-integration
6062
- run: npm run test:native-agent-task-playground-e2e
63+
- run: npm run test:native-agent-task-interruption
6164
- run: npm run test:redaction
6265
- run: npm run test:production-boundary-enforcement
6366
- run: npm run test:runtime-tool-policy

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@
130130
"test:runner-workspace-apply": "tsx tests/runner-workspace-apply.test.ts",
131131
"test:runner-workspace-publisher": "node tests/runner-workspace-publisher.test.mjs",
132132
"test:native-agent-task-lifecycle": "node tests/execute-native-agent-task-lifecycle.test.mjs",
133+
"test:native-agent-task-interruption": "node tests/execute-native-agent-task-interruption.test.mjs",
133134
"test:native-agent-task-playground-e2e": "tsx tests/execute-native-agent-task-playground-e2e.test.ts",
134135
"test:bench-command-step-behavior": "tsx tests/bench-command-step-behavior.test.ts",
135136
"test:generic-primitives": "npm run test:artifact-path-primitives && npm run test:browser-callback-materialization-contracts && npm run test:source-package-compiler-primitives && npm run test:bench-command-step-behavior && npm run test:generic-ability-runtime-run",
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import assert from "node:assert/strict"
2+
import { existsSync } from "node:fs"
3+
import { mkdtemp, mkdir, readdir, readFile, writeFile } from "node:fs/promises"
4+
import { tmpdir } from "node:os"
5+
import { join, resolve } from "node:path"
6+
import { spawn } from "node:child_process"
7+
import { setTimeout as delay } from "node:timers/promises"
8+
9+
const root = resolve(".")
10+
const executor = join(root, ".github/scripts/run-agent-task/execute-native-agent-task.mjs")
11+
const SIGNAL_EXIT_CODES = { SIGINT: 130, SIGTERM: 143, SIGHUP: 129 }
12+
13+
async function prepare() {
14+
const temp = await mkdtemp(join(tmpdir(), "wp-codebox-native-interruption-"))
15+
// A dedicated TMPDIR makes seed-snapshot residue deterministic to observe.
16+
const seedTemp = join(temp, "seed-tmp")
17+
const workspace = join(temp, "workspace")
18+
await mkdir(seedTemp, { recursive: true })
19+
await mkdir(join(workspace, ".codebox"), { recursive: true })
20+
await writeFile(join(workspace, "README.md"), "before\n")
21+
const request = {
22+
schema: "wp-codebox/agent-task-workflow-request/v1",
23+
model: { provider: "openai", name: "gpt-5" },
24+
external_package_source: {
25+
repository: "owner/agents",
26+
revision: "a".repeat(40),
27+
path: "agent.agent.json",
28+
digest: `sha256-bytes-v1:${"b".repeat(64)}`,
29+
},
30+
runtime_sources: [],
31+
workload: { id: "interruption-1", label: "Interruption" },
32+
target_repo: "owner/repo",
33+
prompt: "Interruption lifecycle fixture.",
34+
writable_paths: "README.md",
35+
runner_workspace: { enabled: true, repo: "owner/repo", base: "main", branch_prefix: "wp-codebox/agent-task/" },
36+
verification_commands: [],
37+
drift_checks: [],
38+
success: { requires_pr: false },
39+
access: { caller_repo: "owner/repo", allowed_repos: ["owner/repo"], access_token_repos: ["owner/repo"] },
40+
limits: { max_turns: 1, time_budget_ms: 1000 },
41+
artifacts: { expected: [], declarations: [] },
42+
outputs: { projections: {} },
43+
callback_data: {},
44+
run_agent: true,
45+
dry_run: true,
46+
}
47+
await writeFile(join(workspace, ".codebox", "agent-task-request.json"), JSON.stringify(request))
48+
return { temp, seedTemp, workspace }
49+
}
50+
51+
function spawnExecutor({ seedTemp, workspace }, extraEnv = {}) {
52+
const child = spawn(process.execPath, [executor], {
53+
cwd: workspace,
54+
env: {
55+
...process.env,
56+
NODE_ENV: "test",
57+
TMPDIR: seedTemp,
58+
AGENT_TASK_REQUEST_PATH: join(workspace, ".codebox", "agent-task-request.json"),
59+
AGENT_TASK_WORKSPACE: workspace,
60+
WP_CODEBOX_WORKFLOW_ROOT: root,
61+
GITHUB_TOKEN: "token",
62+
EXTERNAL_PACKAGE_SOURCE_POLICY: JSON.stringify({ version: 1, repositories: { "owner/agents": ["agent.agent.json"] } }),
63+
...extraEnv,
64+
},
65+
})
66+
let stderr = ""
67+
child.stderr.on("data", (chunk) => { stderr += chunk })
68+
const closed = new Promise((done) => { child.on("close", (code, signal) => done({ code, signal, stderr })) })
69+
return { child, closed }
70+
}
71+
72+
async function seedSnapshotEntries(seedTemp) {
73+
return (await readdir(seedTemp)).filter((entry) => entry.startsWith("wp-codebox-runner-workspace-seed-"))
74+
}
75+
76+
for (const signal of ["SIGTERM", "SIGINT", "SIGHUP"]) {
77+
const fixture = await prepare()
78+
const pauseFile = join(fixture.temp, "pause.json")
79+
const { child, closed } = spawnExecutor(fixture, { WP_CODEBOX_TEST_SEED_SNAPSHOT_PAUSE_FILE: pauseFile })
80+
81+
const deadline = Date.now() + 30_000
82+
while (!existsSync(pauseFile)) {
83+
assert.ok(Date.now() < deadline, `executor did not reach the seed snapshot pause hook for ${signal}`)
84+
await delay(50)
85+
}
86+
const marker = JSON.parse(await readFile(pauseFile, "utf8"))
87+
assert.equal(marker.schema, "wp-codebox/test-seed-snapshot-pause/v1")
88+
assert.ok(marker.seed_snapshot_source.startsWith(fixture.seedTemp), "seed snapshot is created under the controlled TMPDIR")
89+
assert.ok(existsSync(marker.seed_snapshot_source), "seed snapshot exists while the executor is paused")
90+
91+
child.kill(signal)
92+
const exit = await closed
93+
assert.equal(exit.signal, null, `the executor handles ${signal} itself instead of dying to the default signal action\n${exit.stderr}`)
94+
assert.equal(exit.code, SIGNAL_EXIT_CODES[signal], `conventional exit code for ${signal}\n${exit.stderr}`)
95+
assert.equal(existsSync(marker.seed_snapshot_source), false, `the temp seed snapshot directory is removed on ${signal}`)
96+
assert.deepEqual(await seedSnapshotEntries(fixture.seedTemp), [], `no runner workspace seed content survives ${signal}`)
97+
assert.equal(await readFile(join(fixture.workspace, "README.md"), "utf8"), "before\n", "the host workspace is untouched")
98+
assert.deepEqual((await readdir(join(fixture.workspace, ".codebox"))).sort(), ["agent-task-artifacts", "agent-task-request.json"], "no runner workspace materials survive in the workspace")
99+
assert.deepEqual(await readdir(join(fixture.workspace, ".codebox", "agent-task-artifacts")), [], "no artifacts are staged before interruption")
100+
}
101+
102+
// Normal completion continues to clean up through the same coordinator.
103+
const fixture = await prepare()
104+
const { closed } = spawnExecutor(fixture)
105+
const exit = await closed
106+
assert.equal(exit.code, 0, exit.stderr)
107+
assert.deepEqual(await seedSnapshotEntries(fixture.seedTemp), [], "no runner workspace seed content survives a normal run")
108+
const result = JSON.parse(await readFile(join(fixture.workspace, ".codebox", "agent-task-workflow-result.json"), "utf8"))
109+
assert.equal(result.status, "skipped")
110+
111+
console.log("native agent task interruption ok")

tests/runtime-sources-materialization.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,11 @@ await withTempDir("wp-codebox-runtime-sources-workflow-", async (directory) => {
287287

288288
const executor = await readFile(new URL("../.github/scripts/run-agent-task/execute-native-agent-task.mjs", import.meta.url), "utf8")
289289
assert.match(executor, /for \(const signal of \["SIGINT", "SIGTERM", "SIGHUP"\]\)/)
290-
assert.match(executor, /cleanupPrivateRuntimeSources\(\)\.finally\(\(\) => process\.exit\(128\)\)/)
290+
assert.match(executor, /const SIGNAL_EXIT_CODES = \{ SIGINT: 130, SIGTERM: 143, SIGHUP: 129 \}/)
291+
assert.match(executor, /cleanupMaterializedSources\(\)\.finally\(\(\) => process\.exit\(SIGNAL_EXIT_CODES\[signal\]\)\)/)
292+
assert.match(executor, /\} finally \{\n await cleanupMaterializedSources\(\)\n\}/, "every top-level completion path routes through the single cleanup coordinator")
293+
assert.equal(executor.match(/function cleanupMaterializedSources/g)?.length, 1, "cleanup stays centralized in one coordinator")
294+
assert.doesNotMatch(executor, /cleanupPrivateRuntimeSources|cleanupRunnerWorkspaceSeedSnapshot/, "no duplicate independent cleanup logic")
291295
assert.match(executor, /const executionInputPath = privateRuntimeSourceRoot \? join\(privateRuntimeSourceRoot, "native-agent-task-input\.json"\) : runtimeInputPath/)
292296
assert.match(executor, /const privatePreparationRoot = privateRuntimeSourceRoot \? join\(privateRuntimeSourceRoot, "prepared-runtime-sources"\) : ""/)
293297
assert.match(executor, /sanitizeRuntimeSourceValue\(nativeRuntimeResult, privateRuntimeSourceRootForSanitization\)/)

0 commit comments

Comments
 (0)