From fc04642628b5a516381d08c093e44488df900f5d Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 14:36:14 +0300 Subject: [PATCH 01/19] docs: design fail-closed Codex gate supervision --- .../2026-08-19-fail-closed-codex-gate.md | 268 ++++++++++++++++++ ...026-08-19-fail-closed-codex-gate-design.md | 58 ++++ 2 files changed, 326 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md create mode 100644 docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md diff --git a/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md b/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md new file mode 100644 index 000000000..5eda2128b --- /dev/null +++ b/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md @@ -0,0 +1,268 @@ +# Fail-Closed Codex Gate Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make detached Codex jobs self-heal after worker loss and make the Claude Stop review gate deterministically fail closed without duplicate reviews of the same turn. + +**Architecture:** Persist queued work before spawning, reconcile active jobs against their worker PID on every control-plane read, and refuse late writes after a terminal transition. Reuse Stop-review results by a hash of session ID plus last assistant message, while retaining the existing foreground timeout. + +**Tech Stack:** Node.js ESM, built-in `node:test`, filesystem JSON state, Claude Code hooks. + +## Global Constraints + +- No new runtime dependencies or daemon. +- The only job transitions are `queued -> running -> completed|failed|cancelled`. +- Enabled review gates fail closed for infrastructure and persistence errors. +- The last assistant message is never stored in a gate cache key. +- All production changes follow RED-GREEN TDD. + +--- + +### Task 1: Self-healing tracked jobs + +**Files:** +- Modify: `tests/process.test.mjs` +- Modify: `tests/runtime.test.mjs` +- Modify: `plugins/codex/scripts/lib/process.mjs` +- Modify: `plugins/codex/scripts/lib/tracked-jobs.mjs` +- Modify: `plugins/codex/scripts/lib/job-control.mjs` +- Modify: `plugins/codex/scripts/lib/render.mjs` + +**Interfaces:** +- Produces: `isProcessAlive(pid, options?) -> boolean` +- Produces: `reconcileTrackedJobs(workspaceRoot, options?) -> Job[]` +- Consumes: existing `listJobs`, `writeJobFile`, and `upsertJob` persistence functions. + +- [ ] **Step 1: Write failing process-liveness tests** + +Add tests proving a successful signal probe and `EPERM` mean alive while `ESRCH` means dead: + +```js +assert.equal(isProcessAlive(123, { killImpl() {} }), true); +assert.equal(isProcessAlive(123, { killImpl() { throw Object.assign(new Error("gone"), { code: "ESRCH" }); } }), false); +assert.equal(isProcessAlive(123, { killImpl() { throw Object.assign(new Error("denied"), { code: "EPERM" }); } }), true); +``` + +- [ ] **Step 2: Verify the process test is RED** + +Run: `node --test --test-name-pattern="isProcessAlive" tests/process.test.mjs` + +Expected: FAIL because `isProcessAlive` is not exported. + +- [ ] **Step 3: Implement the process probe** + +Add `isProcessAlive` to `process.mjs` using `process.kill(pid, 0)`, returning false only for non-finite PIDs and `ESRCH`, and treating `EPERM` as alive. + +- [ ] **Step 4: Write failing reconciliation tests** + +Add runtime tests with persisted jobs proving: + +```js +assert.equal(payload.job.status, "failed"); +assert.equal(payload.waitTimedOut, false); +assert.match(payload.job.errorMessage, /worker exited/i); +``` + +and an old queued job without a PID fails with `/did not start within 5 seconds/i`. Update the existing active-timeout fixture to use `pid: process.pid` so it continues to represent a live worker. + +- [ ] **Step 5: Verify the reconciliation tests are RED** + +Run: `node --test --test-name-pattern="dead worker|startup grace|still active" tests/runtime.test.mjs` + +Expected: dead and unstarted jobs remain active, so the new assertions fail. + +- [ ] **Step 6: Implement reconciliation and rendering** + +In `tracked-jobs.mjs`, reconcile each active job: + +```js +if (job.status === "queued" && !Number.isFinite(job.pid) && ageMs >= 5000) { + return failTrackedJob(workspaceRoot, job, "Background worker did not start within 5 seconds."); +} +if (Number.isFinite(job.pid) && !isProcessAlive(job.pid)) { + return failTrackedJob(workspaceRoot, job, "Background worker exited before completing the job."); +} +``` + +Use reconciled jobs in all job-control read paths. Render `Error: ${job.errorMessage}` in failed-job details. + +- [ ] **Step 7: Verify Task 1 GREEN** + +Run: `node --test tests/process.test.mjs tests/runtime.test.mjs` + +Expected: PASS. + +### Task 2: Race-free launch and terminal-state protection + +**Files:** +- Modify: `tests/runtime.test.mjs` +- Modify: `plugins/codex/scripts/codex-companion.mjs` +- Modify: `plugins/codex/scripts/lib/tracked-jobs.mjs` + +**Interfaces:** +- Consumes: `reconcileTrackedJobs` from Task 1. +- Produces: queued job publication before detached worker spawn. +- Produces: `runTrackedJob(..., { skipIfTerminal: true })` for background workers. + +- [ ] **Step 1: Write failing terminal-state tests** + +Persist a cancelled job, call `runTrackedJob` with `skipIfTerminal: true`, and assert the runner is not called and the stored state remains cancelled. Remove a running job during a deferred runner and assert finalization does not recreate it. + +- [ ] **Step 2: Verify the terminal-state tests are RED** + +Run: `node --test --test-name-pattern="terminal job|removed job" tests/runtime.test.mjs` + +Expected: FAIL because current finalization overwrites or recreates the job. + +- [ ] **Step 3: Publish before spawn** + +Change enqueue ordering to: + +```js +writeJobFile(job.workspaceRoot, job.id, queuedRecord); +upsertJob(job.workspaceRoot, queuedRecord); +spawnDetachedTaskWorker(cwd, job.id); +``` + +The worker sets its own PID when it enters `runTrackedJob`; queued jobs receive the five-second startup grace from Task 1. + +- [ ] **Step 4: Protect terminal transitions** + +Before running a background job, return without executing when its stored status is terminal. Before success or failure writes, re-read the job and do not write when it is terminal or missing. + +- [ ] **Step 5: Verify Task 2 GREEN** + +Run: `node --test --test-name-pattern="background|terminal job|removed job|cancel|SessionEnd" tests/runtime.test.mjs` + +Expected: PASS. + +### Task 3: Fail-closed, idempotent Stop review + +**Files:** +- Modify: `tests/runtime.test.mjs` +- Modify: `plugins/codex/scripts/lib/tracked-jobs.mjs` +- Modify: `plugins/codex/scripts/codex-companion.mjs` +- Modify: `plugins/codex/scripts/stop-review-gate-hook.mjs` + +**Interfaces:** +- Produces: `CODEX_COMPANION_GATE_KEY` metadata on Stop-review jobs. +- Produces: deterministic gate-key hashing of session ID and last assistant message. +- Consumes: stored `result.rawOutput` for exact-turn reuse. + +- [ ] **Step 1: Write failing gate tests** + +Change the unavailable-Codex test to require: + +```js +assert.equal(JSON.parse(result.stdout).decision, "block"); +assert.match(JSON.parse(result.stdout).reason, /not set up/i); +``` + +Run the Stop hook twice with the same session and non-empty `last_assistant_message`; assert both decisions match and the fake Codex `nextTurnId` does not increase on the second call. + +- [ ] **Step 2: Verify the gate tests are RED** + +Run: `node --test --test-name-pattern="unavailable|same Claude response" tests/runtime.test.mjs` + +Expected: unavailable Codex produces no decision and the second Stop starts another turn. + +- [ ] **Step 3: Propagate and reuse the gate key** + +Hash without retaining message content: + +```js +createHash("sha256").update(`${sessionId}\0${lastAssistantMessage}`).digest("hex") +``` + +Pass it through `CODEX_COMPANION_GATE_KEY`, record it on the tracked job, and before launching find the matching current-session Stop job. Reparse completed output; block with the existing job ID for active, failed, or cancelled matches. + +- [ ] **Step 4: Make setup failures fail closed** + +When `buildSetupNote` returns a message for an enabled gate, emit: + +```js +emitDecision({ decision: "block", reason: setupNote }); +``` + +- [ ] **Step 5: Verify Task 3 GREEN** + +Run: `node --test --test-name-pattern="stop hook" tests/runtime.test.mjs` + +Expected: PASS. + +### Task 4: Atomic, strict JSON persistence + +**Files:** +- Modify: `tests/state.test.mjs` +- Modify: `plugins/codex/scripts/lib/state.mjs` +- Modify: `plugins/codex/scripts/stop-review-gate-hook.mjs` + +**Interfaces:** +- Produces: same-directory temporary write plus `renameSync` for state and job JSON. +- Produces: explicit parse errors from invalid state JSON. + +- [ ] **Step 1: Write failing persistence tests** + +Write invalid `state.json` and assert `loadState` throws an error containing the state path. Run the enabled Stop hook against invalid state and assert it emits `decision: block` with a persistence error. + +- [ ] **Step 2: Verify persistence tests are RED** + +Run: `node --test --test-name-pattern="invalid state|corrupt state" tests/state.test.mjs tests/runtime.test.mjs` + +Expected: `loadState` silently returns defaults and the hook emits no block decision. + +- [ ] **Step 3: Implement atomic strict persistence** + +Write JSON through a unique sibling temporary file and `fs.renameSync`. On parse failure throw `Failed to read Codex Companion state at : ` instead of returning defaults. Catch top-level Stop-hook errors and emit a block decision. + +- [ ] **Step 4: Verify Task 4 GREEN** + +Run: `node --test tests/state.test.mjs tests/runtime.test.mjs` + +Expected: PASS. + +### Task 5: Full verification and delivery + +**Files:** +- Modify: `plugins/codex/CHANGELOG.md` + +**Interfaces:** +- Consumes: all behavior from Tasks 1-4. +- Produces: release note and verified branch. + +- [ ] **Step 1: Add a concise changelog entry** + +Document dead-worker reconciliation, race-free background launch, exact-turn Stop reuse, and fail-closed setup/persistence failures under the current unreleased section. + +- [ ] **Step 2: Run complete verification** + +Run: + +```bash +npm test +npm run build +npm run check-version +``` + +Expected: all tests pass, TypeScript exits 0, and version metadata is consistent. + +- [ ] **Step 3: Review the final diff** + +Run: `git diff --check && git diff --stat origin/main...HEAD && git status --short --branch` + +Expected: no whitespace errors and only planned files changed. + +- [ ] **Step 4: Commit** + +```bash +git add docs/superpowers plugins/codex tests +git commit -m "fix: harden Codex stop gate supervision" +``` + +- [ ] **Step 5: Request independent review, fix blocking findings, and re-run verification** + +Dispatch a read-only verifier against `origin/main...HEAD`. Critical and Important findings must be fixed before publishing. + +- [ ] **Step 6: Publish and integrate through repository policy** + +Fetch `origin`, reconcile with its current default branch, push `codex/gate-supervision`, and use the required PR/review path. Record the final main-branch SHA or the external blocker if upstream permissions prevent merge. diff --git a/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md b/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md new file mode 100644 index 000000000..3efe2373d --- /dev/null +++ b/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md @@ -0,0 +1,58 @@ +# Fail-Closed Codex Gate Design + +## Goal + +Make Codex Companion jobs self-heal after worker loss and make the Claude Stop review gate block with an actionable error instead of silently allowing, looping, or launching the same review twice. + +## Confirmed failure modes + +- A detached worker is spawned before its queued job is persisted. The worker can start first, fail to find the job, and leave the parent to publish a job that will remain queued forever. +- Status and wait paths trust `queued` and `running` JSON without checking whether the worker PID still exists. +- A worker killed outside the normal exception path never reaches `runTrackedJob` finalization, so its job remains active forever. +- A late worker completion can recreate or overwrite a job already cancelled or removed by SessionEnd. +- When the review gate is enabled but Codex is unavailable, the Stop hook only writes a note and allows the session to end. +- Repeating Stop for the same Claude response always starts a fresh Codex review. +- State files are overwritten in place, so an interrupted write can be parsed as an empty default state with the gate disabled. + +## Required behavior + +### Job lifecycle + +- The only transitions are `queued -> running -> completed|failed|cancelled`. +- The queued record and request exist before the detached worker is spawned. +- A queued job without a worker PID receives a five-second startup grace period. After that it becomes `failed` with `Background worker did not start within 5 seconds.` +- A running or queued job with a dead PID becomes `failed` with `Background worker exited before completing the job.` +- Reconciliation runs before status, wait, result, cancel, task resume selection, and Stop-hook decisions. +- A terminal or removed job cannot be overwritten by a late worker. +- Failed job status output includes the stored error message. + +### Stop gate + +- An enabled gate is fail-closed for unavailable Codex, task failure, timeout, missing output, invalid output, and corrupt state. +- Every block explains the failure and how to retry. Active jobs also show the exact status and cancel commands. +- The gate key is a SHA-256 hash of the Claude session ID and the exact last assistant message. No message content is stored in the key. +- A completed Stop review for the same gate key is reused. An active matching review blocks with its existing job ID instead of starting another review. +- A different last assistant message gets a new gate key and a fresh review. +- If no last assistant message is supplied, the hook runs a fresh review and does not cache it. + +### Persistence + +- `state.json` and per-job JSON files are written to a same-directory temporary file and atomically renamed. +- Invalid persisted JSON is an explicit error. It must not silently reset `stopReviewGate` to `false`. +- No new daemon, dependency, heartbeat file, or long-lived lock is introduced. PID liveness covers the observed worker-loss failure; the existing 15-minute Stop timeout covers a live but non-returning gate review. + +## Verification + +- A dead running worker becomes `failed` during `status --wait` and returns without timing out. +- A queued job missing a PID past startup grace becomes `failed`. +- Existing live-job timeout behavior remains unchanged when the PID is alive. +- Background task enqueue and completion remain green. +- Cancelling or removing a job prevents late finalization from resurrecting it. +- An unavailable Codex emits `decision: block` when the gate is enabled. +- Running the Stop hook twice with the same session and last response starts one Codex turn and returns the same decision. +- Invalid state blocks the Stop hook with a clear persistence error. +- The complete Node test suite and TypeScript build pass. + +## Scope boundary + +This change does not add retries, a supervisor daemon, configurable heartbeat intervals, or arbitrary background-job runtime limits. Add those only after evidence of a live worker hanging while its process remains healthy. From 2a07cce0d90648356224738dae47e65c6e7f136f Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 14:43:45 +0300 Subject: [PATCH 02/19] fix: reconcile dead tracked jobs --- plugins/codex/scripts/lib/job-control.mjs | 13 +-- plugins/codex/scripts/lib/process.mjs | 14 +++ plugins/codex/scripts/lib/render.mjs | 3 + plugins/codex/scripts/lib/tracked-jobs.mjs | 38 +++++++- tests/process.test.mjs | 25 ++++- tests/runtime.test.mjs | 101 +++++++++++++++++++++ 6 files changed, 186 insertions(+), 8 deletions(-) diff --git a/plugins/codex/scripts/lib/job-control.mjs b/plugins/codex/scripts/lib/job-control.mjs index ad152c157..daad42980 100644 --- a/plugins/codex/scripts/lib/job-control.mjs +++ b/plugins/codex/scripts/lib/job-control.mjs @@ -1,8 +1,8 @@ import fs from "node:fs"; import { getSessionRuntimeStatus } from "./codex.mjs"; -import { getConfig, listJobs, readJobFile, resolveJobFile } from "./state.mjs"; -import { SESSION_ID_ENV } from "./tracked-jobs.mjs"; +import { getConfig, readJobFile, resolveJobFile } from "./state.mjs"; +import { reconcileTrackedJobs, SESSION_ID_ENV } from "./tracked-jobs.mjs"; import { resolveWorkspaceRoot } from "./workspace.mjs"; export const DEFAULT_MAX_STATUS_JOBS = 8; @@ -213,7 +213,7 @@ function matchJobReference(jobs, reference, predicate = () => true) { export function buildStatusSnapshot(cwd, options = {}) { const workspaceRoot = resolveWorkspaceRoot(cwd); const config = getConfig(workspaceRoot); - const jobs = sortJobsNewestFirst(filterJobsForCurrentSession(listJobs(workspaceRoot), options)); + const jobs = sortJobsNewestFirst(filterJobsForCurrentSession(reconcileTrackedJobs(workspaceRoot), options)); const maxJobs = options.maxJobs ?? DEFAULT_MAX_STATUS_JOBS; const maxProgressLines = options.maxProgressLines ?? DEFAULT_MAX_PROGRESS_LINES; @@ -241,7 +241,7 @@ export function buildStatusSnapshot(cwd, options = {}) { export function buildSingleJobSnapshot(cwd, reference, options = {}) { const workspaceRoot = resolveWorkspaceRoot(cwd); - const jobs = sortJobsNewestFirst(listJobs(workspaceRoot)); + const jobs = sortJobsNewestFirst(reconcileTrackedJobs(workspaceRoot)); const selected = matchJobReference(jobs, reference); if (!selected) { throw new Error(`No job found for "${reference}". Run /codex:status to inspect known jobs.`); @@ -255,7 +255,8 @@ export function buildSingleJobSnapshot(cwd, reference, options = {}) { export function resolveResultJob(cwd, reference) { const workspaceRoot = resolveWorkspaceRoot(cwd); - const jobs = sortJobsNewestFirst(reference ? listJobs(workspaceRoot) : filterJobsForCurrentSession(listJobs(workspaceRoot))); + const reconciledJobs = reconcileTrackedJobs(workspaceRoot); + const jobs = sortJobsNewestFirst(reference ? reconciledJobs : filterJobsForCurrentSession(reconciledJobs)); const selected = matchJobReference( jobs, reference, @@ -280,7 +281,7 @@ export function resolveResultJob(cwd, reference) { export function resolveCancelableJob(cwd, reference, options = {}) { const workspaceRoot = resolveWorkspaceRoot(cwd); - const jobs = sortJobsNewestFirst(listJobs(workspaceRoot)); + const jobs = sortJobsNewestFirst(reconcileTrackedJobs(workspaceRoot)); const activeJobs = jobs.filter((job) => job.status === "queued" || job.status === "running"); if (reference) { diff --git a/plugins/codex/scripts/lib/process.mjs b/plugins/codex/scripts/lib/process.mjs index dd8fc3751..adfa923f9 100644 --- a/plugins/codex/scripts/lib/process.mjs +++ b/plugins/codex/scripts/lib/process.mjs @@ -50,6 +50,20 @@ export function binaryAvailable(command, versionArgs = ["--version"], options = return { available: true, detail: result.stdout.trim() || result.stderr.trim() || "ok" }; } +export function isProcessAlive(pid, options = {}) { + if (!Number.isFinite(pid)) { + return false; + } + + const killImpl = options.killImpl ?? process.kill.bind(process); + try { + killImpl(pid, 0); + return true; + } catch (error) { + return error?.code !== "ESRCH"; + } +} + function looksLikeMissingProcessMessage(text) { return /not found|no running instance|cannot find|does not exist|no such process/i.test(text); } diff --git a/plugins/codex/scripts/lib/render.mjs b/plugins/codex/scripts/lib/render.mjs index 2ec185236..89c597ddc 100644 --- a/plugins/codex/scripts/lib/render.mjs +++ b/plugins/codex/scripts/lib/render.mjs @@ -126,6 +126,9 @@ function pushJobDetails(lines, job, options = {}) { if (job.summary) { lines.push(` Summary: ${job.summary}`); } + if (job.status === "failed" && job.errorMessage) { + lines.push(` Error: ${job.errorMessage}`); + } if (job.phase) { lines.push(` Phase: ${job.phase}`); } diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 902869012..5fe4e8574 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -1,7 +1,8 @@ import fs from "node:fs"; import process from "node:process"; -import { readJobFile, resolveJobFile, resolveJobLogFile, upsertJob, writeJobFile } from "./state.mjs"; +import { isProcessAlive } from "./process.mjs"; +import { listJobs, readJobFile, resolveJobFile, resolveJobLogFile, upsertJob, writeJobFile } from "./state.mjs"; export const SESSION_ID_ENV = "CODEX_COMPANION_SESSION_ID"; @@ -139,6 +140,41 @@ function readStoredJobOrNull(workspaceRoot, jobId) { return readJobFile(jobFile); } +function failTrackedJob(workspaceRoot, job, errorMessage) { + const completedAt = nowIso(); + const failedJob = { + ...job, + status: "failed", + phase: "failed", + errorMessage, + pid: null, + completedAt + }; + writeJobFile(workspaceRoot, job.id, failedJob); + upsertJob(workspaceRoot, failedJob); + return failedJob; +} + +export function reconcileTrackedJobs(workspaceRoot, options = {}) { + const now = options.now ?? Date.now(); + + return listJobs(workspaceRoot).map((job) => { + if (job.status !== "queued" && job.status !== "running") { + return job; + } + + const ageMs = now - Date.parse(job.createdAt ?? ""); + if (job.status === "queued" && !Number.isFinite(job.pid) && Number.isFinite(ageMs) && ageMs >= 5000) { + return failTrackedJob(workspaceRoot, job, "Background worker did not start within 5 seconds."); + } + if (Number.isFinite(job.pid) && !isProcessAlive(job.pid, { killImpl: options.killImpl })) { + return failTrackedJob(workspaceRoot, job, "Background worker exited before completing the job."); + } + + return job; + }); +} + export async function runTrackedJob(job, runner, options = {}) { const runningRecord = { ...job, diff --git a/tests/process.test.mjs b/tests/process.test.mjs index 80e0715b0..8f6dac2f1 100644 --- a/tests/process.test.mjs +++ b/tests/process.test.mjs @@ -1,7 +1,30 @@ import test from "node:test"; import assert from "node:assert/strict"; -import { terminateProcessTree } from "../plugins/codex/scripts/lib/process.mjs"; +import { isProcessAlive, terminateProcessTree } from "../plugins/codex/scripts/lib/process.mjs"; + +test("isProcessAlive treats successful and permission-denied probes as alive", () => { + assert.equal(isProcessAlive(123, { killImpl() {} }), true); + assert.equal( + isProcessAlive(123, { + killImpl() { + throw Object.assign(new Error("denied"), { code: "EPERM" }); + } + }), + true + ); +}); + +test("isProcessAlive treats a missing process as dead", () => { + assert.equal( + isProcessAlive(123, { + killImpl() { + throw Object.assign(new Error("gone"), { code: "ESRCH" }); + } + }), + false + ); +}); test("terminateProcessTree uses taskkill on Windows", () => { let captured = null; diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..fb3e4f01a 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1307,6 +1307,7 @@ test("status --wait times out cleanly when a job is still active", () => { id: "task-live", status: "running", title: "Codex Task", + pid: process.pid, logFile }, null, @@ -1328,6 +1329,7 @@ test("status --wait times out cleanly when a job is still active", () => { title: "Codex Task", jobClass: "task", summary: "Investigate flaky test", + pid: process.pid, logFile, createdAt: "2026-03-18T15:30:00.000Z", startedAt: "2026-03-18T15:30:01.000Z", @@ -1352,6 +1354,105 @@ test("status --wait times out cleanly when a job is still active", () => { assert.equal(payload.waitTimedOut, true); }); +test("status --wait marks a dead worker as failed before timing out", () => { + const workspace = makeTempDir(); + const stateDir = resolveStateDir(workspace); + const jobsDir = path.join(stateDir, "jobs"); + fs.mkdirSync(jobsDir, { recursive: true }); + + const logFile = path.join(jobsDir, "task-dead.log"); + fs.writeFileSync(logFile, "", "utf8"); + fs.writeFileSync( + path.join(jobsDir, "task-dead.json"), + JSON.stringify({ id: "task-dead", status: "running", title: "Codex Task", pid: 999999, logFile }, null, 2), + "utf8" + ); + fs.writeFileSync( + path.join(stateDir, "state.json"), + `${JSON.stringify( + { + version: 1, + config: { stopReviewGate: false }, + jobs: [ + { + id: "task-dead", + status: "running", + title: "Codex Task", + jobClass: "task", + pid: 999999, + logFile, + createdAt: "2026-03-18T15:30:00.000Z", + startedAt: "2026-03-18T15:30:01.000Z", + updatedAt: "2026-03-18T15:30:02.000Z" + } + ] + }, + null, + 2 + )}\n`, + "utf8" + ); + + const result = run("node", [SCRIPT, "status", "task-dead", "--wait", "--timeout-ms", "25", "--json"], { + cwd: workspace + }); + + assert.equal(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.equal(payload.job.status, "failed"); + assert.equal(payload.waitTimedOut, false); + assert.match(payload.job.errorMessage, /worker exited/i); + + const rendered = run("node", [SCRIPT, "status", "task-dead"], { cwd: workspace }); + assert.equal(rendered.status, 0, rendered.stderr); + assert.match(rendered.stdout, /Error: Background worker exited before completing the job\./); +}); + +test("status marks a queued job past startup grace as failed", () => { + const workspace = makeTempDir(); + const stateDir = resolveStateDir(workspace); + const jobsDir = path.join(stateDir, "jobs"); + fs.mkdirSync(jobsDir, { recursive: true }); + + const logFile = path.join(jobsDir, "task-unstarted.log"); + fs.writeFileSync(logFile, "", "utf8"); + fs.writeFileSync( + path.join(jobsDir, "task-unstarted.json"), + JSON.stringify({ id: "task-unstarted", status: "queued", title: "Codex Task", logFile }, null, 2), + "utf8" + ); + fs.writeFileSync( + path.join(stateDir, "state.json"), + `${JSON.stringify( + { + version: 1, + config: { stopReviewGate: false }, + jobs: [ + { + id: "task-unstarted", + status: "queued", + title: "Codex Task", + jobClass: "task", + logFile, + createdAt: "2026-03-18T15:30:00.000Z", + updatedAt: "2026-03-18T15:30:00.000Z" + } + ] + }, + null, + 2 + )}\n`, + "utf8" + ); + + const result = run("node", [SCRIPT, "status", "task-unstarted", "--json"], { cwd: workspace }); + + assert.equal(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.equal(payload.job.status, "failed"); + assert.match(payload.job.errorMessage, /did not start within 5 seconds/i); +}); + test("result returns the stored output for the latest finished job by default", () => { const workspace = makeTempDir(); const stateDir = resolveStateDir(workspace); From 7932b1d88c863dd9f718abcf1895a43fcbaf040b Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 14:51:54 +0300 Subject: [PATCH 03/19] fix: prevent tracked job resurrection --- plugins/codex/scripts/lib/tracked-jobs.mjs | 10 +++- tests/tracked-jobs.test.mjs | 58 ++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 tests/tracked-jobs.test.mjs diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 5fe4e8574..97330bd33 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -176,8 +176,16 @@ export function reconcileTrackedJobs(workspaceRoot, options = {}) { } export async function runTrackedJob(job, runner, options = {}) { + const storedJob = readStoredJobOrNull(job.workspaceRoot, job.id); + if (storedJob && storedJob.status !== "queued") { + return storedJob; + } + if (!storedJob && job.request) { + return null; + } + const runningRecord = { - ...job, + ...(storedJob ?? job), status: "running", startedAt: nowIso(), phase: "starting", diff --git a/tests/tracked-jobs.test.mjs b/tests/tracked-jobs.test.mjs new file mode 100644 index 000000000..df0d2a081 --- /dev/null +++ b/tests/tracked-jobs.test.mjs @@ -0,0 +1,58 @@ +import fs from "node:fs"; +import test from "node:test"; +import assert from "node:assert/strict"; + +import { makeTempDir } from "./helpers.mjs"; +import { readJobFile, resolveJobFile, upsertJob, writeJobFile } from "../plugins/codex/scripts/lib/state.mjs"; +import { runTrackedJob } from "../plugins/codex/scripts/lib/tracked-jobs.mjs"; + +test("runTrackedJob does not resurrect a terminal persisted job", async () => { + const workspaceRoot = makeTempDir(); + const job = { + id: "task-terminal", + workspaceRoot, + status: "queued", + request: { prompt: "do not run" } + }; + const terminalJob = { + ...job, + status: "failed", + phase: "failed", + errorMessage: "Background worker exited before completing the job.", + pid: null, + completedAt: "2026-08-19T12:00:00.000Z" + }; + writeJobFile(workspaceRoot, job.id, terminalJob); + upsertJob(workspaceRoot, terminalJob); + + let runnerInvoked = false; + const result = await runTrackedJob(job, async () => { + runnerInvoked = true; + return { exitStatus: 0 }; + }); + + assert.equal(runnerInvoked, false); + assert.deepEqual(result, terminalJob); + assert.deepEqual(readJobFile(resolveJobFile(workspaceRoot, job.id)), terminalJob); +}); + +test("runTrackedJob does not recreate a removed background job", async () => { + const workspaceRoot = makeTempDir(); + const job = { + id: "task-removed", + workspaceRoot, + status: "queued", + request: { prompt: "do not run" } + }; + const jobFile = resolveJobFile(workspaceRoot, job.id); + + let runnerInvoked = false; + const result = await runTrackedJob(job, async () => { + runnerInvoked = true; + return { exitStatus: 0 }; + }); + + assert.equal(runnerInvoked, false); + assert.equal(result, null); + assert.equal(fs.existsSync(jobFile), false); +}); From 32dbafa8c41f7c0692436e90ae00e670ed7c4420 Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 15:24:13 +0300 Subject: [PATCH 04/19] fix: fence terminal Codex jobs --- .../2026-08-19-fail-closed-codex-gate.md | 6 +- ...026-08-19-fail-closed-codex-gate-design.md | 4 +- plugins/codex/scripts/codex-companion.mjs | 30 ++-- plugins/codex/scripts/lib/job-control.mjs | 10 +- plugins/codex/scripts/lib/tracked-jobs.mjs | 147 +++++++++++++----- .../codex/scripts/session-lifecycle-hook.mjs | 8 + .../codex/scripts/stop-review-gate-hook.mjs | 6 +- tests/runtime.test.mjs | 130 +++++++++++++++- 8 files changed, 267 insertions(+), 74 deletions(-) diff --git a/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md b/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md index 5eda2128b..2e85d9ede 100644 --- a/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md +++ b/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md @@ -102,11 +102,11 @@ Expected: PASS. **Interfaces:** - Consumes: `reconcileTrackedJobs` from Task 1. - Produces: queued job publication before detached worker spawn. -- Produces: `runTrackedJob(..., { skipIfTerminal: true })` for background workers. +- Produces: an immutable per-job terminal fence, created with exclusive filesystem creation, whose first writer wins. - [ ] **Step 1: Write failing terminal-state tests** -Persist a cancelled job, call `runTrackedJob` with `skipIfTerminal: true`, and assert the runner is not called and the stored state remains cancelled. Remove a running job during a deferred runner and assert finalization does not recreate it. +Persist a terminal fence and assert a late worker cannot run or overwrite its first terminal outcome. Remove a running job during a deferred runner after SessionEnd/cancellation fences it and assert progress/finalization does not recreate a visible job. Add a deterministic reconciliation-versus-worker interleaving test. - [ ] **Step 2: Verify the terminal-state tests are RED** @@ -128,7 +128,7 @@ The worker sets its own PID when it enters `runTrackedJob`; queued jobs receive - [ ] **Step 4: Protect terminal transitions** -Before running a background job, return without executing when its stored status is terminal. Before success or failure writes, re-read the job and do not write when it is terminal or missing. +Every terminal writer (reconciliation, completion/failure, cancel, and SessionEnd) claims `jobs/.terminal.json` with `openSync(..., "wx")`; the first claimed status overrides stale mutable JSON. Empty/corrupt fences are failed, never overwritten. Progress/upsert and effective control-plane reads honor the fence, and SessionEnd removes mutable records only after fencing active jobs. - [ ] **Step 5: Verify Task 2 GREEN** diff --git a/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md b/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md index 3efe2373d..77967a340 100644 --- a/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md +++ b/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md @@ -23,7 +23,9 @@ Make Codex Companion jobs self-heal after worker loss and make the Claude Stop r - A queued job without a worker PID receives a five-second startup grace period. After that it becomes `failed` with `Background worker did not start within 5 seconds.` - A running or queued job with a dead PID becomes `failed` with `Background worker exited before completing the job.` - Reconciliation runs before status, wait, result, cancel, task resume selection, and Stop-hook decisions. -- A terminal or removed job cannot be overwritten by a late worker. +- Terminal state is claimed by an immutable per-job `jobs/.terminal.json` fence, created with exclusive filesystem creation. The first terminal writer wins; terminal fences contain only status and completion time, never request, prompt, or log content. +- A terminal or removed job cannot be overwritten by a late worker. Corrupt or empty fences fail closed as `failed`; all control-plane reads use the fence over mutable job/index JSON. +- SessionEnd fences active jobs before removing their mutable job and index records. It may leave the small terminal fence so a late worker cannot resurrect the job. - Failed job status output includes the stored error message. ### Stop gate diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..d0c0b1835 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -29,7 +29,6 @@ import { loadPromptTemplate, interpolateTemplate } from "./lib/prompts.mjs"; import { generateJobId, getConfig, - listJobs, setConfig, upsertJob, writeJobFile @@ -49,8 +48,10 @@ import { createJobRecord, createProgressReporter, nowIso, + reconcileTrackedJobs, runTrackedJob, - SESSION_ID_ENV + SESSION_ID_ENV, + terminalizeTrackedJob } from "./lib/tracked-jobs.mjs"; import { resolveWorkspaceRoot } from "./lib/workspace.mjs"; import { @@ -336,7 +337,7 @@ async function waitForSingleJobSnapshot(cwd, reference, options = {}) { async function resolveLatestTrackedTaskThread(cwd, options = {}) { const workspaceRoot = resolveWorkspaceRoot(cwd); const sessionId = getCurrentClaudeSessionId(); - const jobs = sortJobsNewestFirst(listJobs(workspaceRoot)).filter((job) => job.id !== options.excludeJobId); + const jobs = sortJobsNewestFirst(reconcileTrackedJobs(workspaceRoot)).filter((job) => job.id !== options.excludeJobId); const visibleJobs = filterJobsForCurrentClaudeSession(jobs); const activeTask = visibleJobs.find((job) => job.jobClass === "task" && (job.status === "queued" || job.status === "running")); if (activeTask) { @@ -685,17 +686,17 @@ function enqueueBackgroundTask(cwd, job, request) { const { logFile } = createTrackedProgress(job); appendLogLine(logFile, "Queued for background execution."); - const child = spawnDetachedTaskWorker(cwd, job.id); const queuedRecord = { ...job, status: "queued", phase: "queued", - pid: child.pid ?? null, + pid: null, logFile, request }; writeJobFile(job.workspaceRoot, job.id, queuedRecord); upsertJob(job.workspaceRoot, queuedRecord); + spawnDetachedTaskWorker(cwd, job.id); return { payload: { @@ -934,7 +935,7 @@ function handleTaskResumeCandidate(argv) { const cwd = resolveCommandCwd(options); const workspaceRoot = resolveCommandWorkspace(options); const sessionId = getCurrentClaudeSessionId(); - const jobs = filterJobsForCurrentClaudeSession(sortJobsNewestFirst(listJobs(workspaceRoot))); + const jobs = filterJobsForCurrentClaudeSession(sortJobsNewestFirst(reconcileTrackedJobs(workspaceRoot))); const candidate = findLatestResumableTaskJob(jobs); const payload = { @@ -983,9 +984,6 @@ async function handleCancel(argv) { ); } - terminateProcessTree(job.pid ?? Number.NaN); - appendLogLine(job.logFile, "Cancelled by user."); - const completedAt = nowIso(); const nextJob = { ...job, @@ -996,19 +994,15 @@ async function handleCancel(argv) { errorMessage: "Cancelled by user." }; - writeJobFile(workspaceRoot, job.id, { + terminalizeTrackedJob(workspaceRoot, { ...existing, + ...nextJob + }, { ...nextJob, cancelledAt: completedAt }); - upsertJob(workspaceRoot, { - id: job.id, - status: "cancelled", - phase: "cancelled", - pid: null, - errorMessage: "Cancelled by user.", - completedAt - }); + terminateProcessTree(job.pid ?? Number.NaN); + appendLogLine(job.logFile, "Cancelled by user."); const payload = { jobId: job.id, diff --git a/plugins/codex/scripts/lib/job-control.mjs b/plugins/codex/scripts/lib/job-control.mjs index daad42980..89b264ce6 100644 --- a/plugins/codex/scripts/lib/job-control.mjs +++ b/plugins/codex/scripts/lib/job-control.mjs @@ -1,8 +1,8 @@ import fs from "node:fs"; import { getSessionRuntimeStatus } from "./codex.mjs"; -import { getConfig, readJobFile, resolveJobFile } from "./state.mjs"; -import { reconcileTrackedJobs, SESSION_ID_ENV } from "./tracked-jobs.mjs"; +import { getConfig } from "./state.mjs"; +import { readEffectiveStoredJob, reconcileTrackedJobs, SESSION_ID_ENV } from "./tracked-jobs.mjs"; import { resolveWorkspaceRoot } from "./workspace.mjs"; export const DEFAULT_MAX_STATUS_JOBS = 8; @@ -181,11 +181,7 @@ export function enrichJob(job, options = {}) { } export function readStoredJob(workspaceRoot, jobId) { - const jobFile = resolveJobFile(workspaceRoot, jobId); - if (!fs.existsSync(jobFile)) { - return null; - } - return readJobFile(jobFile); + return readEffectiveStoredJob(workspaceRoot, jobId); } function matchJobReference(jobs, reference, predicate = () => true) { diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 97330bd33..4a312756c 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -100,10 +100,8 @@ export function createJobProgressUpdater(workspaceRoot, jobId) { return; } - upsertJob(workspaceRoot, patch); - const jobFile = resolveJobFile(workspaceRoot, jobId); - if (!fs.existsSync(jobFile)) { + if (readTerminalFence(workspaceRoot, jobId) || !fs.existsSync(jobFile)) { return; } @@ -112,6 +110,7 @@ export function createJobProgressUpdater(workspaceRoot, jobId) { ...storedJob, ...patch }); + upsertJob(workspaceRoot, patch); }; } @@ -140,43 +139,128 @@ function readStoredJobOrNull(workspaceRoot, jobId) { return readJobFile(jobFile); } -function failTrackedJob(workspaceRoot, job, errorMessage) { - const completedAt = nowIso(); - const failedJob = { +const TERMINAL_STATUSES = new Set(["completed", "failed", "cancelled"]); + +function resolveTerminalFenceFile(workspaceRoot, jobId) { + return resolveJobFile(workspaceRoot, jobId).replace(/\.json$/, ".terminal.json"); +} + +function terminalPhase(status) { + return status === "completed" ? "done" : status; +} + +export function readTerminalFence(workspaceRoot, jobId) { + const fenceFile = resolveTerminalFenceFile(workspaceRoot, jobId); + if (!fs.existsSync(fenceFile)) { + return null; + } + + try { + const parsed = JSON.parse(fs.readFileSync(fenceFile, "utf8")); + if (!parsed || typeof parsed !== "object" || !TERMINAL_STATUSES.has(parsed.status)) { + throw new Error("invalid terminal status"); + } + return { + status: parsed.status, + completedAt: typeof parsed.completedAt === "string" ? parsed.completedAt : null + }; + } catch { + return { status: "failed", completedAt: null, corrupt: true }; + } +} + +function applyTerminalFence(job, fence) { + if (!fence) { + return job; + } + return { ...job, + status: fence.status, + phase: terminalPhase(fence.status), + pid: null, + completedAt: fence.completedAt ?? job.completedAt ?? null, + ...(fence.corrupt ? { errorMessage: "Terminal job fence is corrupt." } : {}) + }; +} + +function claimTerminalFence(workspaceRoot, jobId, status, completedAt) { + const fenceFile = resolveTerminalFenceFile(workspaceRoot, jobId); + try { + const descriptor = fs.openSync(fenceFile, "wx"); + try { + fs.writeFileSync(descriptor, `${JSON.stringify({ status, completedAt })}\n`, "utf8"); + } finally { + fs.closeSync(descriptor); + } + return { fence: { status, completedAt }, claimed: true }; + } catch (error) { + if (error?.code !== "EEXIST") { + throw error; + } + return { fence: readTerminalFence(workspaceRoot, jobId), claimed: false }; + } +} + +export function readEffectiveStoredJob(workspaceRoot, jobId) { + const storedJob = readStoredJobOrNull(workspaceRoot, jobId); + return storedJob ? applyTerminalFence(storedJob, readTerminalFence(workspaceRoot, jobId)) : null; +} + +export function terminalizeTrackedJob(workspaceRoot, job, terminal) { + const completedAt = terminal.completedAt ?? nowIso(); + const { fence, claimed } = claimTerminalFence(workspaceRoot, job.id, terminal.status, completedAt); + const storedJob = readStoredJobOrNull(workspaceRoot, job.id); + const effectiveJob = applyTerminalFence({ ...(storedJob ?? job), ...terminal }, fence); + + if (!claimed) { + return { job: storedJob ? applyTerminalFence(storedJob, fence) : null, claimed }; + } + + writeJobFile(workspaceRoot, job.id, effectiveJob); + upsertJob(workspaceRoot, effectiveJob); + return { job: effectiveJob, claimed }; +} + +function failTrackedJob(workspaceRoot, job, errorMessage) { + return terminalizeTrackedJob(workspaceRoot, job, { status: "failed", phase: "failed", errorMessage, pid: null, - completedAt - }; - writeJobFile(workspaceRoot, job.id, failedJob); - upsertJob(workspaceRoot, failedJob); - return failedJob; + completedAt: nowIso() + }).job; } export function reconcileTrackedJobs(workspaceRoot, options = {}) { const now = options.now ?? Date.now(); - return listJobs(workspaceRoot).map((job) => { + return listJobs(workspaceRoot).flatMap((job) => { + const fence = readTerminalFence(workspaceRoot, job.id); + if (fence) { + return fs.existsSync(resolveJobFile(workspaceRoot, job.id)) ? [applyTerminalFence(job, fence)] : []; + } if (job.status !== "queued" && job.status !== "running") { - return job; + return [job]; } const ageMs = now - Date.parse(job.createdAt ?? ""); if (job.status === "queued" && !Number.isFinite(job.pid) && Number.isFinite(ageMs) && ageMs >= 5000) { - return failTrackedJob(workspaceRoot, job, "Background worker did not start within 5 seconds."); + return [failTrackedJob(workspaceRoot, job, "Background worker did not start within 5 seconds.")]; } if (Number.isFinite(job.pid) && !isProcessAlive(job.pid, { killImpl: options.killImpl })) { - return failTrackedJob(workspaceRoot, job, "Background worker exited before completing the job."); + return [failTrackedJob(workspaceRoot, job, "Background worker exited before completing the job.")]; } - return job; + return [job]; }); } export async function runTrackedJob(job, runner, options = {}) { const storedJob = readStoredJobOrNull(job.workspaceRoot, job.id); + const fence = readTerminalFence(job.workspaceRoot, job.id); + if (fence) { + return storedJob ? applyTerminalFence(storedJob, fence) : null; + } if (storedJob && storedJob.status !== "queued") { return storedJob; } @@ -199,7 +283,7 @@ export async function runTrackedJob(job, runner, options = {}) { const execution = await runner(); const completionStatus = execution.exitStatus === 0 ? "completed" : "failed"; const completedAt = nowIso(); - writeJobFile(job.workspaceRoot, job.id, { + const terminal = terminalizeTrackedJob(job.workspaceRoot, runningRecord, { ...runningRecord, status: completionStatus, threadId: execution.threadId ?? null, @@ -210,38 +294,21 @@ export async function runTrackedJob(job, runner, options = {}) { result: execution.payload, rendered: execution.rendered }); - upsertJob(job.workspaceRoot, { - id: job.id, - status: completionStatus, - threadId: execution.threadId ?? null, - turnId: execution.turnId ?? null, - summary: execution.summary, - phase: completionStatus === "completed" ? "done" : "failed", - pid: null, - completedAt - }); - appendLogBlock(options.logFile ?? job.logFile ?? null, "Final output", execution.rendered); + if (terminal.claimed) { + appendLogBlock(options.logFile ?? job.logFile ?? null, "Final output", execution.rendered); + } return execution; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); - const existing = readStoredJobOrNull(job.workspaceRoot, job.id) ?? runningRecord; const completedAt = nowIso(); - writeJobFile(job.workspaceRoot, job.id, { - ...existing, + terminalizeTrackedJob(job.workspaceRoot, runningRecord, { + ...runningRecord, status: "failed", phase: "failed", errorMessage, pid: null, completedAt, - logFile: options.logFile ?? job.logFile ?? existing.logFile ?? null - }); - upsertJob(job.workspaceRoot, { - id: job.id, - status: "failed", - phase: "failed", - pid: null, - errorMessage, - completedAt + logFile: options.logFile ?? job.logFile ?? runningRecord.logFile ?? null }); throw error; } diff --git a/plugins/codex/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs index 778571e6c..ec4038dfc 100644 --- a/plugins/codex/scripts/session-lifecycle-hook.mjs +++ b/plugins/codex/scripts/session-lifecycle-hook.mjs @@ -14,6 +14,7 @@ import { teardownBrokerSession } from "./lib/broker-lifecycle.mjs"; import { loadState, resolveStateFile, saveState } from "./lib/state.mjs"; +import { terminalizeTrackedJob } from "./lib/tracked-jobs.mjs"; import { TRANSCRIPT_PATH_ENV } from "./lib/claude-session-transfer.mjs"; import { resolveWorkspaceRoot } from "./lib/workspace.mjs"; @@ -61,6 +62,13 @@ function cleanupSessionJobs(cwd, sessionId) { if (!stillRunning) { continue; } + terminalizeTrackedJob(workspaceRoot, job, { + status: "cancelled", + phase: "cancelled", + pid: null, + completedAt: new Date().toISOString(), + errorMessage: "Cancelled because the Claude session ended." + }); try { terminateProcessTree(job.pid ?? Number.NaN); } catch { diff --git a/plugins/codex/scripts/stop-review-gate-hook.mjs b/plugins/codex/scripts/stop-review-gate-hook.mjs index 2346bdcf4..2c5eb44f4 100644 --- a/plugins/codex/scripts/stop-review-gate-hook.mjs +++ b/plugins/codex/scripts/stop-review-gate-hook.mjs @@ -8,9 +8,9 @@ import { fileURLToPath } from "node:url"; import { getCodexAvailability } from "./lib/codex.mjs"; import { loadPromptTemplate, interpolateTemplate } from "./lib/prompts.mjs"; -import { getConfig, listJobs } from "./lib/state.mjs"; +import { getConfig } from "./lib/state.mjs"; import { sortJobsNewestFirst } from "./lib/job-control.mjs"; -import { SESSION_ID_ENV } from "./lib/tracked-jobs.mjs"; +import { reconcileTrackedJobs, SESSION_ID_ENV } from "./lib/tracked-jobs.mjs"; import { resolveWorkspaceRoot } from "./lib/workspace.mjs"; const STOP_REVIEW_TIMEOUT_MS = 15 * 60 * 1000; @@ -145,7 +145,7 @@ function main() { const workspaceRoot = resolveWorkspaceRoot(cwd); const config = getConfig(workspaceRoot); - const jobs = sortJobsNewestFirst(filterJobsForCurrentSession(listJobs(workspaceRoot), input)); + const jobs = sortJobsNewestFirst(filterJobsForCurrentSession(reconcileTrackedJobs(workspaceRoot), input)); const runningJob = jobs.find((job) => job.status === "queued" || job.status === "running"); const runningTaskNote = runningJob ? `Codex task ${runningJob.id} is still running. Check /codex:status and use /codex:cancel ${runningJob.id} if you want to stop it before ending the session.` diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index fb3e4f01a..e2a8b733e 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -8,7 +8,8 @@ import { fileURLToPath } from "node:url"; import { buildEnv, installFakeCodex } from "./fake-codex-fixture.mjs"; import { initGitRepo, makeTempDir, run } from "./helpers.mjs"; import { loadBrokerSession, saveBrokerSession } from "../plugins/codex/scripts/lib/broker-lifecycle.mjs"; -import { resolveStateDir } from "../plugins/codex/scripts/lib/state.mjs"; +import { listJobs, readJobFile, resolveJobFile, resolveStateDir, saveState, upsertJob, writeJobFile } from "../plugins/codex/scripts/lib/state.mjs"; +import { createJobProgressUpdater, reconcileTrackedJobs, runTrackedJob } from "../plugins/codex/scripts/lib/tracked-jobs.mjs"; const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const PLUGIN_ROOT = path.join(ROOT, "plugins", "codex"); @@ -28,6 +29,10 @@ async function waitFor(predicate, { timeoutMs = 5000, intervalMs = 50 } = {}) { throw new Error("Timed out waiting for condition."); } +function resolveTerminalFenceFile(workspaceRoot, jobId) { + return resolveJobFile(workspaceRoot, jobId).replace(/\.json$/, ".terminal.json"); +} + test("setup reports ready when fake codex is installed and authenticated", () => { const binDir = makeTempDir(); installFakeCodex(binDir); @@ -969,6 +974,127 @@ test("task --background enqueues a detached worker and exposes per-job status", assert.match(resultPayload.storedJob.rendered, /Handled the requested task/); }); +test("background task publishes its queued record before spawning the detached worker", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const markerFile = path.join(makeTempDir(), "task-worker-spawned-at"); + const preloadFile = path.join(makeTempDir(), "record-task-worker-spawn.cjs"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync( + preloadFile, + [ + 'const fs = require("node:fs");', + 'const childProcess = require("node:child_process");', + "const originalSpawn = childProcess.spawn;", + "childProcess.spawn = (...args) => {", + ' if (args[1]?.includes("task-worker")) fs.writeFileSync(process.env.CODEX_TASK_WORKER_SPAWN_MARKER, String(Date.now()));', + " return originalSpawn(...args);", + "};" + ].join("\n"), + "utf8" + ); + + const result = run("node", [SCRIPT, "task", "--background", "--json", "check queue publication"], { + cwd: repo, + env: { + ...buildEnv(binDir), + CODEX_TASK_WORKER_SPAWN_MARKER: markerFile, + NODE_OPTIONS: `--require ${preloadFile}` + } + }); + + assert.equal(result.status, 0, result.stderr); + const jobId = JSON.parse(result.stdout).jobId; + const jobFile = resolveJobFile(repo, jobId); + assert.equal(fs.existsSync(jobFile), true); + assert.ok(fs.statSync(jobFile).mtimeMs <= Number(fs.readFileSync(markerFile, "utf8"))); +}); + +test("terminal job fence prevents a queued worker from running", async () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-fenced", workspaceRoot, status: "queued", request: { prompt: "do not run" } }; + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + fs.writeFileSync( + resolveTerminalFenceFile(workspaceRoot, job.id), + JSON.stringify({ status: "cancelled", completedAt: "2026-08-19T12:00:00.000Z" }), + "utf8" + ); + + let runnerInvoked = false; + const result = await runTrackedJob(job, async () => { + runnerInvoked = true; + return { exitStatus: 0 }; + }); + + assert.equal(runnerInvoked, false); + assert.equal(result.status, "cancelled"); + assert.equal(readJobFile(resolveJobFile(workspaceRoot, job.id)).status, "queued"); +}); + +test("terminal job reconciliation wins over late worker finalization", async () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-race", workspaceRoot, status: "queued" }; + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + + let releaseRunner; + const runnerStarted = new Promise((resolve) => { + releaseRunner = resolve; + }); + const execution = runTrackedJob(job, async () => { + await runnerStarted; + return { exitStatus: 0 }; + }); + + await waitFor(() => listJobs(workspaceRoot).some((candidate) => candidate.id === job.id && candidate.status === "running")); + reconcileTrackedJobs(workspaceRoot, { + killImpl() { + throw Object.assign(new Error("gone"), { code: "ESRCH" }); + } + }); + releaseRunner(); + await execution; + + assert.equal(listJobs(workspaceRoot).find((candidate) => candidate.id === job.id).status, "failed"); + assert.equal(readJobFile(resolveJobFile(workspaceRoot, job.id)).status, "failed"); +}); + +test("removed job terminal fence prevents progress and finalization from recreating it", async () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-removed", workspaceRoot, status: "queued" }; + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + const progress = createJobProgressUpdater(workspaceRoot, job.id); + + let releaseRunner; + const runnerStarted = new Promise((resolve) => { + releaseRunner = resolve; + }); + const execution = runTrackedJob(job, async () => { + await runnerStarted; + return { exitStatus: 0 }; + }); + + await waitFor(() => listJobs(workspaceRoot).some((candidate) => candidate.id === job.id && candidate.status === "running")); + fs.writeFileSync( + resolveTerminalFenceFile(workspaceRoot, job.id), + JSON.stringify({ status: "cancelled", completedAt: "2026-08-19T12:00:00.000Z" }), + "utf8" + ); + saveState(workspaceRoot, { config: { stopReviewGate: false }, jobs: [] }); + progress({ phase: "investigating" }); + releaseRunner(); + await execution; + + assert.deepEqual(listJobs(workspaceRoot), []); + assert.equal(fs.existsSync(resolveJobFile(workspaceRoot, job.id)), false); +}); + test("review rejects focus text because it is native-review only", () => { const repo = makeTempDir(); const binDir = makeTempDir(); @@ -2006,7 +2132,7 @@ test("session end fully cleans up jobs for the ending session", async (t) => { assert.equal(fs.existsSync(otherJobFile), true); assert.deepEqual( fs.readdirSync(path.dirname(otherJobFile)).sort(), - [path.basename(otherJobFile), path.basename(otherSessionLog)].sort() + [path.basename(otherJobFile), path.basename(otherSessionLog), "review-running.terminal.json"].sort() ); await waitFor(() => { From 74b446ba04f51082e90420adc28e034a7240d26f Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 15:41:18 +0300 Subject: [PATCH 05/19] fix: close Codex job claim races --- .../2026-08-19-fail-closed-codex-gate.md | 2 +- ...026-08-19-fail-closed-codex-gate-design.md | 3 +- plugins/codex/scripts/codex-companion.mjs | 34 ++-- plugins/codex/scripts/lib/tracked-jobs.mjs | 150 ++++++++++++++++-- .../codex/scripts/session-lifecycle-hook.mjs | 3 +- tests/runtime.test.mjs | 97 ++++++++++- 6 files changed, 253 insertions(+), 36 deletions(-) diff --git a/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md b/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md index 2e85d9ede..6149ba8d8 100644 --- a/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md +++ b/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md @@ -128,7 +128,7 @@ The worker sets its own PID when it enters `runTrackedJob`; queued jobs receive - [ ] **Step 4: Protect terminal transitions** -Every terminal writer (reconciliation, completion/failure, cancel, and SessionEnd) claims `jobs/.terminal.json` with `openSync(..., "wx")`; the first claimed status overrides stale mutable JSON. Empty/corrupt fences are failed, never overwritten. Progress/upsert and effective control-plane reads honor the fence, and SessionEnd removes mutable records only after fencing active jobs. +Workers first claim `jobs/.started.json` with `openSync(..., "wx")`; reconciliation, SessionEnd, and startup compete there before a running publication. Later terminal outcomes compete on `jobs/.terminal.json`. Empty/corrupt claims fail as failed, progress never upserts the state index, and effective reads merge mutable job-file fields. SessionEnd writes a dominant empty `jobs/.removed` marker before cleanup so a late mutable artifact is never visible. - [ ] **Step 5: Verify Task 2 GREEN** diff --git a/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md b/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md index 77967a340..4f293172c 100644 --- a/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md +++ b/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md @@ -24,8 +24,9 @@ Make Codex Companion jobs self-heal after worker loss and make the Claude Stop r - A running or queued job with a dead PID becomes `failed` with `Background worker exited before completing the job.` - Reconciliation runs before status, wait, result, cancel, task resume selection, and Stop-hook decisions. - Terminal state is claimed by an immutable per-job `jobs/.terminal.json` fence, created with exclusive filesystem creation. The first terminal writer wins; terminal fences contain only status and completion time, never request, prompt, or log content. +- Worker startup is separately claimed by `jobs/.started.json`: its first writer is either a running PID/start time or a terminal outcome. A duplicate worker that loses this initial claim does not publish or execute. - A terminal or removed job cannot be overwritten by a late worker. Corrupt or empty fences fail closed as `failed`; all control-plane reads use the fence over mutable job/index JSON. -- SessionEnd fences active jobs before removing their mutable job and index records. It may leave the small terminal fence so a late worker cannot resurrect the job. +- SessionEnd writes an immutable empty `jobs/.removed` marker before cleanup. Removal overrides every terminal or running claim, including a completion that won before SessionEnd, and may remain as a tiny orphan fence. - Failed job status output includes the stored error message. ### Stop gate diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index d0c0b1835..88d67bc65 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -974,16 +974,6 @@ async function handleCancel(argv) { const threadId = existing.threadId ?? job.threadId ?? null; const turnId = existing.turnId ?? job.turnId ?? null; - const interrupt = await interruptAppServerTurn(cwd, { threadId, turnId }); - if (interrupt.attempted) { - appendLogLine( - job.logFile, - interrupt.interrupted - ? `Requested Codex turn interrupt for ${turnId} on ${threadId}.` - : `Codex turn interrupt failed${interrupt.detail ? `: ${interrupt.detail}` : "."}` - ); - } - const completedAt = nowIso(); const nextJob = { ...job, @@ -994,13 +984,35 @@ async function handleCancel(argv) { errorMessage: "Cancelled by user." }; - terminalizeTrackedJob(workspaceRoot, { + const terminal = terminalizeTrackedJob(workspaceRoot, { ...existing, ...nextJob }, { ...nextJob, cancelledAt: completedAt }); + if (!terminal.claimed) { + const firstOutcome = terminal.job ?? job; + const payload = { + jobId: job.id, + status: firstOutcome.status, + title: job.title, + turnInterruptAttempted: false, + turnInterrupted: false + }; + outputCommandResult(payload, renderCancelReport(firstOutcome), options.json); + return; + } + + const interrupt = await interruptAppServerTurn(cwd, { threadId, turnId }); + if (interrupt.attempted) { + appendLogLine( + job.logFile, + interrupt.interrupted + ? `Requested Codex turn interrupt for ${turnId} on ${threadId}.` + : `Codex turn interrupt failed${interrupt.detail ? `: ${interrupt.detail}` : "."}` + ); + } terminateProcessTree(job.pid ?? Number.NaN); appendLogLine(job.logFile, "Cancelled by user."); diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 4a312756c..c46b7435c 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -101,7 +101,7 @@ export function createJobProgressUpdater(workspaceRoot, jobId) { } const jobFile = resolveJobFile(workspaceRoot, jobId); - if (readTerminalFence(workspaceRoot, jobId) || !fs.existsSync(jobFile)) { + if (isJobRemoved(workspaceRoot, jobId) || readTerminalFence(workspaceRoot, jobId) || !fs.existsSync(jobFile)) { return; } @@ -110,7 +110,6 @@ export function createJobProgressUpdater(workspaceRoot, jobId) { ...storedJob, ...patch }); - upsertJob(workspaceRoot, patch); }; } @@ -145,6 +144,22 @@ function resolveTerminalFenceFile(workspaceRoot, jobId) { return resolveJobFile(workspaceRoot, jobId).replace(/\.json$/, ".terminal.json"); } +function resolveInitialClaimFile(workspaceRoot, jobId) { + return resolveJobFile(workspaceRoot, jobId).replace(/\.json$/, ".started.json"); +} + +function resolveRemovedFenceFile(workspaceRoot, jobId) { + return resolveJobFile(workspaceRoot, jobId).replace(/\.json$/, ".removed"); +} + +function isJobRemoved(workspaceRoot, jobId) { + return fs.existsSync(resolveRemovedFenceFile(workspaceRoot, jobId)); +} + +export function markTrackedJobRemoved(workspaceRoot, jobId) { + claimFile(resolveRemovedFenceFile(workspaceRoot, jobId), ""); +} + function terminalPhase(status) { return status === "completed" ? "done" : status; } @@ -162,13 +177,41 @@ export function readTerminalFence(workspaceRoot, jobId) { } return { status: parsed.status, - completedAt: typeof parsed.completedAt === "string" ? parsed.completedAt : null + completedAt: typeof parsed.completedAt === "string" ? parsed.completedAt : null, + removed: parsed.removed === true }; } catch { return { status: "failed", completedAt: null, corrupt: true }; } } +function readInitialClaim(workspaceRoot, jobId) { + const claimFile = resolveInitialClaimFile(workspaceRoot, jobId); + if (!fs.existsSync(claimFile)) { + return null; + } + + try { + const parsed = JSON.parse(fs.readFileSync(claimFile, "utf8")); + if (!parsed || typeof parsed !== "object") { + throw new Error("invalid initial claim"); + } + if (parsed.status === "running" && Number.isFinite(parsed.pid) && typeof parsed.startedAt === "string") { + return { status: "running", pid: parsed.pid, startedAt: parsed.startedAt }; + } + if (TERMINAL_STATUSES.has(parsed.status)) { + return { + status: parsed.status, + completedAt: typeof parsed.completedAt === "string" ? parsed.completedAt : null, + removed: parsed.removed === true + }; + } + throw new Error("invalid initial claim status"); + } catch { + return { status: "failed", completedAt: null, corrupt: true }; + } +} + function applyTerminalFence(job, fence) { if (!fence) { return job; @@ -183,32 +226,79 @@ function applyTerminalFence(job, fence) { }; } -function claimTerminalFence(workspaceRoot, jobId, status, completedAt) { - const fenceFile = resolveTerminalFenceFile(workspaceRoot, jobId); +function applyInitialClaim(job, claim) { + if (!claim) { + return job; + } + if (claim.status === "running") { + return { ...job, status: "running", phase: job.phase === "queued" ? "starting" : job.phase, pid: claim.pid, startedAt: claim.startedAt }; + } + return applyTerminalFence(job, claim); +} + +function claimFile(file, payload) { try { - const descriptor = fs.openSync(fenceFile, "wx"); + const descriptor = fs.openSync(file, "wx"); try { - fs.writeFileSync(descriptor, `${JSON.stringify({ status, completedAt })}\n`, "utf8"); + fs.writeFileSync(descriptor, `${JSON.stringify(payload)}\n`, "utf8"); } finally { fs.closeSync(descriptor); } - return { fence: { status, completedAt }, claimed: true }; + return true; } catch (error) { - if (error?.code !== "EEXIST") { - throw error; + if (error?.code === "EEXIST") { + return false; } - return { fence: readTerminalFence(workspaceRoot, jobId), claimed: false }; + throw error; + } +} + +function claimTerminalFence(workspaceRoot, jobId, status, completedAt, removed = false) { + const fenceFile = resolveTerminalFenceFile(workspaceRoot, jobId); + if (claimFile(fenceFile, { status, completedAt, ...(removed ? { removed: true } : {}) })) { + return { fence: { status, completedAt, removed }, claimed: true }; } + return { fence: readTerminalFence(workspaceRoot, jobId), claimed: false }; } export function readEffectiveStoredJob(workspaceRoot, jobId) { + if (isJobRemoved(workspaceRoot, jobId)) { + return null; + } const storedJob = readStoredJobOrNull(workspaceRoot, jobId); - return storedJob ? applyTerminalFence(storedJob, readTerminalFence(workspaceRoot, jobId)) : null; + if (!storedJob) { + return null; + } + const initial = readInitialClaim(workspaceRoot, jobId); + const fence = initial?.status === "running" ? readTerminalFence(workspaceRoot, jobId) : null; + return applyTerminalFence(applyInitialClaim(storedJob, initial), fence); } export function terminalizeTrackedJob(workspaceRoot, job, terminal) { + if (isJobRemoved(workspaceRoot, job.id)) { + return { job: null, claimed: false }; + } const completedAt = terminal.completedAt ?? nowIso(); - const { fence, claimed } = claimTerminalFence(workspaceRoot, job.id, terminal.status, completedAt); + const initial = readInitialClaim(workspaceRoot, job.id); + if (!initial) { + const claimed = claimFile(resolveInitialClaimFile(workspaceRoot, job.id), { + status: terminal.status, + completedAt + }); + const winner = claimed ? { status: terminal.status, completedAt } : readInitialClaim(workspaceRoot, job.id); + const storedJob = readStoredJobOrNull(workspaceRoot, job.id); + if (!claimed) { + return { job: storedJob ? applyTerminalFence(storedJob, winner) : null, claimed }; + } + const effectiveJob = applyTerminalFence({ ...(storedJob ?? job), ...terminal }, winner); + writeJobFile(workspaceRoot, job.id, effectiveJob); + upsertJob(workspaceRoot, effectiveJob); + return { job: effectiveJob, claimed }; + } + if (initial.status !== "running") { + return { job: applyTerminalFence(readStoredJobOrNull(workspaceRoot, job.id) ?? job, initial), claimed: false }; + } + const { fence, claimed } = claimTerminalFence(workspaceRoot, job.id, terminal.status, completedAt, terminal.removed); const storedJob = readStoredJobOrNull(workspaceRoot, job.id); const effectiveJob = applyTerminalFence({ ...(storedJob ?? job), ...terminal }, fence); @@ -235,10 +325,22 @@ export function reconcileTrackedJobs(workspaceRoot, options = {}) { const now = options.now ?? Date.now(); return listJobs(workspaceRoot).flatMap((job) => { + if (isJobRemoved(workspaceRoot, job.id)) { + return []; + } + const storedJob = readStoredJobOrNull(workspaceRoot, job.id); + if (storedJob) { + job = { ...job, ...storedJob }; + } + const initial = readInitialClaim(workspaceRoot, job.id); + if (initial && initial.status !== "running") { + return fs.existsSync(resolveJobFile(workspaceRoot, job.id)) ? [applyTerminalFence(job, initial)] : []; + } const fence = readTerminalFence(workspaceRoot, job.id); if (fence) { return fs.existsSync(resolveJobFile(workspaceRoot, job.id)) ? [applyTerminalFence(job, fence)] : []; } + job = applyInitialClaim(job, initial); if (job.status !== "queued" && job.status !== "running") { return [job]; } @@ -256,7 +358,21 @@ export function reconcileTrackedJobs(workspaceRoot, options = {}) { } export async function runTrackedJob(job, runner, options = {}) { + if (isJobRemoved(job.workspaceRoot, job.id)) { + return null; + } const storedJob = readStoredJobOrNull(job.workspaceRoot, job.id); + const initial = readInitialClaim(job.workspaceRoot, job.id); + if (initial && initial.status !== "running") { + return !storedJob ? null : applyTerminalFence(storedJob, initial); + } + if (initial?.status === "running") { + const terminal = readTerminalFence(job.workspaceRoot, job.id); + if (terminal) { + return !storedJob ? null : applyTerminalFence(storedJob, terminal); + } + return storedJob ? applyInitialClaim(storedJob, initial) : null; + } const fence = readTerminalFence(job.workspaceRoot, job.id); if (fence) { return storedJob ? applyTerminalFence(storedJob, fence) : null; @@ -268,10 +384,16 @@ export async function runTrackedJob(job, runner, options = {}) { return null; } + const startedAt = nowIso(); + const runningClaim = { status: "running", pid: process.pid, startedAt }; + if (!claimFile(resolveInitialClaimFile(job.workspaceRoot, job.id), runningClaim)) { + const winner = readInitialClaim(job.workspaceRoot, job.id); + return !storedJob ? null : applyInitialClaim(storedJob, winner); + } const runningRecord = { ...(storedJob ?? job), status: "running", - startedAt: nowIso(), + startedAt, phase: "starting", pid: process.pid, logFile: options.logFile ?? job.logFile ?? null diff --git a/plugins/codex/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs index ec4038dfc..886b67ab1 100644 --- a/plugins/codex/scripts/session-lifecycle-hook.mjs +++ b/plugins/codex/scripts/session-lifecycle-hook.mjs @@ -14,7 +14,7 @@ import { teardownBrokerSession } from "./lib/broker-lifecycle.mjs"; import { loadState, resolveStateFile, saveState } from "./lib/state.mjs"; -import { terminalizeTrackedJob } from "./lib/tracked-jobs.mjs"; +import { markTrackedJobRemoved, terminalizeTrackedJob } from "./lib/tracked-jobs.mjs"; import { TRANSCRIPT_PATH_ENV } from "./lib/claude-session-transfer.mjs"; import { resolveWorkspaceRoot } from "./lib/workspace.mjs"; @@ -58,6 +58,7 @@ function cleanupSessionJobs(cwd, sessionId) { } for (const job of removedJobs) { + markTrackedJobRemoved(workspaceRoot, job.id); const stillRunning = job.status === "queued" || job.status === "running"; if (!stillRunning) { continue; diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index e2a8b733e..3882902e0 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1036,6 +1036,39 @@ test("terminal job fence prevents a queued worker from running", async () => { assert.equal(readJobFile(resolveJobFile(workspaceRoot, job.id)).status, "queued"); }); +test("terminal initial claim prevents a late worker from publishing running state", async () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-initial-terminal", workspaceRoot, status: "queued", request: { prompt: "do not run" } }; + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + fs.writeFileSync( + resolveJobFile(workspaceRoot, job.id).replace(/\.json$/, ".started.json"), + JSON.stringify({ status: "failed", completedAt: "2026-08-19T12:00:00.000Z" }), + "utf8" + ); + + let runnerInvoked = false; + const result = await runTrackedJob(job, async () => { + runnerInvoked = true; + return { exitStatus: 0 }; + }); + + assert.equal(runnerInvoked, false); + assert.equal(result.status, "failed"); + assert.equal(readJobFile(resolveJobFile(workspaceRoot, job.id)).status, "queued"); +}); + +test("progress updates an unindexed mutable job without making it visible", () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-unindexed-progress", workspaceRoot, status: "running" }; + writeJobFile(workspaceRoot, job.id, job); + + createJobProgressUpdater(workspaceRoot, job.id)({ phase: "investigating" }); + + assert.deepEqual(listJobs(workspaceRoot), []); + assert.equal(readJobFile(resolveJobFile(workspaceRoot, job.id)).phase, "investigating"); +}); + test("terminal job reconciliation wins over late worker finalization", async () => { const workspaceRoot = makeTempDir(); const job = { id: "task-race", workspaceRoot, status: "queued" }; @@ -1081,11 +1114,7 @@ test("removed job terminal fence prevents progress and finalization from recreat }); await waitFor(() => listJobs(workspaceRoot).some((candidate) => candidate.id === job.id && candidate.status === "running")); - fs.writeFileSync( - resolveTerminalFenceFile(workspaceRoot, job.id), - JSON.stringify({ status: "cancelled", completedAt: "2026-08-19T12:00:00.000Z" }), - "utf8" - ); + fs.writeFileSync(resolveJobFile(workspaceRoot, job.id).replace(/\.json$/, ".removed"), "", "utf8"); saveState(workspaceRoot, { config: { stopReviewGate: false }, jobs: [] }); progress({ phase: "investigating" }); releaseRunner(); @@ -1964,6 +1993,54 @@ test("cancel with a job id can still target an active job from another Claude se assert.equal(state.jobs[0].status, "cancelled"); }); +test("cancel reports a completed first terminal outcome without killing the worker", async (t) => { + const workspace = makeTempDir(); + const job = { id: "task-cancel-race", status: "running", title: "Codex Task", jobClass: "task" }; + const sleeper = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { detached: true, stdio: "ignore" }); + sleeper.unref(); + t.after(() => { + try { + process.kill(-sleeper.pid, "SIGTERM"); + } catch { + process.kill(sleeper.pid, "SIGTERM"); + } + }); + const preloadFile = path.join(makeTempDir(), "complete-before-cancel-claim.cjs"); + const jobFile = resolveJobFile(workspace, job.id); + const terminalFile = jobFile.replace(/\.json$/, ".terminal.json"); + writeJobFile(workspace, job.id, { ...job, pid: sleeper.pid }); + upsertJob(workspace, { ...job, pid: sleeper.pid }); + fs.writeFileSync( + jobFile.replace(/\.json$/, ".started.json"), + JSON.stringify({ status: "running", pid: sleeper.pid, startedAt: "2026-08-19T12:00:00.000Z" }), + "utf8" + ); + fs.writeFileSync( + preloadFile, + [ + 'const fs = require("node:fs");', + "const originalOpen = fs.openSync;", + "let armed = true;", + "fs.openSync = (file, flags, ...rest) => {", + " if (armed && file === process.env.CODEX_CANCEL_RACE_FENCE && flags === \"wx\") {", + " armed = false; fs.writeFileSync(file, JSON.stringify({ status: \"completed\", completedAt: \"2026-08-19T12:01:00.000Z\" }));", + " }", + " return originalOpen(file, flags, ...rest);", + "};" + ].join("\n"), + "utf8" + ); + + const result = run("node", [SCRIPT, "cancel", job.id, "--json"], { + cwd: workspace, + env: { ...process.env, CODEX_CANCEL_RACE_FENCE: terminalFile, NODE_OPTIONS: `--require ${preloadFile}` } + }); + + assert.equal(result.status, 0, result.stderr); + assert.equal(JSON.parse(result.stdout).status, "completed"); + process.kill(sleeper.pid, 0); +}); + test("cancel sends turn interrupt to the shared app-server before killing a brokered task", async () => { const repo = makeTempDir(); const binDir = makeTempDir(); @@ -1987,8 +2064,7 @@ test("cancel sends turn interrupt to the shared app-server before killing a brok const stateDir = resolveStateDir(repo); const runningJob = await waitFor(() => { - const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); - const job = state.jobs.find((candidate) => candidate.id === jobId); + const job = readJobFile(resolveJobFile(repo, jobId)); if (job?.status === "running" && job.threadId && job.turnId) { return job; } @@ -2132,7 +2208,12 @@ test("session end fully cleans up jobs for the ending session", async (t) => { assert.equal(fs.existsSync(otherJobFile), true); assert.deepEqual( fs.readdirSync(path.dirname(otherJobFile)).sort(), - [path.basename(otherJobFile), path.basename(otherSessionLog), "review-running.terminal.json"].sort() + [ + path.basename(otherJobFile), + path.basename(otherSessionLog), + "review-completed.removed", + "review-running.removed" + ].sort() ); await waitFor(() => { From e5d1ba828ba2b34e88b579ca0212bf13a7a72fd4 Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 15:49:56 +0300 Subject: [PATCH 06/19] fix: close Codex lifecycle races --- plugins/codex/scripts/lib/render.mjs | 3 +- plugins/codex/scripts/lib/tracked-jobs.mjs | 22 ++++++--- .../codex/scripts/session-lifecycle-hook.mjs | 10 ++-- tests/runtime.test.mjs | 49 +++++++++++++++++-- 4 files changed, 69 insertions(+), 15 deletions(-) diff --git a/plugins/codex/scripts/lib/render.mjs b/plugins/codex/scripts/lib/render.mjs index 89c597ddc..e7768484a 100644 --- a/plugins/codex/scripts/lib/render.mjs +++ b/plugins/codex/scripts/lib/render.mjs @@ -449,10 +449,11 @@ export function renderStoredJobResult(job, storedJob) { } export function renderCancelReport(job) { + const outcome = job.status === "cancelled" ? `Cancelled ${job.id}.` : `${job.id} already ${job.status}.`; const lines = [ "# Codex Cancel", "", - `Cancelled ${job.id}.`, + outcome, "" ]; diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index c46b7435c..eb685ff74 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -279,7 +279,7 @@ export function terminalizeTrackedJob(workspaceRoot, job, terminal) { return { job: null, claimed: false }; } const completedAt = terminal.completedAt ?? nowIso(); - const initial = readInitialClaim(workspaceRoot, job.id); + let initial = readInitialClaim(workspaceRoot, job.id); if (!initial) { const claimed = claimFile(resolveInitialClaimFile(workspaceRoot, job.id), { status: terminal.status, @@ -287,13 +287,16 @@ export function terminalizeTrackedJob(workspaceRoot, job, terminal) { }); const winner = claimed ? { status: terminal.status, completedAt } : readInitialClaim(workspaceRoot, job.id); const storedJob = readStoredJobOrNull(workspaceRoot, job.id); - if (!claimed) { + if (!claimed && winner?.status !== "running") { return { job: storedJob ? applyTerminalFence(storedJob, winner) : null, claimed }; } - const effectiveJob = applyTerminalFence({ ...(storedJob ?? job), ...terminal }, winner); - writeJobFile(workspaceRoot, job.id, effectiveJob); - upsertJob(workspaceRoot, effectiveJob); - return { job: effectiveJob, claimed }; + if (claimed) { + const effectiveJob = applyTerminalFence({ ...(storedJob ?? job), ...terminal }, winner); + writeJobFile(workspaceRoot, job.id, effectiveJob); + upsertJob(workspaceRoot, effectiveJob); + return { job: effectiveJob, claimed }; + } + initial = winner; } if (initial.status !== "running") { return { job: applyTerminalFence(readStoredJobOrNull(workspaceRoot, job.id) ?? job, initial), claimed: false }; @@ -400,6 +403,13 @@ export async function runTrackedJob(job, runner, options = {}) { }; writeJobFile(job.workspaceRoot, job.id, runningRecord); upsertJob(job.workspaceRoot, runningRecord); + if (isJobRemoved(job.workspaceRoot, job.id)) { + return null; + } + const terminalAfterStart = readTerminalFence(job.workspaceRoot, job.id); + if (terminalAfterStart) { + return applyTerminalFence(runningRecord, terminalAfterStart); + } try { const execution = await runner(); diff --git a/plugins/codex/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs index 886b67ab1..0d7a344c0 100644 --- a/plugins/codex/scripts/session-lifecycle-hook.mjs +++ b/plugins/codex/scripts/session-lifecycle-hook.mjs @@ -14,7 +14,7 @@ import { teardownBrokerSession } from "./lib/broker-lifecycle.mjs"; import { loadState, resolveStateFile, saveState } from "./lib/state.mjs"; -import { markTrackedJobRemoved, terminalizeTrackedJob } from "./lib/tracked-jobs.mjs"; +import { markTrackedJobRemoved, readEffectiveStoredJob, terminalizeTrackedJob } from "./lib/tracked-jobs.mjs"; import { TRANSCRIPT_PATH_ENV } from "./lib/claude-session-transfer.mjs"; import { resolveWorkspaceRoot } from "./lib/workspace.mjs"; @@ -58,9 +58,10 @@ function cleanupSessionJobs(cwd, sessionId) { } for (const job of removedJobs) { - markTrackedJobRemoved(workspaceRoot, job.id); - const stillRunning = job.status === "queued" || job.status === "running"; + const effectiveJob = { ...job, ...(readEffectiveStoredJob(workspaceRoot, job.id) ?? {}) }; + const stillRunning = effectiveJob.status === "queued" || effectiveJob.status === "running"; if (!stillRunning) { + markTrackedJobRemoved(workspaceRoot, job.id); continue; } terminalizeTrackedJob(workspaceRoot, job, { @@ -70,8 +71,9 @@ function cleanupSessionJobs(cwd, sessionId) { completedAt: new Date().toISOString(), errorMessage: "Cancelled because the Claude session ended." }); + markTrackedJobRemoved(workspaceRoot, job.id); try { - terminateProcessTree(job.pid ?? Number.NaN); + terminateProcessTree(effectiveJob.pid ?? Number.NaN); } catch { // Ignore teardown failures during session shutdown. } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 3882902e0..9ee8ae8ed 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -9,7 +9,7 @@ import { buildEnv, installFakeCodex } from "./fake-codex-fixture.mjs"; import { initGitRepo, makeTempDir, run } from "./helpers.mjs"; import { loadBrokerSession, saveBrokerSession } from "../plugins/codex/scripts/lib/broker-lifecycle.mjs"; import { listJobs, readJobFile, resolveJobFile, resolveStateDir, saveState, upsertJob, writeJobFile } from "../plugins/codex/scripts/lib/state.mjs"; -import { createJobProgressUpdater, reconcileTrackedJobs, runTrackedJob } from "../plugins/codex/scripts/lib/tracked-jobs.mjs"; +import { createJobProgressUpdater, reconcileTrackedJobs, runTrackedJob, terminalizeTrackedJob } from "../plugins/codex/scripts/lib/tracked-jobs.mjs"; const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const PLUGIN_ROOT = path.join(ROOT, "plugins", "codex"); @@ -1058,6 +1058,31 @@ test("terminal initial claim prevents a late worker from publishing running stat assert.equal(readJobFile(resolveJobFile(workspaceRoot, job.id)).status, "queued"); }); +test("terminalizer that loses the initial claim to running wins the terminal fence", () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-stage-two", workspaceRoot, status: "queued" }; + const jobFile = resolveJobFile(workspaceRoot, job.id); + const initialFile = jobFile.replace(/\.json$/, ".started.json"); + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + const originalOpen = fs.openSync; + let intercepted = false; + fs.openSync = (file, flags, ...rest) => { + if (!intercepted && file === initialFile && flags === "wx") { + intercepted = true; + fs.writeFileSync(initialFile, JSON.stringify({ status: "running", pid: process.pid, startedAt: "2026-08-19T12:00:00.000Z" })); + } + return originalOpen(file, flags, ...rest); + }; + try { + const result = terminalizeTrackedJob(workspaceRoot, job, { status: "cancelled", completedAt: "2026-08-19T12:01:00.000Z" }); + assert.equal(result.claimed, true); + assert.equal(result.job.status, "cancelled"); + } finally { + fs.openSync = originalOpen; + } +}); + test("progress updates an unindexed mutable job without making it visible", () => { const workspaceRoot = makeTempDir(); const job = { id: "task-unindexed-progress", workspaceRoot, status: "running" }; @@ -2038,6 +2063,15 @@ test("cancel reports a completed first terminal outcome without killing the work assert.equal(result.status, 0, result.stderr); assert.equal(JSON.parse(result.stdout).status, "completed"); + assert.doesNotMatch(result.stdout, /Cancelled/i); + fs.unlinkSync(terminalFile); + const rendered = run("node", [SCRIPT, "cancel", job.id], { + cwd: workspace, + env: { ...process.env, CODEX_CANCEL_RACE_FENCE: terminalFile, NODE_OPTIONS: `--require ${preloadFile}` } + }); + assert.equal(rendered.status, 0, rendered.stderr); + assert.match(rendered.stdout, /already completed/i); + assert.doesNotMatch(rendered.stdout, /Cancelled/i); process.kill(sleeper.pid, 0); }); @@ -2134,6 +2168,11 @@ test("session end fully cleans up jobs for the ending session", async (t) => { }); sleeper.unref(); fs.writeFileSync(runningJobFile, JSON.stringify({ id: "review-running" }, null, 2), "utf8"); + fs.writeFileSync( + runningJobFile.replace(/\.json$/, ".started.json"), + JSON.stringify({ status: "running", pid: sleeper.pid, startedAt: "2026-08-19T12:00:00.000Z" }), + "utf8" + ); t.after(() => { try { @@ -2165,10 +2204,10 @@ test("session end fully cleans up jobs for the ending session", async (t) => { }, { id: "review-running", - status: "running", + status: "queued", title: "Codex Review", sessionId: "sess-current", - pid: sleeper.pid, + pid: null, logFile: runningLog, createdAt: "2026-03-18T15:32:00.000Z", updatedAt: "2026-03-18T15:33:00.000Z" @@ -2212,7 +2251,9 @@ test("session end fully cleans up jobs for the ending session", async (t) => { path.basename(otherJobFile), path.basename(otherSessionLog), "review-completed.removed", - "review-running.removed" + "review-running.removed", + "review-running.started.json", + "review-running.terminal.json" ].sort() ); From d2a7d11dbfdad4c631645c3e23e40368d3e1e4f4 Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 15:52:38 +0300 Subject: [PATCH 07/19] fix: preserve terminal race outcomes --- plugins/codex/scripts/lib/tracked-jobs.mjs | 10 ++++++---- tests/runtime.test.mjs | 23 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index eb685ff74..a9e69ec1a 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -288,7 +288,7 @@ export function terminalizeTrackedJob(workspaceRoot, job, terminal) { const winner = claimed ? { status: terminal.status, completedAt } : readInitialClaim(workspaceRoot, job.id); const storedJob = readStoredJobOrNull(workspaceRoot, job.id); if (!claimed && winner?.status !== "running") { - return { job: storedJob ? applyTerminalFence(storedJob, winner) : null, claimed }; + return { job: applyTerminalFence(storedJob ?? job, winner), claimed }; } if (claimed) { const effectiveJob = applyTerminalFence({ ...(storedJob ?? job), ...terminal }, winner); @@ -306,7 +306,7 @@ export function terminalizeTrackedJob(workspaceRoot, job, terminal) { const effectiveJob = applyTerminalFence({ ...(storedJob ?? job), ...terminal }, fence); if (!claimed) { - return { job: storedJob ? applyTerminalFence(storedJob, fence) : null, claimed }; + return { job: applyTerminalFence(storedJob ?? job, fence), claimed }; } writeJobFile(workspaceRoot, job.id, effectiveJob); @@ -350,10 +350,12 @@ export function reconcileTrackedJobs(workspaceRoot, options = {}) { const ageMs = now - Date.parse(job.createdAt ?? ""); if (job.status === "queued" && !Number.isFinite(job.pid) && Number.isFinite(ageMs) && ageMs >= 5000) { - return [failTrackedJob(workspaceRoot, job, "Background worker did not start within 5 seconds.")]; + const failedJob = failTrackedJob(workspaceRoot, job, "Background worker did not start within 5 seconds."); + return failedJob ? [failedJob] : []; } if (Number.isFinite(job.pid) && !isProcessAlive(job.pid, { killImpl: options.killImpl })) { - return [failTrackedJob(workspaceRoot, job, "Background worker exited before completing the job.")]; + const failedJob = failTrackedJob(workspaceRoot, job, "Background worker exited before completing the job."); + return failedJob ? [failedJob] : []; } return [job]; diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 9ee8ae8ed..742b351dd 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1083,6 +1083,29 @@ test("terminalizer that loses the initial claim to running wins the terminal fen } }); +test("terminalization and reconciliation never return null jobs after removal races", () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-mutable-gone", workspaceRoot, status: "running" }; + const jobFile = resolveJobFile(workspaceRoot, job.id); + upsertJob(workspaceRoot, job); + fs.writeFileSync( + jobFile.replace(/\.json$/, ".started.json"), + JSON.stringify({ status: "running", pid: process.pid, startedAt: "2026-08-19T12:00:00.000Z" }), + "utf8" + ); + fs.writeFileSync( + resolveTerminalFenceFile(workspaceRoot, job.id), + JSON.stringify({ status: "failed", completedAt: "2026-08-19T12:01:00.000Z" }), + "utf8" + ); + + const result = terminalizeTrackedJob(workspaceRoot, job, { status: "cancelled", completedAt: "2026-08-19T12:02:00.000Z" }); + assert.equal(result.job.status, "failed"); + + fs.writeFileSync(jobFile.replace(/\.json$/, ".removed"), "", "utf8"); + assert.deepEqual(reconcileTrackedJobs(workspaceRoot), []); +}); + test("progress updates an unindexed mutable job without making it visible", () => { const workspaceRoot = makeTempDir(); const job = { id: "task-unindexed-progress", workspaceRoot, status: "running" }; From 871eb1b81d51c3ac0c17983af06681e885e3d0d7 Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 16:02:53 +0300 Subject: [PATCH 08/19] fix: fence Codex job admission --- plugins/codex/scripts/codex-companion.mjs | 3 ++ plugins/codex/scripts/lib/tracked-jobs.mjs | 49 +++++++++++++++++++ tests/runtime.test.mjs | 56 ++++++++++++++++++++-- 3 files changed, 105 insertions(+), 3 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 88d67bc65..54665f0cb 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -662,6 +662,9 @@ async function runForegroundCommand(job, runner, options = {}) { stderr: !options.json }); const execution = await runTrackedJob(job, () => runner(progress), { logFile }); + if (!Number.isFinite(execution?.exitStatus)) { + throw new Error(`Codex job ${job.id} is ${execution?.status ?? "removed"}; no new run was started.`); + } outputResult(options.json ? execution.payload : execution.rendered, options.json); if (execution.exitStatus !== 0) { process.exitCode = execution.exitStatus; diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index a9e69ec1a..ce00aa80a 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -152,6 +152,10 @@ function resolveRemovedFenceFile(workspaceRoot, jobId) { return resolveJobFile(workspaceRoot, jobId).replace(/\.json$/, ".removed"); } +function resolveAdmissionFile(workspaceRoot, jobId) { + return resolveJobFile(workspaceRoot, jobId).replace(/\.json$/, ".admission.json"); +} + function isJobRemoved(workspaceRoot, jobId) { return fs.existsSync(resolveRemovedFenceFile(workspaceRoot, jobId)); } @@ -212,6 +216,25 @@ function readInitialClaim(workspaceRoot, jobId) { } } +function readAdmissionClaim(workspaceRoot, jobId) { + const admissionFile = resolveAdmissionFile(workspaceRoot, jobId); + if (!fs.existsSync(admissionFile)) { + return null; + } + try { + const parsed = JSON.parse(fs.readFileSync(admissionFile, "utf8")); + if (parsed?.status === "admitted") { + return { status: "admitted" }; + } + if (TERMINAL_STATUSES.has(parsed?.status)) { + return { status: parsed.status, completedAt: typeof parsed.completedAt === "string" ? parsed.completedAt : null }; + } + throw new Error("invalid admission claim"); + } catch { + return { status: "failed", completedAt: null, corrupt: true }; + } +} + function applyTerminalFence(job, fence) { if (!fence) { return job; @@ -270,6 +293,10 @@ export function readEffectiveStoredJob(workspaceRoot, jobId) { return null; } const initial = readInitialClaim(workspaceRoot, jobId); + const admission = initial?.status === "running" ? readAdmissionClaim(workspaceRoot, jobId) : null; + if (admission && admission.status !== "admitted") { + return applyTerminalFence(applyInitialClaim(storedJob, initial), admission); + } const fence = initial?.status === "running" ? readTerminalFence(workspaceRoot, jobId) : null; return applyTerminalFence(applyInitialClaim(storedJob, initial), fence); } @@ -301,6 +328,16 @@ export function terminalizeTrackedJob(workspaceRoot, job, terminal) { if (initial.status !== "running") { return { job: applyTerminalFence(readStoredJobOrNull(workspaceRoot, job.id) ?? job, initial), claimed: false }; } + const admission = readAdmissionClaim(workspaceRoot, job.id); + if (!admission) { + const claimed = claimFile(resolveAdmissionFile(workspaceRoot, job.id), { status: terminal.status, completedAt }); + const winner = claimed ? { status: terminal.status, completedAt } : readAdmissionClaim(workspaceRoot, job.id); + if (winner?.status !== "admitted") { + return { job: applyTerminalFence(readStoredJobOrNull(workspaceRoot, job.id) ?? job, winner), claimed }; + } + } else if (admission.status !== "admitted") { + return { job: applyTerminalFence(readStoredJobOrNull(workspaceRoot, job.id) ?? job, admission), claimed: false }; + } const { fence, claimed } = claimTerminalFence(workspaceRoot, job.id, terminal.status, completedAt, terminal.removed); const storedJob = readStoredJobOrNull(workspaceRoot, job.id); const effectiveJob = applyTerminalFence({ ...(storedJob ?? job), ...terminal }, fence); @@ -339,6 +376,10 @@ export function reconcileTrackedJobs(workspaceRoot, options = {}) { if (initial && initial.status !== "running") { return fs.existsSync(resolveJobFile(workspaceRoot, job.id)) ? [applyTerminalFence(job, initial)] : []; } + const admission = initial?.status === "running" ? readAdmissionClaim(workspaceRoot, job.id) : null; + if (admission && admission.status !== "admitted") { + return fs.existsSync(resolveJobFile(workspaceRoot, job.id)) ? [applyTerminalFence(job, admission)] : []; + } const fence = readTerminalFence(workspaceRoot, job.id); if (fence) { return fs.existsSync(resolveJobFile(workspaceRoot, job.id)) ? [applyTerminalFence(job, fence)] : []; @@ -376,6 +417,10 @@ export async function runTrackedJob(job, runner, options = {}) { if (terminal) { return !storedJob ? null : applyTerminalFence(storedJob, terminal); } + const admission = readAdmissionClaim(job.workspaceRoot, job.id); + if (admission?.status !== "admitted") { + return storedJob ? applyTerminalFence(storedJob, admission) : null; + } return storedJob ? applyInitialClaim(storedJob, initial) : null; } const fence = readTerminalFence(job.workspaceRoot, job.id); @@ -412,6 +457,10 @@ export async function runTrackedJob(job, runner, options = {}) { if (terminalAfterStart) { return applyTerminalFence(runningRecord, terminalAfterStart); } + const admitted = claimFile(resolveAdmissionFile(job.workspaceRoot, job.id), { status: "admitted" }); + if (!admitted) { + return applyTerminalFence(runningRecord, readAdmissionClaim(job.workspaceRoot, job.id)); + } try { const execution = await runner(); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 742b351dd..62f0547cd 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1100,12 +1100,61 @@ test("terminalization and reconciliation never return null jobs after removal ra ); const result = terminalizeTrackedJob(workspaceRoot, job, { status: "cancelled", completedAt: "2026-08-19T12:02:00.000Z" }); - assert.equal(result.job.status, "failed"); + assert.equal(result.job.status, "cancelled"); fs.writeFileSync(jobFile.replace(/\.json$/, ".removed"), "", "utf8"); assert.deepEqual(reconcileTrackedJobs(workspaceRoot), []); }); +test("terminal admission claim after running publication prevents runner invocation", async () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-admission-terminal", workspaceRoot, status: "queued" }; + const jobFile = resolveJobFile(workspaceRoot, job.id); + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + fs.writeFileSync(jobFile.replace(/\.json$/, ".started.json"), JSON.stringify({ status: "running", pid: process.pid, startedAt: "2026-08-19T12:00:00.000Z" })); + const terminal = terminalizeTrackedJob(workspaceRoot, job, { status: "cancelled", completedAt: "2026-08-19T12:01:00.000Z" }); + let invoked = false; + const result = await runTrackedJob(job, async () => { + invoked = true; + return { exitStatus: 0 }; + }); + assert.equal(terminal.claimed, true); + assert.equal(result.status, "cancelled"); + assert.equal(invoked, false); +}); + +test("admitted running job uses the later terminal fence", () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-admitted", workspaceRoot, status: "running" }; + const jobFile = resolveJobFile(workspaceRoot, job.id); + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + fs.writeFileSync(jobFile.replace(/\.json$/, ".started.json"), JSON.stringify({ status: "running", pid: process.pid, startedAt: "2026-08-19T12:00:00.000Z" })); + fs.writeFileSync(jobFile.replace(/\.json$/, ".admission.json"), JSON.stringify({ status: "admitted" })); + const result = terminalizeTrackedJob(workspaceRoot, job, { status: "cancelled", completedAt: "2026-08-19T12:01:00.000Z" }); + assert.equal(result.claimed, true); + assert.equal(result.job.status, "cancelled"); + assert.equal(fs.existsSync(resolveTerminalFenceFile(workspaceRoot, job.id)), true); +}); + +test("corrupt admission fails closed before runner invocation", async () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-admission-corrupt", workspaceRoot, status: "queued" }; + const jobFile = resolveJobFile(workspaceRoot, job.id); + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + fs.writeFileSync(jobFile.replace(/\.json$/, ".started.json"), JSON.stringify({ status: "running", pid: process.pid, startedAt: "2026-08-19T12:00:00.000Z" })); + fs.writeFileSync(jobFile.replace(/\.json$/, ".admission.json"), "{"); + let invoked = false; + const result = await runTrackedJob(job, async () => { + invoked = true; + return { exitStatus: 0 }; + }); + assert.equal(result.status, "failed"); + assert.equal(invoked, false); +}); + test("progress updates an unindexed mutable job without making it visible", () => { const workspaceRoot = makeTempDir(); const job = { id: "task-unindexed-progress", workspaceRoot, status: "running" }; @@ -2063,6 +2112,7 @@ test("cancel reports a completed first terminal outcome without killing the work JSON.stringify({ status: "running", pid: sleeper.pid, startedAt: "2026-08-19T12:00:00.000Z" }), "utf8" ); + fs.writeFileSync(jobFile.replace(/\.json$/, ".admission.json"), JSON.stringify({ status: "admitted" }), "utf8"); fs.writeFileSync( preloadFile, [ @@ -2274,9 +2324,9 @@ test("session end fully cleans up jobs for the ending session", async (t) => { path.basename(otherJobFile), path.basename(otherSessionLog), "review-completed.removed", + "review-running.admission.json", "review-running.removed", - "review-running.started.json", - "review-running.terminal.json" + "review-running.started.json" ].sort() ); From b58be3a5979010282a0075b3dd9e80f49edeeeb1 Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 16:05:18 +0300 Subject: [PATCH 09/19] fix: return typed Codex lifecycle outcomes --- plugins/codex/scripts/lib/tracked-jobs.mjs | 28 ++++++++++++++-------- tests/tracked-jobs.test.mjs | 23 ++++++++++++++++-- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index ce00aa80a..6c21151be 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -249,6 +249,14 @@ function applyTerminalFence(job, fence) { }; } +function removedLifecycleRecord(job) { + return { ...job, status: "cancelled", phase: "cancelled", pid: null, removed: true }; +} + +function missingTrackedJobRecord(job) { + return { ...job, status: "failed", phase: "failed", pid: null, errorMessage: "Tracked job record is missing." }; +} + function applyInitialClaim(job, claim) { if (!claim) { return job; @@ -303,7 +311,7 @@ export function readEffectiveStoredJob(workspaceRoot, jobId) { export function terminalizeTrackedJob(workspaceRoot, job, terminal) { if (isJobRemoved(workspaceRoot, job.id)) { - return { job: null, claimed: false }; + return { job: removedLifecycleRecord(job), claimed: false }; } const completedAt = terminal.completedAt ?? nowIso(); let initial = readInitialClaim(workspaceRoot, job.id); @@ -405,40 +413,40 @@ export function reconcileTrackedJobs(workspaceRoot, options = {}) { export async function runTrackedJob(job, runner, options = {}) { if (isJobRemoved(job.workspaceRoot, job.id)) { - return null; + return removedLifecycleRecord(job); } const storedJob = readStoredJobOrNull(job.workspaceRoot, job.id); const initial = readInitialClaim(job.workspaceRoot, job.id); if (initial && initial.status !== "running") { - return !storedJob ? null : applyTerminalFence(storedJob, initial); + return applyTerminalFence(storedJob ?? job, initial); } if (initial?.status === "running") { const terminal = readTerminalFence(job.workspaceRoot, job.id); if (terminal) { - return !storedJob ? null : applyTerminalFence(storedJob, terminal); + return applyTerminalFence(storedJob ?? job, terminal); } const admission = readAdmissionClaim(job.workspaceRoot, job.id); if (admission?.status !== "admitted") { - return storedJob ? applyTerminalFence(storedJob, admission) : null; + return applyTerminalFence(storedJob ?? job, admission); } - return storedJob ? applyInitialClaim(storedJob, initial) : null; + return applyInitialClaim(storedJob ?? job, initial); } const fence = readTerminalFence(job.workspaceRoot, job.id); if (fence) { - return storedJob ? applyTerminalFence(storedJob, fence) : null; + return applyTerminalFence(storedJob ?? job, fence); } if (storedJob && storedJob.status !== "queued") { return storedJob; } if (!storedJob && job.request) { - return null; + return missingTrackedJobRecord(job); } const startedAt = nowIso(); const runningClaim = { status: "running", pid: process.pid, startedAt }; if (!claimFile(resolveInitialClaimFile(job.workspaceRoot, job.id), runningClaim)) { const winner = readInitialClaim(job.workspaceRoot, job.id); - return !storedJob ? null : applyInitialClaim(storedJob, winner); + return applyInitialClaim(storedJob ?? job, winner); } const runningRecord = { ...(storedJob ?? job), @@ -451,7 +459,7 @@ export async function runTrackedJob(job, runner, options = {}) { writeJobFile(job.workspaceRoot, job.id, runningRecord); upsertJob(job.workspaceRoot, runningRecord); if (isJobRemoved(job.workspaceRoot, job.id)) { - return null; + return removedLifecycleRecord(runningRecord); } const terminalAfterStart = readTerminalFence(job.workspaceRoot, job.id); if (terminalAfterStart) { diff --git a/tests/tracked-jobs.test.mjs b/tests/tracked-jobs.test.mjs index df0d2a081..6bdcd6b25 100644 --- a/tests/tracked-jobs.test.mjs +++ b/tests/tracked-jobs.test.mjs @@ -36,7 +36,7 @@ test("runTrackedJob does not resurrect a terminal persisted job", async () => { assert.deepEqual(readJobFile(resolveJobFile(workspaceRoot, job.id)), terminalJob); }); -test("runTrackedJob does not recreate a removed background job", async () => { +test("runTrackedJob returns a failed record for missing background state", async () => { const workspaceRoot = makeTempDir(); const job = { id: "task-removed", @@ -53,6 +53,25 @@ test("runTrackedJob does not recreate a removed background job", async () => { }); assert.equal(runnerInvoked, false); - assert.equal(result, null); + assert.equal(result.status, "failed"); + assert.match(result.errorMessage, /record is missing/i); assert.equal(fs.existsSync(jobFile), false); }); + +test("runTrackedJob returns a cancelled record after removal", async () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-removed-fence", workspaceRoot, status: "queued", request: { prompt: "do not run" } }; + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + fs.writeFileSync(resolveJobFile(workspaceRoot, job.id).replace(/\.json$/, ".removed"), "", "utf8"); + + let runnerInvoked = false; + const result = await runTrackedJob(job, async () => { + runnerInvoked = true; + return { exitStatus: 0 }; + }); + + assert.equal(runnerInvoked, false); + assert.equal(result.status, "cancelled"); + assert.equal(result.removed, true); +}); From 10f9a9c648f97b9717e20a0565f32c98364013ac Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 16:16:23 +0300 Subject: [PATCH 10/19] fix: make stop review gate idempotent --- plugins/codex/scripts/codex-companion.mjs | 22 +++- plugins/codex/scripts/lib/tracked-jobs.mjs | 10 ++ .../codex/scripts/stop-review-gate-hook.mjs | 62 ++++++++-- tests/runtime.test.mjs | 110 +++++++++++++++++- 4 files changed, 185 insertions(+), 19 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 54665f0cb..5eeef7762 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -47,6 +47,7 @@ import { createJobProgressUpdater, createJobRecord, createProgressReporter, + GATE_KEY_ENV, nowIso, reconcileTrackedJobs, runTrackedJob, @@ -565,16 +566,17 @@ function getJobKindLabel(kind, jobClass) { return jobClass === "review" ? "review" : "rescue"; } -function createCompanionJob({ prefix, kind, title, workspaceRoot, jobClass, summary, write = false }) { +function createCompanionJob({ prefix, kind, title, workspaceRoot, jobClass, summary, write = false, id, gateKey }) { return createJobRecord({ - id: generateJobId(prefix), + id: id ?? generateJobId(prefix), kind, kindLabel: getJobKindLabel(kind, jobClass), title, workspaceRoot, jobClass, summary, - write + write, + ...(gateKey ? { gateKey } : {}) }); } @@ -590,7 +592,7 @@ function createTrackedProgress(job, options = {}) { }; } -function buildTaskJob(workspaceRoot, taskMetadata, write) { +function buildTaskJob(workspaceRoot, taskMetadata, write, gateKey = null) { return createCompanionJob({ prefix: "task", kind: "task", @@ -598,7 +600,8 @@ function buildTaskJob(workspaceRoot, taskMetadata, write) { workspaceRoot, jobClass: "task", summary: taskMetadata.summary, - write + write, + ...(gateKey ? { id: `gate-${gateKey}`, gateKey } : {}) }); } @@ -663,6 +666,10 @@ async function runForegroundCommand(job, runner, options = {}) { }); const execution = await runTrackedJob(job, () => runner(progress), { logFile }); if (!Number.isFinite(execution?.exitStatus)) { + if (job.gateKey) { + outputResult({ jobId: job.id, status: execution?.status ?? "failed", gateDuplicate: true }, true); + return execution; + } throw new Error(`Codex job ${job.id} is ${execution?.status ?? "removed"}; no new run was started.`); } outputResult(options.json ? execution.payload : execution.rendered, options.json); @@ -808,7 +815,10 @@ async function handleTask(argv) { return; } - const job = buildTaskJob(workspaceRoot, taskMetadata, write); + const gateKey = !resumeLast && taskMetadata.title === "Codex Stop Gate Review" && /^[a-f0-9]{64}$/.test(process.env[GATE_KEY_ENV] ?? "") + ? process.env[GATE_KEY_ENV] + : null; + const job = buildTaskJob(workspaceRoot, taskMetadata, write, gateKey); await runForegroundCommand( job, (progress) => diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 6c21151be..c7ec93d97 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -5,6 +5,7 @@ import { isProcessAlive } from "./process.mjs"; import { listJobs, readJobFile, resolveJobFile, resolveJobLogFile, upsertJob, writeJobFile } from "./state.mjs"; export const SESSION_ID_ENV = "CODEX_COMPANION_SESSION_ID"; +export const GATE_KEY_ENV = "CODEX_COMPANION_GATE_KEY"; export function nowIso() { return new Date().toISOString(); @@ -421,6 +422,15 @@ export async function runTrackedJob(job, runner, options = {}) { return applyTerminalFence(storedJob ?? job, initial); } if (initial?.status === "running") { + if (job.gateKey && !storedJob && !isProcessAlive(initial.pid)) { + return terminalizeTrackedJob(job.workspaceRoot, job, { + status: "failed", + phase: "failed", + errorMessage: "Stop-gate worker exited before publishing its job record.", + pid: null, + completedAt: nowIso() + }).job; + } const terminal = readTerminalFence(job.workspaceRoot, job.id); if (terminal) { return applyTerminalFence(storedJob ?? job, terminal); diff --git a/plugins/codex/scripts/stop-review-gate-hook.mjs b/plugins/codex/scripts/stop-review-gate-hook.mjs index 2c5eb44f4..aa9104f23 100644 --- a/plugins/codex/scripts/stop-review-gate-hook.mjs +++ b/plugins/codex/scripts/stop-review-gate-hook.mjs @@ -4,13 +4,14 @@ import fs from "node:fs"; import process from "node:process"; import path from "node:path"; import { spawnSync } from "node:child_process"; +import { createHash } from "node:crypto"; import { fileURLToPath } from "node:url"; import { getCodexAvailability } from "./lib/codex.mjs"; import { loadPromptTemplate, interpolateTemplate } from "./lib/prompts.mjs"; import { getConfig } from "./lib/state.mjs"; import { sortJobsNewestFirst } from "./lib/job-control.mjs"; -import { reconcileTrackedJobs, SESSION_ID_ENV } from "./lib/tracked-jobs.mjs"; +import { GATE_KEY_ENV, reconcileTrackedJobs, SESSION_ID_ENV } from "./lib/tracked-jobs.mjs"; import { resolveWorkspaceRoot } from "./lib/workspace.mjs"; const STOP_REVIEW_TIMEOUT_MS = 15 * 60 * 1000; @@ -46,7 +47,7 @@ function filterJobsForCurrentSession(jobs, input = {}) { } function buildStopReviewPrompt(input = {}) { - const lastAssistantMessage = String(input.last_assistant_message ?? "").trim(); + const lastAssistantMessage = String(input.last_assistant_message ?? ""); const template = loadPromptTemplate(ROOT_DIR, "stop-review-gate"); const claudeResponseBlock = lastAssistantMessage ? ["Previous Claude response:", lastAssistantMessage].join("\n") @@ -56,6 +57,38 @@ function buildStopReviewPrompt(input = {}) { }); } +function getGateKey(input = {}) { + const sessionId = input.session_id || process.env[SESSION_ID_ENV] || ""; + const lastAssistantMessage = String(input.last_assistant_message ?? ""); + if (!sessionId || !lastAssistantMessage) { + return null; + } + return createHash("sha256").update(`${sessionId}\0${lastAssistantMessage}`).digest("hex"); +} + +function gateJobNote(job) { + const commands = `Check /codex:status ${job.id} and use /codex:cancel ${job.id} if you want to stop it.`; + if (job.status === "queued" || job.status === "running") { + return `The stop-time Codex review is already ${job.status} as ${job.id}. ${commands}`; + } + return `The prior stop-time Codex review ${job.id} is ${job.status}; it will not be rerun automatically. ${commands}`; +} + +function getGateJob(jobs, gateKey) { + return gateKey ? jobs.find((job) => job.gateKey === gateKey) ?? null : null; +} + +function parseStoredGateReview(job) { + const rawOutput = job?.result?.rawOutput; + if (typeof rawOutput !== "string") { + return { + ok: false, + reason: `The completed stop-time Codex review ${job.id} has missing or corrupt cached output and will not be rerun automatically. Run /codex:review --wait manually or bypass the gate.` + }; + } + return parseStopReviewOutput(rawOutput); +} + function buildSetupNote(cwd) { const availability = getCodexAvailability(cwd); if (availability.available) { @@ -95,12 +128,13 @@ function parseStopReviewOutput(rawOutput) { }; } -function runStopReview(cwd, input = {}) { +function runStopReview(cwd, input = {}, gateKey = null) { const scriptPath = path.join(SCRIPT_DIR, "codex-companion.mjs"); const prompt = buildStopReviewPrompt(input); const childEnv = { ...process.env, - ...(input.session_id ? { [SESSION_ID_ENV]: input.session_id } : {}) + ...(input.session_id ? { [SESSION_ID_ENV]: input.session_id } : {}), + ...(gateKey ? { [GATE_KEY_ENV]: gateKey } : {}) }; const result = spawnSync(process.execPath, [scriptPath, "task", "--json", prompt], { cwd, @@ -129,6 +163,9 @@ function runStopReview(cwd, input = {}) { try { const payload = JSON.parse(result.stdout); + if (payload?.gateDuplicate) { + return { ok: false, reason: gateJobNote({ id: payload.jobId, status: payload.status }) }; + } return parseStopReviewOutput(payload?.rawOutput); } catch { return { @@ -158,12 +195,23 @@ function main() { const setupNote = buildSetupNote(cwd); if (setupNote) { - logNote(setupNote); - logNote(runningTaskNote); + emitDecision({ decision: "block", reason: setupNote }); + return; + } + + const gateKey = getGateKey(input); + const cachedJob = getGateJob(jobs, gateKey); + if (cachedJob) { + const review = cachedJob.status === "completed" ? parseStoredGateReview(cachedJob) : { ok: false, reason: gateJobNote(cachedJob) }; + if (!review.ok) { + emitDecision({ decision: "block", reason: runningTaskNote ? `${runningTaskNote} ${review.reason}` : review.reason }); + } else { + logNote(runningTaskNote); + } return; } - const review = runStopReview(cwd, input); + const review = runStopReview(cwd, input, gateKey); if (!review.ok) { emitDecision({ decision: "block", diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 62f0547cd..6486c94f8 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -33,6 +33,19 @@ function resolveTerminalFenceFile(workspaceRoot, jobId) { return resolveJobFile(workspaceRoot, jobId).replace(/\.json$/, ".terminal.json"); } +function runHookAsync(cwd, env, input) { + return new Promise((resolve, reject) => { + const child = spawn(process.execPath, [STOP_HOOK], { cwd, env, stdio: ["pipe", "pipe", "pipe"] }); + let stdout = ""; + let stderr = ""; + child.stdout.on("data", (chunk) => { stdout += chunk; }); + child.stderr.on("data", (chunk) => { stderr += chunk; }); + child.once("error", reject); + child.once("close", (status) => resolve({ status, stdout, stderr })); + child.stdin.end(input); + }); +} + test("setup reports ready when fake codex is installed and authenticated", () => { const binDir = makeTempDir(); installFakeCodex(binDir); @@ -2401,6 +2414,92 @@ test("stop hook runs a stop-time review task and blocks on findings when the rev assert.match(status.stdout, /Codex Stop Gate Review/); }); +test("stop hook reuses the completed review for the same Claude response", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + const env = buildEnv(binDir); + const input = JSON.stringify({ + cwd: repo, + session_id: "sess-stop-idempotent", + last_assistant_message: "I completed the exact same change." + }); + + const setup = run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { cwd: repo, env }); + assert.equal(setup.status, 0, setup.stderr); + + const first = run("node", [STOP_HOOK], { cwd: repo, env, input }); + assert.equal(first.status, 0, first.stderr); + const nextTurnId = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId; + + const second = run("node", [STOP_HOOK], { cwd: repo, env, input }); + assert.equal(second.status, 0, second.stderr); + assert.deepEqual(JSON.parse(second.stdout), JSON.parse(first.stdout)); + assert.equal(JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId, nextTurnId); +}); + +test("stop hook keeps cached ALLOW decisions silent", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "adversarial-clean"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + const env = buildEnv(binDir); + const input = JSON.stringify({ + cwd: repo, + session_id: "sess-stop-cached-allow", + last_assistant_message: "I completed a clean change." + }); + + const setup = run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { cwd: repo, env }); + assert.equal(setup.status, 0, setup.stderr); + const first = run("node", [STOP_HOOK], { cwd: repo, env, input }); + assert.equal(first.status, 0, first.stderr); + assert.equal(first.stdout.trim(), ""); + const nextTurnId = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId; + const second = run("node", [STOP_HOOK], { cwd: repo, env, input }); + assert.equal(second.status, 0, second.stderr); + assert.equal(second.stdout.trim(), ""); + assert.equal(JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId, nextTurnId); +}); + +test("concurrent stop hooks start exactly one review for the same Claude response", async () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + const env = buildEnv(binDir); + const input = JSON.stringify({ + cwd: repo, + session_id: "sess-stop-concurrent", + last_assistant_message: "I completed this concurrent change." + }); + + const setup = run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { cwd: repo, env }); + assert.equal(setup.status, 0, setup.stderr); + + const [first, second] = await Promise.all([runHookAsync(repo, env, input), runHookAsync(repo, env, input)]); + assert.equal(first.status, 0, first.stderr); + assert.equal(second.status, 0, second.stderr); + assert.equal(JSON.parse(first.stdout).decision, "block"); + assert.equal(JSON.parse(second.stdout).decision, "block"); + assert.equal(JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId, 2); + const jobs = JSON.parse(fs.readFileSync(path.join(resolveStateDir(repo), "state.json"), "utf8")).jobs; + assert.equal(jobs.filter((job) => job.title === "Codex Stop Gate Review").length, 1); +}); + test("stop hook logs running tasks to stderr without blocking when the review gate is disabled", () => { const repo = makeTempDir(); initGitRepo(repo); @@ -2483,7 +2582,7 @@ test("stop hook allows the stop when the review gate is enabled and the stop-tim assert.equal(allowed.stdout.trim(), ""); }); -test("stop hook does not block when Codex is unavailable even if the review gate is enabled", () => { +test("stop hook blocks when Codex is unavailable and the review gate is enabled", () => { const repo = makeTempDir(); initGitRepo(repo); fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); @@ -2495,7 +2594,7 @@ test("stop hook does not block when Codex is unavailable even if the review gate }); assert.equal(setup.status, 0, setup.stderr); - const allowed = run(process.execPath, [STOP_HOOK], { + const blocked = run(process.execPath, [STOP_HOOK], { cwd: repo, env: { ...process.env, @@ -2504,10 +2603,9 @@ test("stop hook does not block when Codex is unavailable even if the review gate input: JSON.stringify({ cwd: repo }) }); - assert.equal(allowed.status, 0, allowed.stderr); - assert.equal(allowed.stdout.trim(), ""); - assert.match(allowed.stderr, /Codex is not set up for the review gate/i); - assert.match(allowed.stderr, /Run \/codex:setup/i); + assert.equal(blocked.status, 0, blocked.stderr); + assert.equal(JSON.parse(blocked.stdout).decision, "block"); + assert.match(JSON.parse(blocked.stdout).reason, /not set up/i); }); test("stop hook runs the actual task when auth status looks stale", () => { From 586c816811f346564d9101998461210ea25af0b6 Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 16:16:43 +0300 Subject: [PATCH 11/19] docs: record stop gate verification --- .superpowers/sdd/task-3-report.md | 43 +++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 .superpowers/sdd/task-3-report.md diff --git a/.superpowers/sdd/task-3-report.md b/.superpowers/sdd/task-3-report.md new file mode 100644 index 000000000..a05c9f436 --- /dev/null +++ b/.superpowers/sdd/task-3-report.md @@ -0,0 +1,43 @@ +# Task 3 Report + +## RED + +`node --test --test-name-pattern="unavailable|same Claude response" tests/runtime.test.mjs` + +Observed the expected failures: unavailable Codex produced empty stdout, and a second identical Stop review started a new fake Codex turn. + +## GREEN + +`node --test --test-name-pattern="stop hook" tests/runtime.test.mjs` + +Passed: 8 Stop-hook tests, including cached `ALLOW` silence and two hooks started concurrently before either was awaited. + +`node --test --test-reporter=dot tests/process.test.mjs tests/tracked-jobs.test.mjs tests/runtime.test.mjs tests/state.test.mjs` + +Passed: focused process, lifecycle, runtime, and state suites. + +`git diff --check` + +Passed with no whitespace errors. + +## Files + +- `plugins/codex/scripts/lib/tracked-jobs.mjs` +- `plugins/codex/scripts/codex-companion.mjs` +- `plugins/codex/scripts/stop-review-gate-hook.mjs` +- `tests/runtime.test.mjs` + +## Self-review + +- Gate keys are SHA-256 hashes of the raw session/message pair; assistant-message content is not persisted. +- `gate-` makes concurrent children compete on the existing immutable initial claim. +- Completed results use stored `result.rawOutput`; active, failed, cancelled, missing, or corrupt cached results block and never rerun. +- A dead initial keyed claimant without mutable state is materialized as failed rather than retried. + +## Concerns + +None. The fake Codex fixture is not invoked by the losing concurrent hook, so its JSON state file has no concurrent writer in this test. + +## SHA + +Implementation commit: `10f9a9c648f97b9717e20a0565f32c98364013ac` From 809aa8ec586d8191cb025db4213c708bc2f2ed99 Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 16:25:01 +0300 Subject: [PATCH 12/19] fix: preserve stop gate cache and logs --- .superpowers/sdd/task-3-report.md | 43 -------- plugins/codex/scripts/lib/tracked-jobs.mjs | 12 ++- .../codex/scripts/stop-review-gate-hook.mjs | 12 +-- tests/runtime.test.mjs | 100 +++++++++++++++++- 4 files changed, 114 insertions(+), 53 deletions(-) delete mode 100644 .superpowers/sdd/task-3-report.md diff --git a/.superpowers/sdd/task-3-report.md b/.superpowers/sdd/task-3-report.md deleted file mode 100644 index a05c9f436..000000000 --- a/.superpowers/sdd/task-3-report.md +++ /dev/null @@ -1,43 +0,0 @@ -# Task 3 Report - -## RED - -`node --test --test-name-pattern="unavailable|same Claude response" tests/runtime.test.mjs` - -Observed the expected failures: unavailable Codex produced empty stdout, and a second identical Stop review started a new fake Codex turn. - -## GREEN - -`node --test --test-name-pattern="stop hook" tests/runtime.test.mjs` - -Passed: 8 Stop-hook tests, including cached `ALLOW` silence and two hooks started concurrently before either was awaited. - -`node --test --test-reporter=dot tests/process.test.mjs tests/tracked-jobs.test.mjs tests/runtime.test.mjs tests/state.test.mjs` - -Passed: focused process, lifecycle, runtime, and state suites. - -`git diff --check` - -Passed with no whitespace errors. - -## Files - -- `plugins/codex/scripts/lib/tracked-jobs.mjs` -- `plugins/codex/scripts/codex-companion.mjs` -- `plugins/codex/scripts/stop-review-gate-hook.mjs` -- `tests/runtime.test.mjs` - -## Self-review - -- Gate keys are SHA-256 hashes of the raw session/message pair; assistant-message content is not persisted. -- `gate-` makes concurrent children compete on the existing immutable initial claim. -- Completed results use stored `result.rawOutput`; active, failed, cancelled, missing, or corrupt cached results block and never rerun. -- A dead initial keyed claimant without mutable state is materialized as failed rather than retried. - -## Concerns - -None. The fake Codex fixture is not invoked by the losing concurrent hook, so its JSON state file has no concurrent writer in this test. - -## SHA - -Implementation commit: `10f9a9c648f97b9717e20a0565f32c98364013ac` diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index c7ec93d97..635e13d72 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -52,8 +52,16 @@ export function appendLogBlock(logFile, title, body) { export function createJobLogFile(workspaceRoot, jobId, title) { const logFile = resolveJobLogFile(workspaceRoot, jobId); - fs.writeFileSync(logFile, "", "utf8"); - if (title) { + let created = false; + try { + fs.closeSync(fs.openSync(logFile, "wx")); + created = true; + } catch (error) { + if (error?.code !== "EEXIST") { + throw error; + } + } + if (created && title) { appendLogLine(logFile, `Starting ${title}.`); } return logFile; diff --git a/plugins/codex/scripts/stop-review-gate-hook.mjs b/plugins/codex/scripts/stop-review-gate-hook.mjs index aa9104f23..c91979113 100644 --- a/plugins/codex/scripts/stop-review-gate-hook.mjs +++ b/plugins/codex/scripts/stop-review-gate-hook.mjs @@ -193,12 +193,6 @@ function main() { return; } - const setupNote = buildSetupNote(cwd); - if (setupNote) { - emitDecision({ decision: "block", reason: setupNote }); - return; - } - const gateKey = getGateKey(input); const cachedJob = getGateJob(jobs, gateKey); if (cachedJob) { @@ -211,6 +205,12 @@ function main() { return; } + const setupNote = buildSetupNote(cwd); + if (setupNote) { + emitDecision({ decision: "block", reason: setupNote }); + return; + } + const review = runStopReview(cwd, input, gateKey); if (!review.ok) { emitDecision({ diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 6486c94f8..93c0f06c3 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1,4 +1,5 @@ import fs from "node:fs"; +import { createHash } from "node:crypto"; import path from "node:path"; import test from "node:test"; import assert from "node:assert/strict"; @@ -8,7 +9,7 @@ import { fileURLToPath } from "node:url"; import { buildEnv, installFakeCodex } from "./fake-codex-fixture.mjs"; import { initGitRepo, makeTempDir, run } from "./helpers.mjs"; import { loadBrokerSession, saveBrokerSession } from "../plugins/codex/scripts/lib/broker-lifecycle.mjs"; -import { listJobs, readJobFile, resolveJobFile, resolveStateDir, saveState, upsertJob, writeJobFile } from "../plugins/codex/scripts/lib/state.mjs"; +import { listJobs, readJobFile, resolveJobFile, resolveJobLogFile, resolveStateDir, saveState, upsertJob, writeJobFile } from "../plugins/codex/scripts/lib/state.mjs"; import { createJobProgressUpdater, reconcileTrackedJobs, runTrackedJob, terminalizeTrackedJob } from "../plugins/codex/scripts/lib/tracked-jobs.mjs"; const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); @@ -2465,12 +2466,82 @@ test("stop hook keeps cached ALLOW decisions silent", () => { assert.equal(first.status, 0, first.stderr); assert.equal(first.stdout.trim(), ""); const nextTurnId = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId; - const second = run("node", [STOP_HOOK], { cwd: repo, env, input }); + const second = run(process.execPath, [STOP_HOOK], { cwd: repo, env: { ...env, PATH: "" }, input }); assert.equal(second.status, 0, second.stderr); assert.equal(second.stdout.trim(), ""); assert.equal(JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId, nextTurnId); }); +test("stop hook reuses a cached BLOCK before checking Codex availability", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir); + initGitRepo(repo); + const env = buildEnv(binDir); + const input = JSON.stringify({ cwd: repo, session_id: "sess-stop-cached-block", last_assistant_message: "A blocked change." }); + const setup = run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { cwd: repo, env }); + assert.equal(setup.status, 0, setup.stderr); + const first = run("node", [STOP_HOOK], { cwd: repo, env, input }); + assert.equal(first.status, 0, first.stderr); + const second = run(process.execPath, [STOP_HOOK], { cwd: repo, env: { ...env, PATH: "" }, input }); + assert.equal(second.status, 0, second.stderr); + assert.deepEqual(JSON.parse(second.stdout), JSON.parse(first.stdout)); +}); + +test("stop hook blocks every matching non-reusable gate job without starting a turn", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + const env = buildEnv(binDir); + const sessionId = "sess-stop-lifecycle"; + const message = "Exact lifecycle response."; + const gateKey = createHash("sha256").update(`${sessionId}\0${message}`).digest("hex"); + const jobId = `gate-${gateKey}`; + const setup = run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { cwd: repo, env }); + assert.equal(setup.status, 0, setup.stderr); + + for (const [status, cachedResult] of [ + ["queued", undefined], + ["running", undefined], + ["failed", undefined], + ["cancelled", undefined], + ["completed", undefined], + ["completed", { rawOutput: null }], + ["completed", "corrupt"] + ]) { + saveState(repo, { + version: 1, + config: { stopReviewGate: true }, + jobs: [{ id: jobId, gateKey, sessionId, status, title: "Codex Stop Gate Review", ...(cachedResult === undefined ? {} : { result: cachedResult }) }] + }); + const beforeTurns = fs.existsSync(fakeStatePath) ? JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId : 1; + const hookResult = run("node", [STOP_HOOK], { cwd: repo, env, input: JSON.stringify({ cwd: repo, session_id: sessionId, last_assistant_message: message }) }); + assert.equal(hookResult.status, 0, hookResult.stderr); + assert.equal(JSON.parse(hookResult.stdout).decision, "block"); + assert.match(JSON.parse(hookResult.stdout).reason, new RegExp(jobId)); + const afterTurns = fs.existsSync(fakeStatePath) ? JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId : 1; + assert.equal(afterTurns, beforeTurns); + } +}); + +test("stop hook without an assistant message runs fresh reviews", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + const env = buildEnv(binDir); + const input = JSON.stringify({ cwd: repo, session_id: "sess-stop-empty-message", last_assistant_message: "" }); + const setup = run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { cwd: repo, env }); + assert.equal(setup.status, 0, setup.stderr); + assert.equal(run("node", [STOP_HOOK], { cwd: repo, env, input }).status, 0); + const nextTurnId = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId; + assert.equal(run("node", [STOP_HOOK], { cwd: repo, env, input }).status, 0); + assert.equal(JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId, nextTurnId + 1); +}); + test("concurrent stop hooks start exactly one review for the same Claude response", async () => { const repo = makeTempDir(); const binDir = makeTempDir(); @@ -2500,6 +2571,31 @@ test("concurrent stop hooks start exactly one review for the same Claude respons assert.equal(jobs.filter((job) => job.title === "Codex Stop Gate Review").length, 1); }); +test("concurrent stop hooks do not truncate or duplicate the winner log", async () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "slow-task"); + initGitRepo(repo); + const env = buildEnv(binDir); + const sessionId = "sess-stop-log-race"; + const message = "Concurrent log response."; + const gateKey = createHash("sha256").update(`${sessionId}\0${message}`).digest("hex"); + const logFile = resolveJobLogFile(repo, `gate-${gateKey}`); + const input = JSON.stringify({ cwd: repo, session_id: sessionId, last_assistant_message: message }); + const setup = run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { cwd: repo, env }); + assert.equal(setup.status, 0, setup.stderr); + + const first = runHookAsync(repo, env, input); + await waitFor(() => fs.existsSync(logFile) && fs.readFileSync(logFile, "utf8").includes("Starting Codex Stop Gate Review.")); + fs.appendFileSync(logFile, "winner-marker\n", "utf8"); + const second = runHookAsync(repo, env, input); + await Promise.all([first, second]); + + const log = fs.readFileSync(logFile, "utf8"); + assert.match(log, /winner-marker/); + assert.equal((log.match(/Starting Codex Stop Gate Review\./g) ?? []).length, 1); +}); + test("stop hook logs running tasks to stderr without blocking when the review gate is disabled", () => { const repo = makeTempDir(); initGitRepo(repo); From 563914aa2b685243fc600752e650e826c4be1aa8 Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 16:33:22 +0300 Subject: [PATCH 13/19] fix: make Codex state persistence fail closed --- plugins/codex/scripts/lib/state.mjs | 36 ++++++++++---- .../codex/scripts/stop-review-gate-hook.mjs | 6 ++- tests/runtime.test.mjs | 21 ++++++++ tests/state.test.mjs | 48 ++++++++++++++++++- 4 files changed, 98 insertions(+), 13 deletions(-) diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs index 2da23498f..2c6946d5e 100644 --- a/plugins/codex/scripts/lib/state.mjs +++ b/plugins/codex/scripts/lib/state.mjs @@ -1,4 +1,4 @@ -import { createHash } from "node:crypto"; +import { createHash, randomUUID } from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -56,11 +56,7 @@ export function ensureStateDir(cwd) { } export function loadState(cwd) { - const stateFile = resolveStateFile(cwd); - if (!fs.existsSync(stateFile)) { - return defaultState(); - } - + const stateFile = path.resolve(resolveStateFile(cwd)); try { const parsed = JSON.parse(fs.readFileSync(stateFile, "utf8")); return { @@ -72,8 +68,28 @@ export function loadState(cwd) { }, jobs: Array.isArray(parsed.jobs) ? parsed.jobs : [] }; - } catch { - return defaultState(); + } catch (error) { + if (error?.code === "ENOENT") { + return defaultState(); + } + const detail = error instanceof Error ? error.message : String(error); + throw new Error(`Failed to read Codex Companion state at ${stateFile}: ${detail}`, { cause: error }); + } +} + +function writeAtomicJson(filePath, value) { + const temporaryFile = `${filePath}.${process.pid}.${randomUUID()}.tmp`; + const content = `${JSON.stringify(value, null, 2)}\n`; + try { + fs.writeFileSync(temporaryFile, content, "utf8"); + fs.renameSync(temporaryFile, filePath); + } catch (error) { + try { + fs.unlinkSync(temporaryFile); + } catch { + // The temporary file may not have been created or may already have been renamed. + } + throw error; } } @@ -111,7 +127,7 @@ export function saveState(cwd, state) { removeFileIfExists(job.logFile); } - fs.writeFileSync(resolveStateFile(cwd), `${JSON.stringify(nextState, null, 2)}\n`, "utf8"); + writeAtomicJson(resolveStateFile(cwd), nextState); return nextState; } @@ -166,7 +182,7 @@ export function getConfig(cwd) { export function writeJobFile(cwd, jobId, payload) { ensureStateDir(cwd); const jobFile = resolveJobFile(cwd, jobId); - fs.writeFileSync(jobFile, `${JSON.stringify(payload, null, 2)}\n`, "utf8"); + writeAtomicJson(jobFile, payload); return jobFile; } diff --git a/plugins/codex/scripts/stop-review-gate-hook.mjs b/plugins/codex/scripts/stop-review-gate-hook.mjs index c91979113..293f43717 100644 --- a/plugins/codex/scripts/stop-review-gate-hook.mjs +++ b/plugins/codex/scripts/stop-review-gate-hook.mjs @@ -227,6 +227,8 @@ try { main(); } catch (error) { const message = error instanceof Error ? error.message : String(error); - process.stderr.write(`${message}\n`); - process.exitCode = 1; + emitDecision({ + decision: "block", + reason: `Codex stop-review gate could not safely continue: ${message}` + }); } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 93c0f06c3..2acfbd64c 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -2704,6 +2704,27 @@ test("stop hook blocks when Codex is unavailable and the review gate is enabled" assert.match(JSON.parse(blocked.stdout).reason, /not set up/i); }); +test("stop hook blocks on corrupt state instead of silently allowing", () => { + const repo = makeTempDir(); + initGitRepo(repo); + const stateFile = path.join(resolveStateDir(repo), "state.json"); + fs.mkdirSync(path.dirname(stateFile), { recursive: true }); + fs.writeFileSync(stateFile, "{invalid json", "utf8"); + + const blocked = run(process.execPath, [STOP_HOOK], { + cwd: repo, + env: process.env, + input: JSON.stringify({ cwd: repo }) + }); + + assert.equal(blocked.status, 0, blocked.stderr); + const payload = JSON.parse(blocked.stdout); + assert.equal(payload.decision, "block"); + assert.match(payload.reason, /Failed to read Codex Companion state at/); + assert.match(payload.reason, new RegExp(stateFile.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"))); + assert.match(payload.reason, /Unexpected|Expected/); +}); + test("stop hook runs the actual task when auth status looks stale", () => { const repo = makeTempDir(); const binDir = makeTempDir(); diff --git a/tests/state.test.mjs b/tests/state.test.mjs index 0f8f57cea..bde2d39c5 100644 --- a/tests/state.test.mjs +++ b/tests/state.test.mjs @@ -5,7 +5,15 @@ import test from "node:test"; import assert from "node:assert/strict"; import { makeTempDir } from "./helpers.mjs"; -import { resolveJobFile, resolveJobLogFile, resolveStateDir, resolveStateFile, saveState } from "../plugins/codex/scripts/lib/state.mjs"; +import { + loadState, + resolveJobFile, + resolveJobLogFile, + resolveStateDir, + resolveStateFile, + saveState, + writeJobFile +} from "../plugins/codex/scripts/lib/state.mjs"; test("resolveStateDir uses a temp-backed per-workspace directory", () => { const workspace = makeTempDir(); @@ -40,6 +48,44 @@ test("resolveStateDir uses CLAUDE_PLUGIN_DATA when it is provided", () => { } }); +test("loadState rejects invalid state JSON with its absolute path and parse detail", () => { + const workspace = makeTempDir(); + const stateFile = resolveStateFile(workspace); + fs.mkdirSync(path.dirname(stateFile), { recursive: true }); + fs.writeFileSync(stateFile, "{invalid json", "utf8"); + + assert.throws( + () => loadState(workspace), + (error) => + error instanceof Error && + error.message.includes(stateFile) && + error.message.startsWith("Failed to read Codex Companion state at ") && + /Unexpected|Expected/.test(error.message) + ); +}); + +test("state and job writes leave valid JSON without sibling temporary artifacts", () => { + const workspace = makeTempDir(); + const stateFile = resolveStateFile(workspace); + const jobFile = writeJobFile(workspace, "job-atomic", { id: "job-atomic", status: "queued" }); + + saveState(workspace, { + config: { stopReviewGate: true }, + jobs: [{ id: "job-atomic", status: "queued" }] + }); + + assert.equal(JSON.parse(fs.readFileSync(stateFile, "utf8")).config.stopReviewGate, true); + assert.deepEqual(JSON.parse(fs.readFileSync(jobFile, "utf8")), { id: "job-atomic", status: "queued" }); + assert.deepEqual( + fs.readdirSync(path.dirname(stateFile)).filter((entry) => entry.startsWith(`${path.basename(stateFile)}.`)), + [] + ); + assert.deepEqual( + fs.readdirSync(path.dirname(jobFile)).filter((entry) => entry.startsWith(`${path.basename(jobFile)}.`)), + [] + ); +}); + test("saveState prunes dropped job artifacts when indexed jobs exceed the cap", () => { const workspace = makeTempDir(); const stateFile = resolveStateFile(workspace); From 1e8aaa7f35167bd2791c2f070723e395f49d7fc1 Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 16:40:09 +0300 Subject: [PATCH 14/19] docs: document Codex gate supervision --- .../plans/2026-08-19-fail-closed-codex-gate.md | 10 +++++----- .../specs/2026-08-19-fail-closed-codex-gate-design.md | 8 ++++---- plugins/codex/CHANGELOG.md | 7 +++++++ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md b/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md index 6149ba8d8..d04d00a3c 100644 --- a/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md +++ b/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md @@ -4,7 +4,7 @@ **Goal:** Make detached Codex jobs self-heal after worker loss and make the Claude Stop review gate deterministically fail closed without duplicate reviews of the same turn. -**Architecture:** Persist queued work before spawning, reconcile active jobs against their worker PID on every control-plane read, and refuse late writes after a terminal transition. Reuse Stop-review results by a hash of session ID plus last assistant message, while retaining the existing foreground timeout. +**Architecture:** Persist queued work before spawning, reconcile active jobs against their worker PID on every control-plane read, and refuse late writes after a terminal transition. Use a deterministic full-hash Stop job ID and immutable claims to single-flight an exact raw Claude turn, while retaining the existing foreground timeout. **Tech Stack:** Node.js ESM, built-in `node:test`, filesystem JSON state, Claude Code hooks. @@ -13,7 +13,7 @@ - No new runtime dependencies or daemon. - The only job transitions are `queued -> running -> completed|failed|cancelled`. - Enabled review gates fail closed for infrastructure and persistence errors. -- The last assistant message is never stored in a gate cache key. +- The raw, untrimmed last assistant message is never stored in a gate cache key; an empty message is not cached. - All production changes follow RED-GREEN TDD. --- @@ -128,7 +128,7 @@ The worker sets its own PID when it enters `runTrackedJob`; queued jobs receive - [ ] **Step 4: Protect terminal transitions** -Workers first claim `jobs/.started.json` with `openSync(..., "wx")`; reconciliation, SessionEnd, and startup compete there before a running publication. Later terminal outcomes compete on `jobs/.terminal.json`. Empty/corrupt claims fail as failed, progress never upserts the state index, and effective reads merge mutable job-file fields. SessionEnd writes a dominant empty `jobs/.removed` marker before cleanup so a late mutable artifact is never visible. +Workers first claim `jobs/.started.json` with `openSync(..., "wx")`; reconciliation, SessionEnd, and startup compete there before a running publication. After publishing `running`, only the exclusive `jobs/.admission.json` winner may call the runner. Later terminal outcomes compete on `jobs/.terminal.json`. Empty/corrupt claims fail as failed, progress never upserts the state index, and effective reads merge mutable job-file fields. SessionEnd writes a dominant empty `jobs/.removed` marker before cleanup so a late mutable artifact is never visible. - [ ] **Step 5: Verify Task 2 GREEN** @@ -168,13 +168,13 @@ Expected: unavailable Codex produces no decision and the second Stop starts anot - [ ] **Step 3: Propagate and reuse the gate key** -Hash without retaining message content: +Hash the raw, untrimmed non-empty message without retaining its content: ```js createHash("sha256").update(`${sessionId}\0${lastAssistantMessage}`).digest("hex") ``` -Pass it through `CODEX_COMPANION_GATE_KEY`, record it on the tracked job, and before launching find the matching current-session Stop job. Reparse completed output; block with the existing job ID for active, failed, or cancelled matches. +Pass it through `CODEX_COMPANION_GATE_KEY`, use deterministic `gate-` as the tracked-job ID, and before availability checks find the matching current-session Stop job. The immutable startup claim makes concurrent matching launches single-flight. Reparse completed output; block with the existing job ID for active, failed, or cancelled matches. With no key, run fresh without caching. - [ ] **Step 4: Make setup failures fail closed** diff --git a/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md b/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md index 4f293172c..5ee6dc0a6 100644 --- a/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md +++ b/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md @@ -24,7 +24,7 @@ Make Codex Companion jobs self-heal after worker loss and make the Claude Stop r - A running or queued job with a dead PID becomes `failed` with `Background worker exited before completing the job.` - Reconciliation runs before status, wait, result, cancel, task resume selection, and Stop-hook decisions. - Terminal state is claimed by an immutable per-job `jobs/.terminal.json` fence, created with exclusive filesystem creation. The first terminal writer wins; terminal fences contain only status and completion time, never request, prompt, or log content. -- Worker startup is separately claimed by `jobs/.started.json`: its first writer is either a running PID/start time or a terminal outcome. A duplicate worker that loses this initial claim does not publish or execute. +- Worker startup is separately claimed by `jobs/.started.json`: its first writer is either a running PID/start time or a terminal outcome. The winner publishes `running`, then exclusively claims `jobs/.admission.json` before calling the runner; a duplicate or terminal winner cannot execute. - A terminal or removed job cannot be overwritten by a late worker. Corrupt or empty fences fail closed as `failed`; all control-plane reads use the fence over mutable job/index JSON. - SessionEnd writes an immutable empty `jobs/.removed` marker before cleanup. Removal overrides every terminal or running claim, including a completion that won before SessionEnd, and may remain as a tiny orphan fence. - Failed job status output includes the stored error message. @@ -33,10 +33,10 @@ Make Codex Companion jobs self-heal after worker loss and make the Claude Stop r - An enabled gate is fail-closed for unavailable Codex, task failure, timeout, missing output, invalid output, and corrupt state. - Every block explains the failure and how to retry. Active jobs also show the exact status and cancel commands. -- The gate key is a SHA-256 hash of the Claude session ID and the exact last assistant message. No message content is stored in the key. -- A completed Stop review for the same gate key is reused. An active matching review blocks with its existing job ID instead of starting another review. +- The gate key is a SHA-256 hash of the Claude session ID and the raw, untrimmed last assistant message. No message content is stored in the key; an empty message has no key and is never cached. +- A Stop review uses deterministic `gate-` job ID plus the immutable startup claim, making concurrent same-turn invocations single-flight. A completed matching review is reused; an active matching review blocks with its existing job ID instead of starting another review. - A different last assistant message gets a new gate key and a fresh review. -- If no last assistant message is supplied, the hook runs a fresh review and does not cache it. +- Matching cached jobs are checked before Codex availability, so a prior same-turn decision is still reusable when Codex later becomes unavailable. ### Persistence diff --git a/plugins/codex/CHANGELOG.md b/plugins/codex/CHANGELOG.md index d647561bb..81ced9089 100644 --- a/plugins/codex/CHANGELOG.md +++ b/plugins/codex/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## Unreleased + +- Hardened background supervision: queue records are published before spawn, and queued or running jobs with missing/dead workers are reconciled to failure. +- Added immutable startup, admission, terminal, and removal claims so late workers cannot resurrect jobs or execute duplicates. +- Stop-gate reviews now single-flight per exact Claude turn, reuse the same-turn result, and fail closed for unavailable or corrupt persistence. +- Made mutable state and job JSON writes atomic. + ## 1.0.0 - Initial version of the Codex plugin for Claude Code From f9854ff4d4f874b020d5e92a896dbed05a5e7ad4 Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 16:56:14 +0300 Subject: [PATCH 15/19] fix: clean retired Codex job sidecars --- plugins/codex/scripts/lib/state.mjs | 48 +++++++++++++++++++ .../codex/scripts/stop-review-gate-hook.mjs | 7 ++- tests/runtime.test.mjs | 13 ++++- tests/state.test.mjs | 5 ++ 4 files changed, 67 insertions(+), 6 deletions(-) diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs index 2c6946d5e..b71709077 100644 --- a/plugins/codex/scripts/lib/state.mjs +++ b/plugins/codex/scripts/lib/state.mjs @@ -3,6 +3,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { isProcessAlive } from "./process.mjs"; import { resolveWorkspaceRoot } from "./workspace.mjs"; const STATE_VERSION = 1; @@ -105,6 +106,51 @@ function removeFileIfExists(filePath) { } } +function resolveJobSidecarFile(cwd, jobId, suffix) { + return path.join(resolveJobsDir(cwd), `${jobId}${suffix}`); +} + +function markJobRemoved(cwd, jobId) { + try { + fs.closeSync(fs.openSync(resolveJobSidecarFile(cwd, jobId, ".removed"), "wx")); + } catch (error) { + if (error?.code !== "EEXIST") { + throw error; + } + } +} + +function canRemoveJobSidecars(cwd, jobId) { + const startedFile = resolveJobSidecarFile(cwd, jobId, ".started.json"); + if (!fs.existsSync(startedFile)) { + return true; + } + try { + const started = JSON.parse(fs.readFileSync(startedFile, "utf8")); + return started?.status !== "running" || (Number.isFinite(started.pid) && !isProcessAlive(started.pid)); + } catch { + return false; + } +} + +function removeJobSidecars(cwd, jobId) { + for (const suffix of [".started.json", ".admission.json", ".terminal.json", ".removed"]) { + removeFileIfExists(resolveJobSidecarFile(cwd, jobId, suffix)); + } +} + +function sweepRemovedJobSidecars(cwd) { + for (const entry of fs.readdirSync(resolveJobsDir(cwd))) { + if (!entry.endsWith(".removed")) { + continue; + } + const jobId = entry.slice(0, -".removed".length); + if (!fs.existsSync(resolveJobFile(cwd, jobId)) && canRemoveJobSidecars(cwd, jobId)) { + removeJobSidecars(cwd, jobId); + } + } +} + export function saveState(cwd, state) { const previousJobs = loadState(cwd).jobs; ensureStateDir(cwd); @@ -123,11 +169,13 @@ export function saveState(cwd, state) { if (retainedIds.has(job.id)) { continue; } + markJobRemoved(cwd, job.id); removeJobFile(resolveJobFile(cwd, job.id)); removeFileIfExists(job.logFile); } writeAtomicJson(resolveStateFile(cwd), nextState); + sweepRemovedJobSidecars(cwd); return nextState; } diff --git a/plugins/codex/scripts/stop-review-gate-hook.mjs b/plugins/codex/scripts/stop-review-gate-hook.mjs index 293f43717..e885b2eea 100644 --- a/plugins/codex/scripts/stop-review-gate-hook.mjs +++ b/plugins/codex/scripts/stop-review-gate-hook.mjs @@ -67,11 +67,10 @@ function getGateKey(input = {}) { } function gateJobNote(job) { - const commands = `Check /codex:status ${job.id} and use /codex:cancel ${job.id} if you want to stop it.`; if (job.status === "queued" || job.status === "running") { - return `The stop-time Codex review is already ${job.status} as ${job.id}. ${commands}`; + return `The stop-time Codex review is already ${job.status} as ${job.id}. Check /codex:status ${job.id} and use /codex:cancel ${job.id} if you want to stop it.`; } - return `The prior stop-time Codex review ${job.id} is ${job.status}; it will not be rerun automatically. ${commands}`; + return `The prior stop-time Codex review ${job.id} is ${job.status}; it will not be rerun automatically. Check /codex:status ${job.id}, then run /codex:review --wait manually or bypass the gate.`; } function getGateJob(jobs, gateKey) { @@ -83,7 +82,7 @@ function parseStoredGateReview(job) { if (typeof rawOutput !== "string") { return { ok: false, - reason: `The completed stop-time Codex review ${job.id} has missing or corrupt cached output and will not be rerun automatically. Run /codex:review --wait manually or bypass the gate.` + reason: `The completed stop-time Codex review ${job.id} has missing or corrupt cached output and will not be rerun automatically. Check /codex:status ${job.id}, then run /codex:review --wait manually or bypass the gate.` }; } return parseStopReviewOutput(rawOutput); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 2acfbd64c..f8aba76fd 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -2337,7 +2337,6 @@ test("session end fully cleans up jobs for the ending session", async (t) => { [ path.basename(otherJobFile), path.basename(otherSessionLog), - "review-completed.removed", "review-running.admission.json", "review-running.removed", "review-running.started.json" @@ -2357,6 +2356,8 @@ test("session end fully cleans up jobs for the ending session", async (t) => { assert.deepEqual(state.jobs.map((job) => job.id), ["review-other"]); const otherJob = state.jobs[0]; assert.equal(otherJob.logFile, otherSessionLog); + saveState(repo, state); + assert.deepEqual(fs.readdirSync(jobsDir).sort(), [path.basename(otherJobFile), path.basename(otherSessionLog)].sort()); }); test("stop hook runs a stop-time review task and blocks on findings when the review gate is enabled", () => { @@ -2520,7 +2521,15 @@ test("stop hook blocks every matching non-reusable gate job without starting a t const hookResult = run("node", [STOP_HOOK], { cwd: repo, env, input: JSON.stringify({ cwd: repo, session_id: sessionId, last_assistant_message: message }) }); assert.equal(hookResult.status, 0, hookResult.stderr); assert.equal(JSON.parse(hookResult.stdout).decision, "block"); - assert.match(JSON.parse(hookResult.stdout).reason, new RegExp(jobId)); + const reason = JSON.parse(hookResult.stdout).reason; + assert.match(reason, new RegExp(jobId)); + if (status === "queued" || status === "running") { + assert.match(reason, new RegExp(`/codex:cancel ${jobId}`)); + } else { + assert.doesNotMatch(reason, /\/codex:cancel/); + assert.match(reason, /\/codex:status/); + assert.match(reason, /manual|bypass/i); + } const afterTurns = fs.existsSync(fakeStatePath) ? JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId : 1; assert.equal(afterTurns, beforeTurns); } diff --git a/tests/state.test.mjs b/tests/state.test.mjs index bde2d39c5..60cebfbfa 100644 --- a/tests/state.test.mjs +++ b/tests/state.test.mjs @@ -98,6 +98,11 @@ test("saveState prunes dropped job artifacts when indexed jobs exceed the cap", const jobFile = resolveJobFile(workspace, jobId); fs.writeFileSync(logFile, `log ${jobId}\n`, "utf8"); fs.writeFileSync(jobFile, JSON.stringify({ id: jobId, status: "completed" }, null, 2), "utf8"); + if (jobId === "job-0") { + fs.writeFileSync(jobFile.replace(/\.json$/, ".started.json"), JSON.stringify({ status: "running", pid: 999999 }), "utf8"); + fs.writeFileSync(jobFile.replace(/\.json$/, ".admission.json"), JSON.stringify({ status: "admitted" }), "utf8"); + fs.writeFileSync(jobFile.replace(/\.json$/, ".terminal.json"), JSON.stringify({ status: "completed" }), "utf8"); + } return { id: jobId, status: "completed", From 2e451e321d6b5b2a6410961f184d7d3098c0d63f Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 17:12:07 +0300 Subject: [PATCH 16/19] fix: serialize Codex job lifecycle state --- plugins/codex/scripts/lib/process.mjs | 22 +-- plugins/codex/scripts/lib/state.mjs | 174 ++++++++++++++---- plugins/codex/scripts/lib/tracked-jobs.mjs | 8 +- .../codex/scripts/session-lifecycle-hook.mjs | 41 ++--- tests/process.test.mjs | 21 +++ tests/runtime.test.mjs | 154 +++++++++++++++- tests/state.test.mjs | 36 ++++ 7 files changed, 367 insertions(+), 89 deletions(-) diff --git a/plugins/codex/scripts/lib/process.mjs b/plugins/codex/scripts/lib/process.mjs index adfa923f9..5918b3674 100644 --- a/plugins/codex/scripts/lib/process.mjs +++ b/plugins/codex/scripts/lib/process.mjs @@ -51,7 +51,7 @@ export function binaryAvailable(command, versionArgs = ["--version"], options = } export function isProcessAlive(pid, options = {}) { - if (!Number.isFinite(pid)) { + if (!Number.isSafeInteger(pid) || pid <= 0) { return false; } @@ -69,7 +69,7 @@ function looksLikeMissingProcessMessage(text) { } export function terminateProcessTree(pid, options = {}) { - if (!Number.isFinite(pid)) { + if (!Number.isSafeInteger(pid) || pid <= 0) { return { attempted: false, delivered: false, method: null }; } @@ -115,19 +115,15 @@ export function terminateProcessTree(pid, options = {}) { killImpl(-pid, "SIGTERM"); return { attempted: true, delivered: true, method: "process-group" }; } catch (error) { - if (error?.code !== "ESRCH") { - try { - killImpl(pid, "SIGTERM"); - return { attempted: true, delivered: true, method: "process" }; - } catch (innerError) { - if (innerError?.code === "ESRCH") { - return { attempted: true, delivered: false, method: "process" }; - } - throw innerError; + try { + killImpl(pid, "SIGTERM"); + return { attempted: true, delivered: true, method: "process" }; + } catch (innerError) { + if (innerError?.code === "ESRCH") { + return { attempted: true, delivered: false, method: "process" }; } + throw innerError; } - - return { attempted: true, delivered: false, method: "process-group" }; } } diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs index b71709077..c5c61b815 100644 --- a/plugins/codex/scripts/lib/state.mjs +++ b/plugins/codex/scripts/lib/state.mjs @@ -12,6 +12,9 @@ const FALLBACK_STATE_ROOT_DIR = path.join(os.tmpdir(), "codex-companion"); const STATE_FILE_NAME = "state.json"; const JOBS_DIR_NAME = "jobs"; const MAX_JOBS = 50; +const STATE_LOCK_FILE_NAME = ".state.lock"; +const STATE_LOCK_WAIT_MS = 5000; +const STATE_LOCK_RETRY_MS = 20; function nowIso() { return new Date().toISOString(); @@ -56,19 +59,27 @@ export function ensureStateDir(cwd) { fs.mkdirSync(resolveJobsDir(cwd), { recursive: true }); } +function isObject(value) { + return value && typeof value === "object" && !Array.isArray(value); +} + +function validateState(parsed) { + if (!isObject(parsed) || parsed.version !== STATE_VERSION || !isObject(parsed.config) || typeof parsed.config.stopReviewGate !== "boolean" || !Array.isArray(parsed.jobs)) { + throw new Error("invalid state schema"); + } + for (const job of parsed.jobs) { + if (!isObject(job) || typeof job.id !== "string" || !job.id || typeof job.status !== "string" || !job.status) { + throw new Error("invalid job schema"); + } + } +} + export function loadState(cwd) { const stateFile = path.resolve(resolveStateFile(cwd)); try { const parsed = JSON.parse(fs.readFileSync(stateFile, "utf8")); - return { - ...defaultState(), - ...parsed, - config: { - ...defaultState().config, - ...(parsed.config ?? {}) - }, - jobs: Array.isArray(parsed.jobs) ? parsed.jobs : [] - }; + validateState(parsed); + return parsed; } catch (error) { if (error?.code === "ENOENT") { return defaultState(); @@ -78,6 +89,108 @@ export function loadState(cwd) { } } +function pauseForStateLock() { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, STATE_LOCK_RETRY_MS); +} + +function readStateLockOwner(lockFile) { + let owner; + try { + owner = JSON.parse(fs.readFileSync(lockFile, "utf8")); + } catch (error) { + if (error?.code === "ENOENT") { + throw error; + } + throw new Error(`Codex Companion state lock at ${path.resolve(lockFile)} has an invalid owner.`, { cause: error }); + } + if (!isObject(owner) || !Number.isSafeInteger(owner.pid) || owner.pid <= 0 || typeof owner.token !== "string" || !owner.token || typeof owner.createdAt !== "string") { + throw new Error(`Codex Companion state lock at ${path.resolve(lockFile)} has an invalid owner.`); + } + return owner; +} + +function tryCreateStateLock(filePath, payload) { + const temporaryFile = `${filePath}.${process.pid}.${payload.token}.tmp`; + try { + fs.writeFileSync(temporaryFile, `${JSON.stringify(payload)}\n`, "utf8"); + fs.linkSync(temporaryFile, filePath); + return true; + } catch (error) { + if (error?.code === "EEXIST") { + return false; + } + throw error; + } finally { + removeFileIfExists(temporaryFile); + } +} + +function releaseStateLock(lockFile, token) { + try { + if (readStateLockOwner(lockFile).token === token) { + fs.unlinkSync(lockFile); + } + } catch (error) { + if (error?.code !== "ENOENT") { + throw error; + } + } +} + +function reapDeadStateLock(lockFile, owner) { + const reapFile = `${lockFile}.reap`; + const token = randomUUID(); + if (!tryCreateStateLock(reapFile, { pid: process.pid, token, createdAt: nowIso() })) { + return; + } + try { + const current = readStateLockOwner(lockFile); + if (current.token === owner.token && current.pid === owner.pid && !isProcessAlive(current.pid)) { + fs.unlinkSync(lockFile); + } + } catch (error) { + if (error?.code !== "ENOENT") { + throw error; + } + } finally { + releaseStateLock(reapFile, token); + } +} + +function withStateLock(cwd, action) { + ensureStateDir(cwd); + const lockFile = path.join(resolveStateDir(cwd), STATE_LOCK_FILE_NAME); + const deadline = Date.now() + STATE_LOCK_WAIT_MS; + const token = randomUUID(); + while (true) { + if (tryCreateStateLock(lockFile, { pid: process.pid, token, createdAt: nowIso() })) { + try { + return action(); + } finally { + releaseStateLock(lockFile, token); + } + } + try { + const owner = readStateLockOwner(lockFile); + if (!isProcessAlive(owner.pid)) { + reapDeadStateLock(lockFile, owner); + continue; + } + } catch (error) { + if (error?.code === "ENOENT") { + continue; + } + if (Date.now() >= deadline) { + throw error; + } + } + if (Date.now() >= deadline) { + throw new Error(`Timed out waiting for Codex Companion state lock at ${path.resolve(lockFile)}.`); + } + pauseForStateLock(); + } +} + function writeAtomicJson(filePath, value) { const temporaryFile = `${filePath}.${process.pid}.${randomUUID()}.tmp`; const content = `${JSON.stringify(value, null, 2)}\n`; @@ -120,38 +233,13 @@ function markJobRemoved(cwd, jobId) { } } -function canRemoveJobSidecars(cwd, jobId) { - const startedFile = resolveJobSidecarFile(cwd, jobId, ".started.json"); - if (!fs.existsSync(startedFile)) { - return true; - } - try { - const started = JSON.parse(fs.readFileSync(startedFile, "utf8")); - return started?.status !== "running" || (Number.isFinite(started.pid) && !isProcessAlive(started.pid)); - } catch { - return false; - } -} - function removeJobSidecars(cwd, jobId) { - for (const suffix of [".started.json", ".admission.json", ".terminal.json", ".removed"]) { + for (const suffix of [".started.json", ".admission.json", ".terminal.json"]) { removeFileIfExists(resolveJobSidecarFile(cwd, jobId, suffix)); } } -function sweepRemovedJobSidecars(cwd) { - for (const entry of fs.readdirSync(resolveJobsDir(cwd))) { - if (!entry.endsWith(".removed")) { - continue; - } - const jobId = entry.slice(0, -".removed".length); - if (!fs.existsSync(resolveJobFile(cwd, jobId)) && canRemoveJobSidecars(cwd, jobId)) { - removeJobSidecars(cwd, jobId); - } - } -} - -export function saveState(cwd, state) { +function saveStateLocked(cwd, state) { const previousJobs = loadState(cwd).jobs; ensureStateDir(cwd); const nextJobs = pruneJobs(state.jobs ?? []); @@ -172,17 +260,23 @@ export function saveState(cwd, state) { markJobRemoved(cwd, job.id); removeJobFile(resolveJobFile(cwd, job.id)); removeFileIfExists(job.logFile); + removeJobSidecars(cwd, job.id); } writeAtomicJson(resolveStateFile(cwd), nextState); - sweepRemovedJobSidecars(cwd); return nextState; } +export function saveState(cwd, state) { + return withStateLock(cwd, () => saveStateLocked(cwd, state)); +} + export function updateState(cwd, mutate) { - const state = loadState(cwd); - mutate(state); - return saveState(cwd, state); + return withStateLock(cwd, () => { + const state = loadState(cwd); + mutate(state); + return saveStateLocked(cwd, state); + }); } export function generateJobId(prefix = "job") { diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 635e13d72..b18fa18a8 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -406,6 +406,11 @@ export function reconcileTrackedJobs(workspaceRoot, options = {}) { return [job]; } + if (job.status === "running" && (!Number.isSafeInteger(job.pid) || job.pid <= 0)) { + const failedJob = failTrackedJob(workspaceRoot, job, "Tracked running job has an invalid process id."); + return failedJob ? [failedJob] : []; + } + const ageMs = now - Date.parse(job.createdAt ?? ""); if (job.status === "queued" && !Number.isFinite(job.pid) && Number.isFinite(ageMs) && ageMs >= 5000) { const failedJob = failTrackedJob(workspaceRoot, job, "Background worker did not start within 5 seconds."); @@ -505,8 +510,9 @@ export async function runTrackedJob(job, runner, options = {}) { }); if (terminal.claimed) { appendLogBlock(options.logFile ?? job.logFile ?? null, "Final output", execution.rendered); + return execution; } - return execution; + return terminal.job; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); const completedAt = nowIso(); diff --git a/plugins/codex/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs index 0d7a344c0..d5bf63367 100644 --- a/plugins/codex/scripts/session-lifecycle-hook.mjs +++ b/plugins/codex/scripts/session-lifecycle-hook.mjs @@ -13,8 +13,8 @@ import { sendBrokerShutdown, teardownBrokerSession } from "./lib/broker-lifecycle.mjs"; -import { loadState, resolveStateFile, saveState } from "./lib/state.mjs"; -import { markTrackedJobRemoved, readEffectiveStoredJob, terminalizeTrackedJob } from "./lib/tracked-jobs.mjs"; +import { resolveStateFile, updateState } from "./lib/state.mjs"; +import { markTrackedJobRemoved, readEffectiveStoredJob } from "./lib/tracked-jobs.mjs"; import { TRANSCRIPT_PATH_ENV } from "./lib/claude-session-transfer.mjs"; import { resolveWorkspaceRoot } from "./lib/workspace.mjs"; @@ -51,38 +51,25 @@ function cleanupSessionJobs(cwd, sessionId) { return; } - const state = loadState(workspaceRoot); - const removedJobs = state.jobs.filter((job) => job.sessionId === sessionId); - if (removedJobs.length === 0) { - return; - } - - for (const job of removedJobs) { - const effectiveJob = { ...job, ...(readEffectiveStoredJob(workspaceRoot, job.id) ?? {}) }; - const stillRunning = effectiveJob.status === "queued" || effectiveJob.status === "running"; - if (!stillRunning) { + const pids = []; + updateState(workspaceRoot, (state) => { + for (const job of state.jobs.filter((candidate) => candidate.sessionId === sessionId)) { + const effectiveJob = { ...job, ...(readEffectiveStoredJob(workspaceRoot, job.id) ?? {}) }; markTrackedJobRemoved(workspaceRoot, job.id); - continue; + if (Number.isSafeInteger(effectiveJob.pid) && effectiveJob.pid > 0) { + pids.push(effectiveJob.pid); + } } - terminalizeTrackedJob(workspaceRoot, job, { - status: "cancelled", - phase: "cancelled", - pid: null, - completedAt: new Date().toISOString(), - errorMessage: "Cancelled because the Claude session ended." - }); - markTrackedJobRemoved(workspaceRoot, job.id); + state.jobs = state.jobs.filter((job) => job.sessionId !== sessionId); + }); + + for (const pid of pids) { try { - terminateProcessTree(effectiveJob.pid ?? Number.NaN); + terminateProcessTree(pid); } catch { // Ignore teardown failures during session shutdown. } } - - saveState(workspaceRoot, { - ...state, - jobs: state.jobs.filter((job) => job.sessionId !== sessionId) - }); } function handleSessionStart(input) { diff --git a/tests/process.test.mjs b/tests/process.test.mjs index 8f6dac2f1..27a4403c7 100644 --- a/tests/process.test.mjs +++ b/tests/process.test.mjs @@ -26,6 +26,27 @@ test("isProcessAlive treats a missing process as dead", () => { ); }); +test("process helpers reject unsafe process ids", () => { + for (const pid of [0, -1, 1.5, Number.MAX_SAFE_INTEGER + 1]) { + assert.equal(isProcessAlive(pid, { killImpl() { throw new Error("must not probe"); } }), false); + assert.equal(terminateProcessTree(pid, { killImpl() { throw new Error("must not kill"); } }).attempted, false); + } +}); + +test("terminateProcessTree falls back from a missing POSIX group to its leader", () => { + const calls = []; + const outcome = terminateProcessTree(1234, { + platform: "linux", + killImpl(pid) { + calls.push(pid); + throw Object.assign(new Error("gone"), { code: "ESRCH" }); + } + }); + assert.deepEqual(calls, [-1234, 1234]); + assert.equal(outcome.delivered, false); + assert.equal(outcome.method, "process"); +}); + test("terminateProcessTree uses taskkill on Windows", () => { let captured = null; const outcome = terminateProcessTree(1234, { diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index f8aba76fd..f91dff4fb 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -34,9 +34,9 @@ function resolveTerminalFenceFile(workspaceRoot, jobId) { return resolveJobFile(workspaceRoot, jobId).replace(/\.json$/, ".terminal.json"); } -function runHookAsync(cwd, env, input) { +function runHookAsync(cwd, env, input, script = STOP_HOOK) { return new Promise((resolve, reject) => { - const child = spawn(process.execPath, [STOP_HOOK], { cwd, env, stdio: ["pipe", "pipe", "pipe"] }); + const child = spawn(process.execPath, [script], { cwd, env, stdio: ["pipe", "pipe", "pipe"] }); let stdout = ""; let stderr = ""; child.stdout.on("data", (chunk) => { stdout += chunk; }); @@ -1202,12 +1202,24 @@ test("terminal job reconciliation wins over late worker finalization", async () } }); releaseRunner(); - await execution; + const result = await execution; + assert.equal(result.status, "failed"); assert.equal(listJobs(workspaceRoot).find((candidate) => candidate.id === job.id).status, "failed"); assert.equal(readJobFile(resolveJobFile(workspaceRoot, job.id)).status, "failed"); }); +test("reconciliation fails a running job with an invalid process id", () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-invalid-pid", workspaceRoot, status: "running", pid: null }; + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + + const [result] = reconcileTrackedJobs(workspaceRoot); + assert.equal(result.status, "failed"); + assert.match(result.errorMessage, /invalid process id/i); +}); + test("removed job terminal fence prevents progress and finalization from recreating it", async () => { const workspaceRoot = makeTempDir(); const job = { id: "task-removed", workspaceRoot, status: "queued" }; @@ -2337,9 +2349,8 @@ test("session end fully cleans up jobs for the ending session", async (t) => { [ path.basename(otherJobFile), path.basename(otherSessionLog), - "review-running.admission.json", - "review-running.removed", - "review-running.started.json" + "review-completed.removed", + "review-running.removed" ].sort() ); @@ -2357,7 +2368,76 @@ test("session end fully cleans up jobs for the ending session", async (t) => { const otherJob = state.jobs[0]; assert.equal(otherJob.logFile, otherSessionLog); saveState(repo, state); - assert.deepEqual(fs.readdirSync(jobsDir).sort(), [path.basename(otherJobFile), path.basename(otherSessionLog)].sort()); + assert.deepEqual( + fs.readdirSync(jobsDir).sort(), + [path.basename(otherJobFile), path.basename(otherSessionLog), "review-completed.removed", "review-running.removed"].sort() + ); +}); + +test("session end preserves an other-session job added after its stale snapshot", async () => { + const repo = makeTempDir(); + initGitRepo(repo); + const stateDir = resolveStateDir(repo); + const jobsDir = path.join(stateDir, "jobs"); + fs.mkdirSync(jobsDir, { recursive: true }); + const staleJob = { id: "stale-current", status: "completed", sessionId: "sess-current", logFile: path.join(jobsDir, "stale-current.log") }; + writeJobFile(repo, staleJob.id, staleJob); + fs.writeFileSync(staleJob.logFile, "stale\n", "utf8"); + saveState(repo, { config: { stopReviewGate: false }, jobs: [staleJob] }); + + const marker = path.join(makeTempDir(), "stale-read"); + const release = path.join(path.dirname(marker), "release"); + const preload = path.join(path.dirname(marker), "pause-state-read.cjs"); + fs.writeFileSync(preload, [ + 'const fs = require("node:fs");', + 'const original = fs.readFileSync;', + 'let paused = false;', + 'fs.readFileSync = function(file, ...args) {', + ' const value = original.call(this, file, ...args);', + ' if (!paused && file === process.env.CODEX_STALE_STATE_FILE) {', + ' paused = true; fs.writeFileSync(process.env.CODEX_STALE_MARKER, "paused");', + ' while (!fs.existsSync(process.env.CODEX_STALE_RELEASE)) Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 10);', + ' }', + ' return value;', + '};' + ].join("\n"), "utf8"); + fs.writeFileSync(marker, "ready", "utf8"); + fs.unlinkSync(marker); + + const hook = runHookAsync(repo, { + ...process.env, + CODEX_COMPANION_SESSION_ID: "sess-current", + CODEX_STALE_STATE_FILE: path.join(stateDir, "state.json"), + CODEX_STALE_MARKER: marker, + CODEX_STALE_RELEASE: release, + NODE_OPTIONS: `--require ${preload}` + }, JSON.stringify({ hook_event_name: "SessionEnd", session_id: "sess-current", cwd: repo }), SESSION_HOOK); + await waitFor(() => fs.existsSync(marker)); + + const freshJob = { id: "fresh-other", status: "completed", sessionId: "sess-other", logFile: path.join(jobsDir, "fresh-other.log") }; + const stateModule = path.join(PLUGIN_ROOT, "scripts", "lib", "state.mjs"); + let writerDone = false; + const writer = new Promise((resolve, reject) => { + const child = spawn(process.execPath, ["--input-type=module", "--eval", [ + 'import fs from "node:fs";', + `import { upsertJob, writeJobFile } from ${JSON.stringify(stateModule)};`, + `const job = ${JSON.stringify(freshJob)};`, + `writeJobFile(${JSON.stringify(repo)}, job.id, job);`, + 'fs.writeFileSync(job.logFile, "fresh\\n", "utf8");', + `upsertJob(${JSON.stringify(repo)}, job);` + ].join("\n")], { stdio: "ignore" }); + child.once("error", reject); + child.once("close", (status) => { writerDone = true; resolve(status); }); + }); + await new Promise((resolve) => setTimeout(resolve, 50)); + assert.equal(writerDone, false); + fs.writeFileSync(release, "go", "utf8"); + const result = await hook; + assert.equal(result.status, 0, result.stderr); + assert.equal(await writer, 0); + assert.deepEqual(listJobs(repo).map((job) => job.id), ["fresh-other"]); + assert.equal(fs.existsSync(resolveJobFile(repo, freshJob.id)), true); + assert.equal(fs.existsSync(freshJob.logFile), true); }); test("stop hook runs a stop-time review task and blocks on findings when the review gate is enabled", () => { @@ -2515,7 +2595,7 @@ test("stop hook blocks every matching non-reusable gate job without starting a t saveState(repo, { version: 1, config: { stopReviewGate: true }, - jobs: [{ id: jobId, gateKey, sessionId, status, title: "Codex Stop Gate Review", ...(cachedResult === undefined ? {} : { result: cachedResult }) }] + jobs: [{ id: jobId, gateKey, sessionId, status, title: "Codex Stop Gate Review", ...(status === "running" ? { pid: process.pid } : {}), ...(cachedResult === undefined ? {} : { result: cachedResult }) }] }); const beforeTurns = fs.existsSync(fakeStatePath) ? JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId : 1; const hookResult = run("node", [STOP_HOOK], { cwd: repo, env, input: JSON.stringify({ cwd: repo, session_id: sessionId, last_assistant_message: message }) }); @@ -2535,6 +2615,64 @@ test("stop hook blocks every matching non-reusable gate job without starting a t } }); +test("stop hook blocks a parsed state with an invalid schema", () => { + const repo = makeTempDir(); + initGitRepo(repo); + const stateDir = resolveStateDir(repo); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync(path.join(stateDir, "state.json"), JSON.stringify({ version: 1, config: {}, jobs: [] }), "utf8"); + + const result = run("node", [STOP_HOOK], { + cwd: repo, + input: JSON.stringify({ cwd: repo, session_id: "sess-invalid-state", last_assistant_message: "done" }) + }); + assert.equal(result.status, 0, result.stderr); + const decision = JSON.parse(result.stdout); + assert.equal(decision.decision, "block"); + assert.match(decision.reason, /could not safely continue.*invalid state schema/i); +}); + +test("stop gate blocks when its foreground review loses the terminal claim", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "adversarial-clean"); + initGitRepo(repo); + const sessionId = "sess-stop-terminal-race"; + const message = "Clean review that loses the terminal race."; + const gateKey = createHash("sha256").update(`${sessionId}\0${message}`).digest("hex"); + const jobId = `gate-${gateKey}`; + const terminalFile = resolveTerminalFenceFile(repo, jobId); + const preload = path.join(makeTempDir(), "terminal-race.cjs"); + fs.writeFileSync(preload, [ + 'const fs = require("node:fs");', + 'const original = fs.openSync;', + 'let armed = true;', + 'fs.openSync = (file, flags, ...rest) => {', + ' if (armed && file === process.env.CODEX_FOREGROUND_TERMINAL_FENCE && flags === "wx") {', + ' armed = false; fs.writeFileSync(file, JSON.stringify({ status: "cancelled", completedAt: "2026-08-19T12:01:00.000Z" }));', + ' }', + ' return original(file, flags, ...rest);', + '};' + ].join("\n"), "utf8"); + const env = { + ...buildEnv(binDir), + CODEX_FOREGROUND_TERMINAL_FENCE: terminalFile, + NODE_OPTIONS: `--require ${preload}` + }; + assert.equal(run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { cwd: repo, env }).status, 0); + + const result = run("node", [STOP_HOOK], { + cwd: repo, + env, + input: JSON.stringify({ cwd: repo, session_id: sessionId, last_assistant_message: message }) + }); + assert.equal(result.status, 0, result.stderr); + const decision = JSON.parse(result.stdout); + assert.equal(decision.decision, "block"); + assert.match(decision.reason, new RegExp(jobId)); + assert.doesNotMatch(result.stdout, /ALLOW:/); +}); + test("stop hook without an assistant message runs fresh reviews", () => { const repo = makeTempDir(); const binDir = makeTempDir(); diff --git a/tests/state.test.mjs b/tests/state.test.mjs index 60cebfbfa..064046b2d 100644 --- a/tests/state.test.mjs +++ b/tests/state.test.mjs @@ -64,6 +64,15 @@ test("loadState rejects invalid state JSON with its absolute path and parse deta ); }); +test("loadState rejects parsed state with an invalid schema", () => { + const workspace = makeTempDir(); + const stateFile = resolveStateFile(workspace); + fs.mkdirSync(path.dirname(stateFile), { recursive: true }); + fs.writeFileSync(stateFile, JSON.stringify({ version: 1, config: { stopReviewGate: "yes" }, jobs: {} }), "utf8"); + + assert.throws(() => loadState(workspace), /Failed to read Codex Companion state.*invalid state schema/); +}); + test("state and job writes leave valid JSON without sibling temporary artifacts", () => { const workspace = makeTempDir(); const stateFile = resolveStateFile(workspace); @@ -86,6 +95,17 @@ test("state and job writes leave valid JSON without sibling temporary artifacts" ); }); +test("saveState reaps a lock left by a dead owner", () => { + const workspace = makeTempDir(); + const stateDir = resolveStateDir(workspace); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync(path.join(stateDir, ".state.lock"), JSON.stringify({ pid: 999999, token: "dead-owner", createdAt: "2026-08-19T12:00:00.000Z" }), "utf8"); + + saveState(workspace, { config: { stopReviewGate: false }, jobs: [] }); + + assert.equal(fs.existsSync(path.join(stateDir, ".state.lock")), false); +}); + test("saveState prunes dropped job artifacts when indexed jobs exceed the cap", () => { const workspace = makeTempDir(); const stateFile = resolveStateFile(workspace); @@ -151,6 +171,22 @@ test("saveState prunes dropped job artifacts when indexed jobs exceed the cap", fs.readdirSync(jobsDir).sort(), Array.from({ length: 50 }, (_, index) => `job-${index + 1}`) .flatMap((jobId) => [`${jobId}.json`, `${jobId}.log`]) + .concat("job-0.removed") .sort() ); }); + +test("saveState retains a removal fence for a pruned job before it starts", () => { + const workspace = makeTempDir(); + const job = { id: "job-prestart", status: "queued", logFile: resolveJobLogFile(workspace, "job-prestart") }; + writeJobFile(workspace, job.id, job); + fs.writeFileSync(job.logFile, "queued\n", "utf8"); + saveState(workspace, { config: { stopReviewGate: false }, jobs: [job] }); + + saveState(workspace, { config: { stopReviewGate: false }, jobs: [] }); + + const jobFile = resolveJobFile(workspace, job.id); + assert.equal(fs.existsSync(jobFile), false); + assert.equal(fs.existsSync(job.logFile), false); + assert.equal(fs.existsSync(jobFile.replace(/\.json$/, ".removed")), true); +}); From 33c9812cd00d8ea1ceb35577efce86d51a046c8c Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 17:38:54 +0300 Subject: [PATCH 17/19] fix: fence removed Codex jobs --- plugins/codex/scripts/lib/state.mjs | 44 ++++++++---- plugins/codex/scripts/lib/tracked-jobs.mjs | 18 ++++- tests/runtime.test.mjs | 54 +++++++++++++-- tests/state.test.mjs | 79 ++++++++++++++++++++++ 4 files changed, 174 insertions(+), 21 deletions(-) diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs index c5c61b815..693108f82 100644 --- a/plugins/codex/scripts/lib/state.mjs +++ b/plugins/codex/scripts/lib/state.mjs @@ -15,6 +15,8 @@ const MAX_JOBS = 50; const STATE_LOCK_FILE_NAME = ".state.lock"; const STATE_LOCK_WAIT_MS = 5000; const STATE_LOCK_RETRY_MS = 20; +const JOB_STATUSES = new Set(["queued", "running", "completed", "failed", "cancelled"]); +const SAFE_JOB_ID = /^(?!.*\.\.)[A-Za-z0-9][A-Za-z0-9._-]*$/; function nowIso() { return new Date().toISOString(); @@ -68,7 +70,7 @@ function validateState(parsed) { throw new Error("invalid state schema"); } for (const job of parsed.jobs) { - if (!isObject(job) || typeof job.id !== "string" || !job.id || typeof job.status !== "string" || !job.status) { + if (!isObject(job) || typeof job.id !== "string" || !SAFE_JOB_ID.test(job.id) || !JOB_STATUSES.has(job.status)) { throw new Error("invalid job schema"); } } @@ -141,12 +143,13 @@ function reapDeadStateLock(lockFile, owner) { const reapFile = `${lockFile}.reap`; const token = randomUUID(); if (!tryCreateStateLock(reapFile, { pid: process.pid, token, createdAt: nowIso() })) { - return; + return false; } try { const current = readStateLockOwner(lockFile); if (current.token === owner.token && current.pid === owner.pid && !isProcessAlive(current.pid)) { fs.unlinkSync(lockFile); + return true; } } catch (error) { if (error?.code !== "ENOENT") { @@ -155,6 +158,7 @@ function reapDeadStateLock(lockFile, owner) { } finally { releaseStateLock(reapFile, token); } + return false; } function withStateLock(cwd, action) { @@ -173,8 +177,9 @@ function withStateLock(cwd, action) { try { const owner = readStateLockOwner(lockFile); if (!isProcessAlive(owner.pid)) { - reapDeadStateLock(lockFile, owner); - continue; + if (reapDeadStateLock(lockFile, owner)) { + continue; + } } } catch (error) { if (error?.code === "ENOENT") { @@ -208,9 +213,11 @@ function writeAtomicJson(filePath, value) { } function pruneJobs(jobs) { - return [...jobs] - .sort((left, right) => String(right.updatedAt ?? "").localeCompare(String(left.updatedAt ?? ""))) - .slice(0, MAX_JOBS); + const sorted = [...jobs].sort((left, right) => String(right.updatedAt ?? "").localeCompare(String(left.updatedAt ?? ""))); + const active = sorted.filter((job) => job.status === "queued" || job.status === "running"); + const terminal = sorted.filter((job) => job.status !== "queued" && job.status !== "running"); + return [...active, ...terminal.slice(0, Math.max(0, MAX_JOBS - active.length))] + .sort((left, right) => String(right.updatedAt ?? "").localeCompare(String(left.updatedAt ?? ""))); } function removeFileIfExists(filePath) { @@ -240,26 +247,37 @@ function removeJobSidecars(cwd, jobId) { } function saveStateLocked(cwd, state) { - const previousJobs = loadState(cwd).jobs; - ensureStateDir(cwd); - const nextJobs = pruneJobs(state.jobs ?? []); - const nextState = { + const candidate = { version: STATE_VERSION, config: { ...defaultState().config, ...(state.config ?? {}) }, + jobs: state.jobs ?? [] + }; + validateState(candidate); + const previousJobs = loadState(cwd).jobs; + ensureStateDir(cwd); + const requestedJobs = candidate.jobs; + const nextJobs = pruneJobs(requestedJobs.filter((job) => !fs.existsSync(resolveJobSidecarFile(cwd, job.id, ".removed")))); + const nextState = { + version: STATE_VERSION, + config: candidate.config, jobs: nextJobs }; const retainedIds = new Set(nextJobs.map((job) => job.id)); - for (const job of previousJobs) { + const knownJobs = new Map(previousJobs.map((job) => [job.id, job])); + for (const job of requestedJobs) { + knownJobs.set(job.id, { ...knownJobs.get(job.id), ...job }); + } + for (const job of knownJobs.values()) { if (retainedIds.has(job.id)) { continue; } markJobRemoved(cwd, job.id); removeJobFile(resolveJobFile(cwd, job.id)); - removeFileIfExists(job.logFile); + removeFileIfExists(resolveJobLogFile(cwd, job.id)); removeJobSidecars(cwd, job.id); } diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index b18fa18a8..6c68325a9 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -2,7 +2,7 @@ import fs from "node:fs"; import process from "node:process"; import { isProcessAlive } from "./process.mjs"; -import { listJobs, readJobFile, resolveJobFile, resolveJobLogFile, upsertJob, writeJobFile } from "./state.mjs"; +import { listJobs, readJobFile, resolveJobFile, resolveJobLogFile, updateState, upsertJob, writeJobFile } from "./state.mjs"; export const SESSION_ID_ENV = "CODEX_COMPANION_SESSION_ID"; export const GATE_KEY_ENV = "CODEX_COMPANION_GATE_KEY"; @@ -412,7 +412,11 @@ export function reconcileTrackedJobs(workspaceRoot, options = {}) { } const ageMs = now - Date.parse(job.createdAt ?? ""); - if (job.status === "queued" && !Number.isFinite(job.pid) && Number.isFinite(ageMs) && ageMs >= 5000) { + if (job.status === "queued" && !Number.isFinite(ageMs)) { + const failedJob = failTrackedJob(workspaceRoot, job, "Tracked queued job has an invalid creation time."); + return failedJob ? [failedJob] : []; + } + if (job.status === "queued" && !Number.isFinite(job.pid) && ageMs >= 5000) { const failedJob = failTrackedJob(workspaceRoot, job, "Background worker did not start within 5 seconds."); return failedJob ? [failedJob] : []; } @@ -488,8 +492,16 @@ export async function runTrackedJob(job, runner, options = {}) { if (terminalAfterStart) { return applyTerminalFence(runningRecord, terminalAfterStart); } - const admitted = claimFile(resolveAdmissionFile(job.workspaceRoot, job.id), { status: "admitted" }); + let admitted = false; + updateState(job.workspaceRoot, () => { + if (!isJobRemoved(job.workspaceRoot, job.id)) { + admitted = claimFile(resolveAdmissionFile(job.workspaceRoot, job.id), { status: "admitted" }); + } + }); if (!admitted) { + if (isJobRemoved(job.workspaceRoot, job.id)) { + return removedLifecycleRecord(runningRecord); + } return applyTerminalFence(runningRecord, readAdmissionClaim(job.workspaceRoot, job.id)); } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index f91dff4fb..b7021b000 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1220,6 +1220,17 @@ test("reconciliation fails a running job with an invalid process id", () => { assert.match(result.errorMessage, /invalid process id/i); }); +test("reconciliation fails a queued job without a valid creation time", () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-invalid-queued-time", workspaceRoot, status: "queued", pid: null, createdAt: "not-a-date" }; + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + + const [result] = reconcileTrackedJobs(workspaceRoot); + assert.equal(result.status, "failed"); + assert.match(result.errorMessage, /invalid creation time/i); +}); + test("removed job terminal fence prevents progress and finalization from recreating it", async () => { const workspaceRoot = makeTempDir(); const job = { id: "task-removed", workspaceRoot, status: "queued" }; @@ -1247,6 +1258,34 @@ test("removed job terminal fence prevents progress and finalization from recreat assert.equal(fs.existsSync(resolveJobFile(workspaceRoot, job.id)), false); }); +test("removal between the pre-admission check and claim prevents runner invocation", async () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-remove-before-admission", workspaceRoot, status: "queued" }; + const jobFile = writeJobFile(workspaceRoot, job.id, job); + const removedFile = jobFile.replace(/\.json$/, ".removed"); + upsertJob(workspaceRoot, job); + const originalExists = fs.existsSync; + let removalChecks = 0; + let invoked = false; + fs.existsSync = (file) => { + if (file === removedFile && ++removalChecks === 2) { + fs.writeFileSync(removedFile, "", "utf8"); + return false; + } + return originalExists(file); + }; + try { + const result = await runTrackedJob(job, async () => { + invoked = true; + return { exitStatus: 0 }; + }); + assert.equal(result.removed, true); + } finally { + fs.existsSync = originalExists; + } + assert.equal(invoked, false); +}); + test("review rejects focus text because it is native-review only", () => { const repo = makeTempDir(); const binDir = makeTempDir(); @@ -2248,9 +2287,9 @@ test("session end fully cleans up jobs for the ending session", async (t) => { const jobsDir = path.join(stateDir, "jobs"); fs.mkdirSync(jobsDir, { recursive: true }); - const completedLog = path.join(jobsDir, "completed.log"); - const runningLog = path.join(jobsDir, "running.log"); - const otherSessionLog = path.join(jobsDir, "other.log"); + const completedLog = resolveJobLogFile(repo, "review-completed"); + const runningLog = resolveJobLogFile(repo, "review-running"); + const otherSessionLog = resolveJobLogFile(repo, "review-other"); const completedJobFile = path.join(jobsDir, "review-completed.json"); const runningJobFile = path.join(jobsDir, "review-running.json"); const otherJobFile = path.join(jobsDir, "review-other.json"); @@ -2370,7 +2409,12 @@ test("session end fully cleans up jobs for the ending session", async (t) => { saveState(repo, state); assert.deepEqual( fs.readdirSync(jobsDir).sort(), - [path.basename(otherJobFile), path.basename(otherSessionLog), "review-completed.removed", "review-running.removed"].sort() + [ + path.basename(otherJobFile), + path.basename(otherSessionLog), + "review-completed.removed", + "review-running.removed" + ].sort() ); }); @@ -2595,7 +2639,7 @@ test("stop hook blocks every matching non-reusable gate job without starting a t saveState(repo, { version: 1, config: { stopReviewGate: true }, - jobs: [{ id: jobId, gateKey, sessionId, status, title: "Codex Stop Gate Review", ...(status === "running" ? { pid: process.pid } : {}), ...(cachedResult === undefined ? {} : { result: cachedResult }) }] + jobs: [{ id: jobId, gateKey, sessionId, status, title: "Codex Stop Gate Review", ...(status === "queued" ? { createdAt: new Date().toISOString() } : {}), ...(status === "running" ? { pid: process.pid } : {}), ...(cachedResult === undefined ? {} : { result: cachedResult }) }] }); const beforeTurns = fs.existsSync(fakeStatePath) ? JSON.parse(fs.readFileSync(fakeStatePath, "utf8")).nextTurnId : 1; const hookResult = run("node", [STOP_HOOK], { cwd: repo, env, input: JSON.stringify({ cwd: repo, session_id: sessionId, last_assistant_message: message }) }); diff --git a/tests/state.test.mjs b/tests/state.test.mjs index 064046b2d..eaa605dfc 100644 --- a/tests/state.test.mjs +++ b/tests/state.test.mjs @@ -7,11 +7,13 @@ import assert from "node:assert/strict"; import { makeTempDir } from "./helpers.mjs"; import { loadState, + listJobs, resolveJobFile, resolveJobLogFile, resolveStateDir, resolveStateFile, saveState, + upsertJob, writeJobFile } from "../plugins/codex/scripts/lib/state.mjs"; @@ -106,6 +108,42 @@ test("saveState reaps a lock left by a dead owner", () => { assert.equal(fs.existsSync(path.join(stateDir, ".state.lock")), false); }); +test("saveState bounds waiting when a dead lock cannot acquire its reap guard", () => { + const workspace = makeTempDir(); + const stateDir = resolveStateDir(workspace); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync(path.join(stateDir, ".state.lock"), JSON.stringify({ pid: 999999, token: "dead-owner", createdAt: "2026-08-19T12:00:00.000Z" }), "utf8"); + fs.writeFileSync(path.join(stateDir, ".state.lock.reap"), JSON.stringify({ pid: process.pid, token: "busy-reaper", createdAt: "2026-08-19T12:00:00.000Z" }), "utf8"); + const originalNow = Date.now; + let calls = 0; + Date.now = () => (calls++ === 0 ? 0 : 5001); + try { + assert.throws(() => saveState(workspace, { config: { stopReviewGate: false }, jobs: [] }), /Timed out waiting for Codex Companion state lock/); + } finally { + Date.now = originalNow; + } +}); + +test("loadState rejects unsafe job ids", () => { + const workspace = makeTempDir(); + const stateFile = resolveStateFile(workspace); + fs.mkdirSync(path.dirname(stateFile), { recursive: true }); + fs.writeFileSync(stateFile, JSON.stringify({ version: 1, config: { stopReviewGate: false }, jobs: [{ id: "../escape", status: "completed" }] }), "utf8"); + assert.throws(() => loadState(workspace), /invalid job schema/); +}); + +test("saveState rejects an unsafe requested job id before touching its path", () => { + const workspace = makeTempDir(); + const sentinel = path.join(workspace, "sentinel"); + fs.writeFileSync(sentinel, "keep", "utf8"); + + assert.throws( + () => saveState(workspace, { config: { stopReviewGate: false }, jobs: [{ id: "../sentinel", status: "completed" }] }), + /invalid job schema/ + ); + assert.equal(fs.readFileSync(sentinel, "utf8"), "keep"); +}); + test("saveState prunes dropped job artifacts when indexed jobs exceed the cap", () => { const workspace = makeTempDir(); const stateFile = resolveStateFile(workspace); @@ -190,3 +228,44 @@ test("saveState retains a removal fence for a pruned job before it starts", () = assert.equal(fs.existsSync(job.logFile), false); assert.equal(fs.existsSync(jobFile.replace(/\.json$/, ".removed")), true); }); + +test("saveState does not delete a noncanonical job log path", () => { + const workspace = makeTempDir(); + const externalLog = path.join(makeTempDir(), "outside.log"); + const job = { id: "job-external-log", status: "completed", logFile: externalLog }; + writeJobFile(workspace, job.id, job); + fs.writeFileSync(externalLog, "keep\n", "utf8"); + saveState(workspace, { config: { stopReviewGate: false }, jobs: [job] }); + + saveState(workspace, { config: { stopReviewGate: false }, jobs: [] }); + + assert.equal(fs.existsSync(externalLog), true); +}); + +test("saveState retains active jobs beyond the terminal history cap", () => { + const workspace = makeTempDir(); + const jobs = [ + { id: "active-old", status: "running", pid: process.pid, updatedAt: "2020-01-01T00:00:00.000Z" }, + ...Array.from({ length: 50 }, (_, index) => ({ id: `done-${index}`, status: "completed", updatedAt: new Date(Date.UTC(2026, 0, 1, 0, index, 0)).toISOString() })) + ]; + const saved = saveState(workspace, { config: { stopReviewGate: false }, jobs }); + assert.equal(saved.jobs.length, 50); + assert.equal(saved.jobs.some((job) => job.id === "active-old"), true); +}); + +test("late publication cannot restore a job behind its removal fence", () => { + const workspace = makeTempDir(); + const job = { id: "job-removed-late", status: "queued", logFile: resolveJobLogFile(workspace, "job-removed-late") }; + const jobFile = writeJobFile(workspace, job.id, job); + upsertJob(workspace, job); + fs.writeFileSync(jobFile.replace(/\.json$/, ".started.json"), JSON.stringify({ status: "running", pid: process.pid, startedAt: new Date().toISOString() }), "utf8"); + fs.writeFileSync(jobFile.replace(/\.json$/, ".removed"), "", "utf8"); + + writeJobFile(workspace, job.id, { ...job, status: "running", pid: process.pid }); + upsertJob(workspace, { ...job, status: "running", pid: process.pid }); + + assert.deepEqual(listJobs(workspace), []); + assert.equal(fs.existsSync(jobFile), false); + assert.equal(fs.existsSync(jobFile.replace(/\.json$/, ".started.json")), false); + assert.equal(fs.existsSync(jobFile.replace(/\.json$/, ".removed")), true); +}); From 7d3f7057608b7a8687013f8d568ebd8888fa8606 Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 17:50:11 +0300 Subject: [PATCH 18/19] fix: authorize terminal Codex outcomes --- .../2026-08-19-fail-closed-codex-gate.md | 3 +- ...026-08-19-fail-closed-codex-gate-design.md | 2 ++ plugins/codex/CHANGELOG.md | 2 +- plugins/codex/scripts/lib/state.mjs | 4 +++ plugins/codex/scripts/lib/tracked-jobs.mjs | 5 ++- tests/runtime.test.mjs | 33 +++++++++++++++++-- 6 files changed, 44 insertions(+), 5 deletions(-) diff --git a/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md b/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md index d04d00a3c..ac823cc2c 100644 --- a/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md +++ b/docs/superpowers/plans/2026-08-19-fail-closed-codex-gate.md @@ -199,6 +199,7 @@ Expected: PASS. **Interfaces:** - Produces: same-directory temporary write plus `renameSync` for state and job JSON. +- Produces: bounded per-workspace serialization for state read-modify-write mutations. - Produces: explicit parse errors from invalid state JSON. - [ ] **Step 1: Write failing persistence tests** @@ -213,7 +214,7 @@ Expected: `loadState` silently returns defaults and the hook emits no block deci - [ ] **Step 3: Implement atomic strict persistence** -Write JSON through a unique sibling temporary file and `fs.renameSync`. On parse failure throw `Failed to read Codex Companion state at : ` instead of returning defaults. Catch top-level Stop-hook errors and emit a block decision. +Write JSON through a unique sibling temporary file and `fs.renameSync`. Serialize state read-modify-write mutations with an exclusive per-workspace lock, recover dead owners, and fail clearly after five seconds of contention. On parse failure throw `Failed to read Codex Companion state at : ` instead of returning defaults. Catch top-level Stop-hook errors and emit a block decision. - [ ] **Step 4: Verify Task 4 GREEN** diff --git a/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md b/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md index 5ee6dc0a6..b643e79e6 100644 --- a/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md +++ b/docs/superpowers/specs/2026-08-19-fail-closed-codex-gate-design.md @@ -41,7 +41,9 @@ Make Codex Companion jobs self-heal after worker loss and make the Claude Stop r ### Persistence - `state.json` and per-job JSON files are written to a same-directory temporary file and atomically renamed. +- State read-modify-write mutations use a short per-workspace filesystem lock. Dead owners are recovered; contention or invalid lock metadata fails with a clear error after five seconds instead of hanging. - Invalid persisted JSON is an explicit error. It must not silently reset `stopReviewGate` to `false`. +- Removed job IDs keep a zero-byte tombstone so an arbitrarily late worker cannot reuse them. This is the deliberate correctness tradeoff for avoiding a daemon, lease, heartbeat, or attempt-token protocol. - No new daemon, dependency, heartbeat file, or long-lived lock is introduced. PID liveness covers the observed worker-loss failure; the existing 15-minute Stop timeout covers a live but non-returning gate review. ## Verification diff --git a/plugins/codex/CHANGELOG.md b/plugins/codex/CHANGELOG.md index 81ced9089..8937d6ee1 100644 --- a/plugins/codex/CHANGELOG.md +++ b/plugins/codex/CHANGELOG.md @@ -5,7 +5,7 @@ - Hardened background supervision: queue records are published before spawn, and queued or running jobs with missing/dead workers are reconciled to failure. - Added immutable startup, admission, terminal, and removal claims so late workers cannot resurrect jobs or execute duplicates. - Stop-gate reviews now single-flight per exact Claude turn, reuse the same-turn result, and fail closed for unavailable or corrupt persistence. -- Made mutable state and job JSON writes atomic. +- Made mutable state and job JSON writes atomic, and serialized state mutations behind a bounded crash-recovering workspace lock. ## 1.0.0 diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs index 693108f82..a3471e448 100644 --- a/plugins/codex/scripts/lib/state.mjs +++ b/plugins/codex/scripts/lib/state.mjs @@ -297,6 +297,10 @@ export function updateState(cwd, mutate) { }); } +export function isJobRemovedLocked(cwd, jobId) { + return withStateLock(cwd, () => fs.existsSync(resolveJobSidecarFile(cwd, jobId, ".removed"))); +} + export function generateJobId(prefix = "job") { const random = Math.random().toString(36).slice(2, 8); return `${prefix}-${Date.now().toString(36)}-${random}`; diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 6c68325a9..2c4c24a05 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -2,7 +2,7 @@ import fs from "node:fs"; import process from "node:process"; import { isProcessAlive } from "./process.mjs"; -import { listJobs, readJobFile, resolveJobFile, resolveJobLogFile, updateState, upsertJob, writeJobFile } from "./state.mjs"; +import { isJobRemovedLocked, listJobs, readJobFile, resolveJobFile, resolveJobLogFile, updateState, upsertJob, writeJobFile } from "./state.mjs"; export const SESSION_ID_ENV = "CODEX_COMPANION_SESSION_ID"; export const GATE_KEY_ENV = "CODEX_COMPANION_GATE_KEY"; @@ -521,6 +521,9 @@ export async function runTrackedJob(job, runner, options = {}) { rendered: execution.rendered }); if (terminal.claimed) { + if (isJobRemovedLocked(job.workspaceRoot, job.id)) { + return removedLifecycleRecord(runningRecord); + } appendLogBlock(options.logFile ?? job.logFile ?? null, "Final output", execution.rendered); return execution; } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index b7021b000..a1e46c563 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1209,6 +1209,33 @@ test("terminal job reconciliation wins over late worker finalization", async () assert.equal(readJobFile(resolveJobFile(workspaceRoot, job.id)).status, "failed"); }); +test("removed fence claimed during completion suppresses stale runner output", async () => { + const workspaceRoot = makeTempDir(); + const job = { id: "task-remove-at-terminal", workspaceRoot, status: "queued" }; + const jobFile = writeJobFile(workspaceRoot, job.id, job); + const terminalFile = resolveTerminalFenceFile(workspaceRoot, job.id); + const removedFile = jobFile.replace(/\.json$/, ".removed"); + upsertJob(workspaceRoot, job); + const originalOpen = fs.openSync; + let runnerInvoked = false; + fs.openSync = (file, flags, ...rest) => { + if (file === terminalFile && flags === "wx" && !fs.existsSync(removedFile)) { + fs.writeFileSync(removedFile, "", "utf8"); + } + return originalOpen(file, flags, ...rest); + }; + try { + const result = await runTrackedJob(job, async () => { + runnerInvoked = true; + return { exitStatus: 0, payload: { rawOutput: "ALLOW: stale" }, rendered: "ALLOW: stale" }; + }); + assert.equal(result.removed, true); + } finally { + fs.openSync = originalOpen; + } + assert.equal(runnerInvoked, true); +}); + test("reconciliation fails a running job with an invalid process id", () => { const workspaceRoot = makeTempDir(); const job = { id: "task-invalid-pid", workspaceRoot, status: "running", pid: null }; @@ -2676,7 +2703,7 @@ test("stop hook blocks a parsed state with an invalid schema", () => { assert.match(decision.reason, /could not safely continue.*invalid state schema/i); }); -test("stop gate blocks when its foreground review loses the terminal claim", () => { +test("stop gate blocks when removal wins during foreground completion", () => { const repo = makeTempDir(); const binDir = makeTempDir(); installFakeCodex(binDir, "adversarial-clean"); @@ -2686,6 +2713,7 @@ test("stop gate blocks when its foreground review loses the terminal claim", () const gateKey = createHash("sha256").update(`${sessionId}\0${message}`).digest("hex"); const jobId = `gate-${gateKey}`; const terminalFile = resolveTerminalFenceFile(repo, jobId); + const removedFile = terminalFile.replace(/\.terminal\.json$/, ".removed"); const preload = path.join(makeTempDir(), "terminal-race.cjs"); fs.writeFileSync(preload, [ 'const fs = require("node:fs");', @@ -2693,7 +2721,7 @@ test("stop gate blocks when its foreground review loses the terminal claim", () 'let armed = true;', 'fs.openSync = (file, flags, ...rest) => {', ' if (armed && file === process.env.CODEX_FOREGROUND_TERMINAL_FENCE && flags === "wx") {', - ' armed = false; fs.writeFileSync(file, JSON.stringify({ status: "cancelled", completedAt: "2026-08-19T12:01:00.000Z" }));', + ' armed = false; fs.writeFileSync(process.env.CODEX_FOREGROUND_REMOVED_FENCE, "");', ' }', ' return original(file, flags, ...rest);', '};' @@ -2701,6 +2729,7 @@ test("stop gate blocks when its foreground review loses the terminal claim", () const env = { ...buildEnv(binDir), CODEX_FOREGROUND_TERMINAL_FENCE: terminalFile, + CODEX_FOREGROUND_REMOVED_FENCE: removedFile, NODE_OPTIONS: `--require ${preload}` }; assert.equal(run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { cwd: repo, env }).status, 0); From 5379993feb9bc8b742726ec2da5d7ca3544de318 Mon Sep 17 00:00:00 2001 From: gon7187 Date: Wed, 19 Aug 2026 17:54:49 +0300 Subject: [PATCH 19/19] test: mark live Codex fixtures with pids --- tests/runtime.test.mjs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index a1e46c563..716a01e4b 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1452,6 +1452,7 @@ test("status shows phases, hints, and the latest finished job", () => { kind: "review", kindLabel: "review", status: "running", + pid: process.pid, title: "Codex Review", jobClass: "review", phase: "reviewing", @@ -1595,6 +1596,7 @@ test("status preserves adversarial review kind labels", () => { id: "review-adv-live", kind: "adversarial-review", status: "running", + pid: process.pid, title: "Codex Adversarial Review", jobClass: "review", phase: "reviewing", @@ -1670,6 +1672,7 @@ test("status --wait times out cleanly when a job is still active", () => { { id: "task-live", status: "running", + pid: process.pid, title: "Codex Task", jobClass: "task", summary: "Investigate flaky test", @@ -2097,6 +2100,7 @@ test("cancel without a job id ignores active jobs from other Claude sessions", ( { id: "task-other", status: "running", + pid: process.pid, title: "Codex Task", jobClass: "task", sessionId: "sess-other", @@ -2134,13 +2138,26 @@ test("cancel without a job id ignores active jobs from other Claude sessions", ( assert.equal(state.jobs[0].status, "running"); }); -test("cancel with a job id can still target an active job from another Claude session", () => { +test("cancel with a job id can still target an active job from another Claude session", (t) => { const workspace = makeTempDir(); const stateDir = resolveStateDir(workspace); const jobsDir = path.join(stateDir, "jobs"); fs.mkdirSync(jobsDir, { recursive: true }); const logFile = path.join(jobsDir, "task-other.log"); + const sleeper = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { detached: true, stdio: "ignore" }); + sleeper.unref(); + t.after(() => { + try { + process.kill(-sleeper.pid, "SIGTERM"); + } catch { + try { + process.kill(sleeper.pid, "SIGTERM"); + } catch { + // Ignore missing process. + } + } + }); fs.writeFileSync(logFile, "", "utf8"); fs.writeFileSync( path.join(stateDir, "state.json"), @@ -2152,6 +2169,7 @@ test("cancel with a job id can still target an active job from another Claude se { id: "task-other", status: "running", + pid: sleeper.pid, title: "Codex Task", jobClass: "task", sessionId: "sess-other", @@ -2842,6 +2860,7 @@ test("stop hook logs running tasks to stderr without blocking when the review ga { id: "task-live", status: "running", + pid: process.pid, title: "Codex Task", jobClass: "task", sessionId: "sess-current",