From c3ea82fefef5af6beb0bc6835428a273f711287a Mon Sep 17 00:00:00 2001 From: Tiberiu Socaci Date: Fri, 6 Nov 2026 21:58:32 +0200 Subject: [PATCH] Preserve container job exit codes in completion audit Signed-off-by: Tiberiu Socaci --- CHANGELOG.md | 3 ++ FEATURES.md | 4 +- TEST-PLAN.md | 5 ++ src/gateway/background.js | 1 + test/runtime-integration-jobs.test.js | 74 +++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82f35da..e6ce1ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ release gates. Stable promotion and publication require approval of the exact te ### Beta development notes +- Preserve parsed container background-job exit codes in the structured completion audit, while + keeping completion messages in plain language and unconfirmed outcomes unknown. + - Restore the agent directory refresh to 03:00 local time after a spring-forward clock change instead of retaining the transition day’s normalized 04:00 hour. diff --git a/FEATURES.md b/FEATURES.md index b089101..398c41d 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -4473,7 +4473,9 @@ are retired, bullet by bullet; everything else stands. attach and no exit code to read from the child. Output goes to a log under the artifact mount — the same absolute path on both sides, so the daemon tails the very file the job writes — and the wrapper appends a `[cg-exit:N]` marker as its last act, so a finished job reports a real exit code - (a missing marker is "unknown", never 0). The persisted row records `{backend, runId, container}`, + in the structured completion audit (a missing marker is "unknown", never 0). Completion notices + and engine continuations retain their plain-language outcome. The persisted row records + `{backend, runId, container}`, so after a daemon restart a recovered job is probed and signalled BY ITS RUN ID through the backend, never by the stale pid of a client process that is long gone. The memory reviewer follows the same rule: it resolves its own target, takes a `review` lease, and runs in the channel's diff --git a/TEST-PLAN.md b/TEST-PLAN.md index f71ba1e..4235e8d 100644 --- a/TEST-PLAN.md +++ b/TEST-PLAN.md @@ -6311,6 +6311,11 @@ structural invariants are automated; rendered navigation and feature claims also restart-unknown outcomes; their continuation prompt carries the semantic outcome and output tail while structured events retain the raw value/signal. The injected tool/Slack guide also instructs agents to report the plain-language outcome rather than an exit status. +- [x] Container audit fidelity (2026-11-06): `test/runtime-integration-jobs.test.js` starts actual + shell wrappers through `BackgroundJobs` and waits for the real poller. Exit 0 and exit 23 + survive into `bg_finish.exitCode`; a deliberately missing durable marker remains null with + unknown outcome. Notice/continuation prose stays semantic, each delivery occurs once and + completed rows and leases are released. - [x] Claude, Codex, and OpenCode generic failures/cancellation; Claude structured provider errors; CLI health; Codex MCP discovery; Drive sync fallback; local voice subprocesses; and transactional update commands have focused regressions proving semantic messages while raw diff --git a/src/gateway/background.js b/src/gateway/background.js index caa9609..335a04c 100644 --- a/src/gateway/background.js +++ b/src/gateway/background.js @@ -1144,6 +1144,7 @@ export class BackgroundJobs { // line of its own log — so a container job reports a real outcome instead of "unknown". const exitCode = rec.runtimeChild && !rec.timedOut ? parseContainerJobExit(raw) : null; await this._finish(rec, { + code: exitCode, outcome: rec.timedOut ? describeProcessOutcome({ timedOut: true, timeout: fmtDur(Date.now() - rec.startedAt) }) : exitCode != null diff --git a/test/runtime-integration-jobs.test.js b/test/runtime-integration-jobs.test.js index 3bfee72..5c097a4 100644 --- a/test/runtime-integration-jobs.test.js +++ b/test/runtime-integration-jobs.test.js @@ -257,3 +257,77 @@ test("proxy-mode background jobs redact hidden placeholders before persistence a assert.equal(persistedRow(started.id), null, "successful continuation retires the durable row"); assert.ok(backend.calls.leases.every((lease) => lease.released)); }); + +test("isolated job poller preserves known exit codes in audit and keeps missing markers unknown", async () => { + const backend = createFakeRuntimeBackend(); + let unknownSlug = ""; + let holdUnknownProbe = true; + const probe = backend.probe.bind(backend); + backend.probe = async (child) => { + // Keep the real poller observing this fixture as live until its marker has been removed. + // Slow CI must not finish the job between the wrapper write and the negative-control edit. + if (holdUnknownProbe && child?.runtime?.target?.slug === unknownSlug) return true; + return probe(child); + }; + const continuations = [], notices = [], delivered = []; + const jobs = new BackgroundJobs({ + resolveTarget: (slug, meta) => fakeTarget(backend, slug, meta), + runner: async ({ text, threadKey }) => { + continuations.push({ text, threadKey }); + return { content: "Safe fixture result", engine: "claude" }; + }, + deliver: async (_client, payload) => { delivered.push(payload.threadKey); }, + }); + jobs._client = () => null; + const fixtures = []; + for (const expected of [0, 23, null]) { + const suffix = expected === null ? "unknown" : String(expected); + const channelId = "C_JOB_EXIT_" + suffix; + const { entry } = await autoChannel(channelId, "job-exit-" + suffix); + if (expected === null) unknownSlug = entry.slug; + const started = await jobs.start({ + channelId, authorId: "U_JOB", threadKey: "exit-" + suffix, label: "safe outcome fixture", + command: "printf 'prepared-safe-marker\\n'; exit " + (expected ?? 0), + }); + assert.equal(started.ok, true, started.error); + fixtures.push({ expected, rec: jobs.jobs.get(started.id) }); + } + // Simulate an incomplete durable log only after the actual wrapper has written its marker. + // This negative control must never turn an unconfirmed result into a fabricated zero exit. + const unknown = fixtures.find((fixture) => fixture.expected === null).rec; + const markerDeadline = Date.now() + 10_000; + let completeLog = ""; + while (Date.now() < markerDeadline) { + try { completeLog = readFileSync(unknown.logFile, "utf8"); } catch {} + if (parseContainerJobExit(completeLog) !== null) break; + await new Promise((resolve) => setTimeout(resolve, 10)); + } + assert.equal(parseContainerJobExit(completeLog), 0, "the real wrapper finished before the fixture truncation"); + writeFileSync(unknown.logFile, stripContainerJobExit(completeLog)); + holdUnknownProbe = false; + const deadline = Date.now() + 12_000; + while (fixtures.some(({ rec }) => !rec.pendingDelivery) && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 25)); + } + for (const { expected, rec } of fixtures) { + assert.ok(rec.pendingDelivery, "the actual poller checkpoints each finished job"); + assert.equal(parseContainerJobExit(readFileSync(rec.logFile, "utf8")), expected); + const event = getDb().prepare("SELECT data FROM events WHERE event = 'bg_finish' ORDER BY id DESC") + .all().map(({ data }) => JSON.parse(data)).find((item) => item.id === rec.id); + assert.equal(event.exitCode, expected, "structured audit retains exactly the observed exit code"); + assert.equal(event.ok, expected === null ? null : expected === 0); + assert.equal(persistedRow(rec.id).pendingDelivery.outcome.ok, event.ok); + } + jobs._client = () => ({ platform: "slack", post: async ({ text }) => { notices.push(text); return { ok: true }; } }); + for (const { rec } of fixtures) await jobs._deliver(rec); + assert.equal(continuations.length, 3); + assert.equal(delivered.length, 3); + for (const text of [...notices, ...continuations.map((item) => item.text)]) { + assert.doesNotMatch(text, /exit code|cg-exit:/i, "semantic completion prose remains unchanged"); + } + assert.ok(continuations.some(({ text }) => text.includes("recorded outcome is success"))); + assert.ok(continuations.some(({ text }) => text.includes("recorded outcome is failure"))); + assert.ok(continuations.some(({ text }) => text.includes("recorded outcome is unknown"))); + assert.ok(fixtures.every(({ rec }) => persistedRow(rec.id) === null)); + assert.ok(backend.calls.leases.every((lease) => lease.released)); +});