From e494a8e9aa2aa1fb32486781cf608e697c1df9b4 Mon Sep 17 00:00:00 2001 From: Julian Alvarado Date: Thu, 28 May 2026 12:50:19 -0600 Subject: [PATCH 01/51] fix: keep background jobs alive across SessionEnd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--background` is supposed to mean "outlive the session that started me", but `session-lifecycle-hook.mjs:cleanupSessionJobs` was unconditionally terminating every running job belonging to the ending session — including the detached worker that `enqueueBackgroundTask` had just spawned. The result: any background task dispatched from a subagent (whose SessionEnd fires as soon as its turn finishes) was SIGTERM'd a few seconds in. The JSONL transcript froze at the kill timestamp, the parent session's later status probe found no live job, and the caller never got a result. Same hook then also tore down the broker the worker depended on. Fix: * Tag jobs created via `enqueueBackgroundTask` with `background: true`. * `cleanupSessionJobs` now skips termination for background jobs and preserves their entry in `state.json` so any session in the workspace can still poll for status / fetch results. * `handleSessionEnd` defers broker shutdown when active background jobs are still present in the workspace — the broker outlives this session until the last background worker is done with it. Adds a runtime test that mixes a background and a foreground job under the same sessionId, drives SessionEnd, and asserts the foreground worker is killed + its state pruned while the background worker stays alive and remains in state. --- plugins/codex/scripts/codex-companion.mjs | 1 + .../codex/scripts/session-lifecycle-hook.mjs | 40 +++++- tests/runtime.test.mjs | 132 ++++++++++++++++++ 3 files changed, 168 insertions(+), 5 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 35222fd5a..76261830f 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -660,6 +660,7 @@ function enqueueBackgroundTask(cwd, job, request) { ...job, status: "queued", phase: "queued", + background: true, pid: child.pid ?? null, logFile, request diff --git a/plugins/codex/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs index 9655eaef4..4638f1696 100644 --- a/plugins/codex/scripts/session-lifecycle-hook.mjs +++ b/plugins/codex/scripts/session-lifecycle-hook.mjs @@ -50,12 +50,18 @@ function cleanupSessionJobs(cwd, sessionId) { } const state = loadState(workspaceRoot); - const removedJobs = state.jobs.filter((job) => job.sessionId === sessionId); - if (removedJobs.length === 0) { + const sessionJobs = state.jobs.filter((job) => job.sessionId === sessionId); + if (sessionJobs.length === 0) { return; } - for (const job of removedJobs) { + for (const job of sessionJobs) { + // Background jobs are explicitly dispatched to outlive the session that + // started them. Leave them running and leave their state entry intact so + // any session in the workspace can still poll for status/results. + if (job.background) { + continue; + } const stillRunning = job.status === "queued" || job.status === "running"; if (!stillRunning) { continue; @@ -69,10 +75,25 @@ function cleanupSessionJobs(cwd, sessionId) { saveState(workspaceRoot, { ...state, - jobs: state.jobs.filter((job) => job.sessionId !== sessionId) + jobs: state.jobs.filter((job) => job.sessionId !== sessionId || job.background) }); } +function hasActiveBackgroundJobs(cwd) { + if (!cwd) { + return false; + } + const workspaceRoot = resolveWorkspaceRoot(cwd); + const stateFile = resolveStateFile(workspaceRoot); + if (!fs.existsSync(stateFile)) { + return false; + } + const state = loadState(workspaceRoot); + return state.jobs.some( + (job) => job.background && (job.status === "queued" || job.status === "running") + ); +} + function handleSessionStart(input) { appendEnvVar(SESSION_ID_ENV, input.session_id); appendEnvVar(PLUGIN_DATA_ENV, process.env[PLUGIN_DATA_ENV]); @@ -95,11 +116,20 @@ async function handleSessionEnd(input) { const sessionDir = brokerSession?.sessionDir ?? null; const pid = brokerSession?.pid ?? null; + cleanupSessionJobs(cwd, input.session_id || process.env[SESSION_ID_ENV]); + + // Detached background workers depend on the broker for codex app-server + // calls. If any background jobs are still active in this workspace, leave + // the broker running — a later SessionEnd (or the workers themselves) will + // tear it down when nothing depends on it anymore. + if (hasActiveBackgroundJobs(cwd)) { + return; + } + if (brokerEndpoint) { await sendBrokerShutdown(brokerEndpoint); } - cleanupSessionJobs(cwd, input.session_id || process.env[SESSION_ID_ENV]); teardownBrokerSession({ endpoint: brokerEndpoint, pidFile, diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 90408372f..96b273634 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1787,6 +1787,138 @@ test("session end fully cleans up jobs for the ending session", async (t) => { assert.equal(otherJob.logFile, otherSessionLog); }); +test("session end preserves background jobs and their broker so workers survive their dispatching session", async (t) => { + const repo = makeTempDir(); + 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 stateDir = resolveStateDir(repo); + const jobsDir = path.join(stateDir, "jobs"); + fs.mkdirSync(jobsDir, { recursive: true }); + + const backgroundLog = path.join(jobsDir, "background.log"); + const foregroundLog = path.join(jobsDir, "foreground.log"); + const backgroundJobFile = path.join(jobsDir, "task-background.json"); + const foregroundJobFile = path.join(jobsDir, "review-foreground.json"); + fs.writeFileSync(backgroundLog, "background\n", "utf8"); + fs.writeFileSync(foregroundLog, "foreground\n", "utf8"); + fs.writeFileSync(backgroundJobFile, JSON.stringify({ id: "task-background" }, null, 2), "utf8"); + fs.writeFileSync(foregroundJobFile, JSON.stringify({ id: "review-foreground" }, null, 2), "utf8"); + + const backgroundSleeper = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { + cwd: repo, + detached: true, + stdio: "ignore" + }); + backgroundSleeper.unref(); + const foregroundSleeper = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { + cwd: repo, + detached: true, + stdio: "ignore" + }); + foregroundSleeper.unref(); + + t.after(() => { + for (const proc of [backgroundSleeper, foregroundSleeper]) { + try { + process.kill(-proc.pid, "SIGTERM"); + } catch { + try { + process.kill(proc.pid, "SIGTERM"); + } catch { + // Ignore missing process. + } + } + } + }); + + fs.writeFileSync( + path.join(stateDir, "state.json"), + `${JSON.stringify( + { + version: 1, + config: { stopReviewGate: false }, + jobs: [ + { + id: "task-background", + status: "running", + title: "Codex Task", + sessionId: "sess-current", + background: true, + pid: backgroundSleeper.pid, + logFile: backgroundLog, + createdAt: "2026-03-18T15:30:00.000Z", + updatedAt: "2026-03-18T15:31:00.000Z" + }, + { + id: "review-foreground", + status: "running", + title: "Codex Review", + sessionId: "sess-current", + pid: foregroundSleeper.pid, + logFile: foregroundLog, + createdAt: "2026-03-18T15:32:00.000Z", + updatedAt: "2026-03-18T15:33:00.000Z" + } + ] + }, + null, + 2 + )}\n`, + "utf8" + ); + + const result = run("node", [SESSION_HOOK, "SessionEnd"], { + cwd: repo, + env: { + ...process.env, + CODEX_COMPANION_SESSION_ID: "sess-current" + }, + input: JSON.stringify({ + hook_event_name: "SessionEnd", + session_id: "sess-current", + cwd: repo + }) + }); + + assert.equal(result.status, 0, result.stderr); + + // Foreground job killed + pruned from state. + await waitFor(() => { + try { + process.kill(foregroundSleeper.pid, 0); + return false; + } catch (error) { + return error?.code === "ESRCH"; + } + }); + + // Background job still alive — its worker outlives the session that started it. + assert.equal( + (() => { + try { + process.kill(backgroundSleeper.pid, 0); + return true; + } catch { + return false; + } + })(), + true, + "background job worker should not be terminated by SessionEnd" + ); + + const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + assert.deepEqual( + state.jobs.map((job) => job.id), + ["task-background"], + "background job stays in state so later sessions can poll it" + ); + assert.equal(fs.existsSync(backgroundJobFile), true, "background job file preserved"); + assert.equal(fs.existsSync(backgroundLog), true, "background log preserved"); +}); + test("stop hook runs a stop-time review task and blocks on findings when the review gate is enabled", () => { const repo = makeTempDir(); const binDir = makeTempDir(); From 758072892b937477f811ab9080babf159cbb0312 Mon Sep 17 00:00:00 2001 From: greenbauer Date: Fri, 3 Jul 2026 20:51:44 -0500 Subject: [PATCH 02/51] fix: reap ghost jobs whose worker died without recording a result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A detached task worker that dies without throwing (unhandled rejection, OOM kill, native crash) never reaches runTrackedJob's catch, so its job file stays status:"running" forever and /codex:status reports a ghost job indefinitely. Two complementary guards: - registerWorkerCrashGuard (worker side): installed in handleTaskWorker before runTrackedJob; marks the job failed on uncaughtException, unhandledRejection, SIGTERM, SIGINT, or SIGHUP, then exits. - reapDeadJobs (reader side): wraps every listJobs call in job-control so status/result/cancel probe each active job's recorded pid with process.kill(pid, 0); ESRCH means the worker is gone and the job is rewritten as failed with a resume hint. The job file is re-read first so a job that finished between the read and the probe keeps its real result, and EPERM (alive but not ours) is treated as alive. Test run: node --test — 5 new tests pass; the 4 pre-existing failures on macOS (tmpdir symlink) are unchanged from main. Co-Authored-By: Claude Fable 5 --- plugins/codex/scripts/codex-companion.mjs | 2 + plugins/codex/scripts/lib/job-control.mjs | 10 +-- plugins/codex/scripts/lib/tracked-jobs.mjs | 81 ++++++++++++++++++ tests/tracked-jobs.test.mjs | 98 ++++++++++++++++++++++ 4 files changed, 186 insertions(+), 5 deletions(-) create mode 100644 tests/tracked-jobs.test.mjs diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..70f45ea2a 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -49,6 +49,7 @@ import { createJobRecord, createProgressReporter, nowIso, + registerWorkerCrashGuard, runTrackedJob, SESSION_ID_ENV } from "./lib/tracked-jobs.mjs"; @@ -865,6 +866,7 @@ async function handleTaskWorker(argv) { logFile: storedJob.logFile ?? null } ); + registerWorkerCrashGuard(workspaceRoot, options["job-id"], logFile); await runTrackedJob( { ...storedJob, diff --git a/plugins/codex/scripts/lib/job-control.mjs b/plugins/codex/scripts/lib/job-control.mjs index ad152c157..68ab7db1b 100644 --- a/plugins/codex/scripts/lib/job-control.mjs +++ b/plugins/codex/scripts/lib/job-control.mjs @@ -2,7 +2,7 @@ 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 { reapDeadJobs, 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(reapDeadJobs(workspaceRoot, listJobs(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(reapDeadJobs(workspaceRoot, listJobs(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,7 @@ 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 jobs = sortJobsNewestFirst(reference ? reapDeadJobs(workspaceRoot, listJobs(workspaceRoot)) : filterJobsForCurrentSession(reapDeadJobs(workspaceRoot, listJobs(workspaceRoot)))); const selected = matchJobReference( jobs, reference, @@ -280,7 +280,7 @@ export function resolveResultJob(cwd, reference) { export function resolveCancelableJob(cwd, reference, options = {}) { const workspaceRoot = resolveWorkspaceRoot(cwd); - const jobs = sortJobsNewestFirst(listJobs(workspaceRoot)); + const jobs = sortJobsNewestFirst(reapDeadJobs(workspaceRoot, listJobs(workspaceRoot))); const activeJobs = jobs.filter((job) => job.status === "queued" || job.status === "running"); if (reference) { diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 902869012..dcbbc5d09 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -202,3 +202,84 @@ export async function runTrackedJob(job, runner, options = {}) { throw error; } } + +function isPidAlive(pid) { + if (!Number.isInteger(pid) || pid <= 0) { + return null; + } + try { + process.kill(pid, 0); + return true; + } catch (error) { + // ESRCH: no such process. EPERM: exists but not ours — treat as alive. + return error?.code === "ESRCH" ? false : true; + } +} + +function markJobDead(workspaceRoot, jobSummary, errorMessage) { + const jobFile = resolveJobFile(workspaceRoot, jobSummary.id); + const stored = fs.existsSync(jobFile) ? readJobFile(jobFile) : null; + const base = stored ?? jobSummary; + if (base.status !== "running" && base.status !== "queued") { + // The job finished between the caller's read and now — keep the real result. + return base; + } + const completedAt = nowIso(); + const record = { + ...base, + status: "failed", + phase: "failed", + errorMessage, + pid: null, + completedAt + }; + writeJobFile(workspaceRoot, jobSummary.id, record); + upsertJob(workspaceRoot, { + id: jobSummary.id, + status: "failed", + phase: "failed", + pid: null, + errorMessage, + completedAt + }); + appendLogLine(base.logFile ?? null, `Marked failed: ${errorMessage}`); + return record; +} + +// A worker that dies without throwing (SIGKILL, OOM, native crash) never +// reaches runTrackedJob's catch, so its job stays "running" forever. Rewrite +// any active job whose recorded pid is no longer alive as failed. +export function reapDeadJobs(workspaceRoot, jobs) { + return jobs.map((job) => { + if (job.status !== "running" && job.status !== "queued") { + return job; + } + if (isPidAlive(job.pid) === false) { + return markJobDead( + workspaceRoot, + job, + `worker process (pid ${job.pid}) died without recording a result — ` + + `likely a crash; resume with task --resume-last` + ); + } + return job; + }); +} + +export function registerWorkerCrashGuard(workspaceRoot, jobId, logFile = null) { + const mark = (label) => (reason) => { + try { + const detail = reason instanceof Error ? reason.stack ?? reason.message : String(reason ?? ""); + appendLogLine(logFile, `Worker ${label}: ${detail}`); + markJobDead(workspaceRoot, { id: jobId, status: "running", logFile }, `worker ${label}: ${detail.split("\n")[0]}`); + } catch { + // Never let the guard itself throw during teardown. + } + process.exit(1); + }; + process.on("uncaughtException", mark("uncaughtException")); + process.on("unhandledRejection", mark("unhandledRejection")); + for (const sig of ["SIGTERM", "SIGINT", "SIGHUP"]) { + process.on(sig, mark(`received ${sig}`)); + } +} diff --git a/tests/tracked-jobs.test.mjs b/tests/tracked-jobs.test.mjs new file mode 100644 index 000000000..ad7219125 --- /dev/null +++ b/tests/tracked-jobs.test.mjs @@ -0,0 +1,98 @@ +import fs from "node:fs"; +import path from "node:path"; +import test from "node:test"; +import assert from "node:assert/strict"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +import { makeTempDir, run } from "./helpers.mjs"; +import { reapDeadJobs } from "../plugins/codex/scripts/lib/tracked-jobs.mjs"; +import { listJobs, readJobFile, resolveJobFile, upsertJob, writeJobFile } from "../plugins/codex/scripts/lib/state.mjs"; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const TRACKED_JOBS_URL = pathToFileURL(path.join(ROOT, "plugins", "codex", "scripts", "lib", "tracked-jobs.mjs")).href; + +function seedJob(workspace, job) { + writeJobFile(workspace, job.id, job); + upsertJob(workspace, job); +} + +function spawnDeadPid() { + const result = run(process.execPath, ["-e", ""]); + assert.equal(result.status, 0); + return result.pid; +} + +test("reapDeadJobs marks a running job with a dead pid as failed", () => { + const workspace = makeTempDir(); + seedJob(workspace, { id: "job-dead", status: "running", phase: "delegating", pid: spawnDeadPid(), logFile: null }); + + const reaped = reapDeadJobs(workspace, listJobs(workspace)); + + assert.equal(reaped.length, 1); + assert.equal(reaped[0].status, "failed"); + assert.equal(reaped[0].pid, null); + assert.match(reaped[0].errorMessage, /died without recording a result/); + + const stored = readJobFile(resolveJobFile(workspace, "job-dead")); + assert.equal(stored.status, "failed"); + assert.equal(stored.pid, null); + assert.equal(listJobs(workspace).find((job) => job.id === "job-dead").status, "failed"); +}); + +test("reapDeadJobs leaves a running job with a live pid untouched", () => { + const workspace = makeTempDir(); + seedJob(workspace, { id: "job-live", status: "running", phase: "delegating", pid: process.pid, logFile: null }); + + const reaped = reapDeadJobs(workspace, listJobs(workspace)); + + assert.equal(reaped[0].status, "running"); + assert.equal(readJobFile(resolveJobFile(workspace, "job-live")).status, "running"); +}); + +test("reapDeadJobs leaves jobs without a recorded pid untouched", () => { + const workspace = makeTempDir(); + seedJob(workspace, { id: "job-no-pid", status: "queued", phase: "queued", pid: null, logFile: null }); + + const reaped = reapDeadJobs(workspace, listJobs(workspace)); + + assert.equal(reaped[0].status, "queued"); +}); + +test("reapDeadJobs keeps the stored result when the job finished between read and probe", () => { + const workspace = makeTempDir(); + const deadPid = spawnDeadPid(); + writeJobFile(workspace, "job-raced", { id: "job-raced", status: "completed", phase: "completed", result: "done" }); + upsertJob(workspace, { id: "job-raced", status: "running", phase: "delegating", pid: deadPid, logFile: null }); + + const reaped = reapDeadJobs(workspace, [{ id: "job-raced", status: "running", pid: deadPid, logFile: null }]); + + assert.equal(reaped[0].status, "completed"); + assert.equal(reaped[0].result, "done"); + assert.equal(readJobFile(resolveJobFile(workspace, "job-raced")).status, "completed"); +}); + +test("registerWorkerCrashGuard marks the job failed when the worker dies on an unhandled rejection", () => { + const workspace = makeTempDir(); + seedJob(workspace, { id: "job-crash", status: "running", phase: "delegating", pid: null, logFile: null }); + + const workerFile = path.join(makeTempDir(), "crashing-worker.mjs"); + fs.writeFileSync( + workerFile, + [ + `import { registerWorkerCrashGuard } from ${JSON.stringify(TRACKED_JOBS_URL)};`, + "registerWorkerCrashGuard(process.argv[2], process.argv[3], null);", + 'Promise.reject(new Error("boom"));', + "setTimeout(() => {}, 5000);", + "" + ].join("\n"), + "utf8" + ); + + const result = run(process.execPath, [workerFile, workspace, "job-crash"]); + + assert.equal(result.status, 1); + const stored = readJobFile(resolveJobFile(workspace, "job-crash")); + assert.equal(stored.status, "failed"); + assert.match(stored.errorMessage, /unhandledRejection/); + assert.match(stored.errorMessage, /boom/); +}); From 954af53dde799fff92f06c7fa2852f3b562a96fb Mon Sep 17 00:00:00 2001 From: greenbauer Date: Wed, 8 Jul 2026 21:49:09 -0500 Subject: [PATCH 03/51] fix: refresh updatedAt when reaping a dead job so it sorts newest-first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit markJobDead built the returned record by spreading base, which kept the old updatedAt. upsertJob wrote a fresh updatedAt only to the state index, so the in-memory record (and the per-job file) still carried the stale timestamp. Callers sort the reaped list with sortJobsNewestFirst (keyed on updatedAt), so a ghost job with more than a page of newer completed jobs ahead of it could be paged out of the first /codex:status report — the user would see no failed job or resume hint until running status again. Set updatedAt: completedAt on the record so the first reader reflects the failure it just recorded. Adds a regression test. Addresses review feedback from @chatgpt-codex-connector and @rajpratham1. Co-Authored-By: Claude Opus 4.8 --- plugins/codex/scripts/lib/tracked-jobs.mjs | 6 +++++- tests/tracked-jobs.test.mjs | 12 ++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index dcbbc5d09..6fd6b61b2 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -231,7 +231,11 @@ function markJobDead(workspaceRoot, jobSummary, errorMessage) { phase: "failed", errorMessage, pid: null, - completedAt + completedAt, + // Keep updatedAt current so the reaped job sorts newest-first in the same + // read that recorded it — otherwise a stale updatedAt can page it out of + // the first /codex:status report. + updatedAt: completedAt }; writeJobFile(workspaceRoot, jobSummary.id, record); upsertJob(workspaceRoot, { diff --git a/tests/tracked-jobs.test.mjs b/tests/tracked-jobs.test.mjs index ad7219125..334f15994 100644 --- a/tests/tracked-jobs.test.mjs +++ b/tests/tracked-jobs.test.mjs @@ -39,6 +39,18 @@ test("reapDeadJobs marks a running job with a dead pid as failed", () => { assert.equal(listJobs(workspace).find((job) => job.id === "job-dead").status, "failed"); }); +test("reapDeadJobs refreshes updatedAt so the reaped job sorts newest-first", () => { + const workspace = makeTempDir(); + const stale = "2000-01-01T00:00:00.000Z"; + seedJob(workspace, { id: "job-stale", status: "running", phase: "delegating", pid: spawnDeadPid(), updatedAt: stale, logFile: null }); + + const reaped = reapDeadJobs(workspace, listJobs(workspace)); + + assert.notEqual(reaped[0].updatedAt, stale); + assert.equal(reaped[0].updatedAt, reaped[0].completedAt); + assert.equal(readJobFile(resolveJobFile(workspace, "job-stale")).updatedAt, reaped[0].updatedAt); +}); + test("reapDeadJobs leaves a running job with a live pid untouched", () => { const workspace = makeTempDir(); seedJob(workspace, { id: "job-live", status: "running", phase: "delegating", pid: process.pid, logFile: null }); From 0cfbff848b7a828c13d51db67e8ca98b66515f04 Mon Sep 17 00:00:00 2001 From: greenbauer Date: Thu, 9 Jul 2026 20:02:40 -0500 Subject: [PATCH 04/51] fix: do not catch teardown signals in the worker crash guard /codex:cancel delivers SIGTERM (via terminateProcessTree) and then writes the job "cancelled". With the crash guard catching SIGTERM, the worker could process that same signal after the cancel command wrote its update and rewrite the job back to "failed", so a normal cancel raced into a failed status/result instead of cancelled. Drop SIGTERM/SIGINT/SIGHUP from registerWorkerCrashGuard. The guard now only handles in-process crashes (uncaughtException / unhandledRejection), where a precise error is available and no other command is writing the job. Every signal/kill death is left to the reader-side reapDeadJobs: SIGKILL is uncatchable so it must live there regardless, and reapDeadJobs never rewrites a job that already reached a terminal status, so an intentional cancel is preserved. Adds a regression test that a SIGTERMed guarded worker leaves a cancelled job untouched. Addresses @chatgpt-codex-connector review on the reaped-job head. Co-Authored-By: Claude Opus 4.8 --- plugins/codex/scripts/lib/tracked-jobs.mjs | 11 ++++-- tests/tracked-jobs.test.mjs | 40 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 6fd6b61b2..41cdaf788 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -270,6 +270,14 @@ export function reapDeadJobs(workspaceRoot, jobs) { }); } +// Guards only against in-process crashes (uncaughtException / unhandledRejection) +// where a precise error is available and no other command is writing the job. +// Signal-based deaths (SIGTERM/SIGINT/SIGHUP/SIGKILL) are intentionally NOT +// caught here: SIGKILL is uncatchable so the reader-side reapDeadJobs must cover +// it regardless, and /codex:cancel delivers SIGTERM as its teardown signal after +// writing the job "cancelled" — catching it here would race that terminal state +// back to "failed". reapDeadJobs handles every signal death and never rewrites a +// job that already reached a terminal status. export function registerWorkerCrashGuard(workspaceRoot, jobId, logFile = null) { const mark = (label) => (reason) => { try { @@ -283,7 +291,4 @@ export function registerWorkerCrashGuard(workspaceRoot, jobId, logFile = null) { }; process.on("uncaughtException", mark("uncaughtException")); process.on("unhandledRejection", mark("unhandledRejection")); - for (const sig of ["SIGTERM", "SIGINT", "SIGHUP"]) { - process.on(sig, mark(`received ${sig}`)); - } } diff --git a/tests/tracked-jobs.test.mjs b/tests/tracked-jobs.test.mjs index 334f15994..2d8f0c761 100644 --- a/tests/tracked-jobs.test.mjs +++ b/tests/tracked-jobs.test.mjs @@ -2,6 +2,7 @@ import fs from "node:fs"; import path from "node:path"; import test from "node:test"; import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; import { fileURLToPath, pathToFileURL } from "node:url"; import { makeTempDir, run } from "./helpers.mjs"; @@ -108,3 +109,42 @@ test("registerWorkerCrashGuard marks the job failed when the worker dies on an u assert.match(stored.errorMessage, /unhandledRejection/); assert.match(stored.errorMessage, /boom/); }); + +test("registerWorkerCrashGuard does not rewrite a cancelled job when the worker is SIGTERMed", async () => { + const workspace = makeTempDir(); + // Simulate handleCancel having already written the terminal state before the + // worker processes the teardown SIGTERM it delivered. + seedJob(workspace, { id: "job-cancelled", status: "cancelled", phase: "cancelled", pid: null, errorMessage: "Cancelled by user." }); + + const workerFile = path.join(makeTempDir(), "long-worker.mjs"); + fs.writeFileSync( + workerFile, + [ + `import { registerWorkerCrashGuard } from ${JSON.stringify(TRACKED_JOBS_URL)};`, + "registerWorkerCrashGuard(process.argv[2], process.argv[3], null);", + 'process.stdout.write("ready\\n");', + "setInterval(() => {}, 1000);", + "" + ].join("\n"), + "utf8" + ); + + const child = spawn(process.execPath, [workerFile, workspace, "job-cancelled"], { stdio: ["ignore", "pipe", "ignore"] }); + await new Promise((resolve, reject) => { + child.stdout.on("data", (chunk) => { + if (chunk.toString().includes("ready")) { + resolve(); + } + }); + child.on("error", reject); + }); + + const exited = new Promise((resolve) => child.on("exit", (code, signal) => resolve({ code, signal }))); + child.kill("SIGTERM"); + const { signal } = await exited; + + assert.equal(signal, "SIGTERM"); + const stored = readJobFile(resolveJobFile(workspace, "job-cancelled")); + assert.equal(stored.status, "cancelled"); + assert.equal(stored.errorMessage, "Cancelled by user."); +}); From f9dff6aff6bd245cab8a7c54f377626f3fe1faa7 Mon Sep 17 00:00:00 2001 From: greenbauer Date: Fri, 10 Jul 2026 09:32:03 -0500 Subject: [PATCH 05/51] fix: reap dead jobs before selecting task resume candidates reapDeadJobs was only applied at the job-control readers (status/result/ cancel). The task-resume paths read listJobs directly, so a crashed background task with a dead pid stayed "running" for them: resolveLatestTrackedTaskThread tripped its active-task guard (task --resume-last failed with "still running") and handleTaskResumeCandidate found no candidate, because findLatestResumableTaskJob skips running jobs. So the resume hint the reaper itself prints ("resume with task --resume-last") did not work until a separate status/result/cancel command reaped the job first. Wrap the two resume readers, plus the stop-review-gate hooks running-task note (same class: it read job status without reaping and would nag that a crashed ghost was still running at session stop). All listJobs readers now go through reapDeadJobs. Adds an end-to-end regression test that task-resume-candidate reaps a dead-pid running task into a resumable failed candidate. Addresses @chatgpt-codex-connector review on the rebased head. Co-Authored-By: Claude Opus 4.8 --- plugins/codex/scripts/codex-companion.mjs | 5 ++- .../codex/scripts/stop-review-gate-hook.mjs | 4 +- tests/runtime.test.mjs | 42 +++++++++++++++++++ 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 70f45ea2a..f8fd19491 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -49,6 +49,7 @@ import { createJobRecord, createProgressReporter, nowIso, + reapDeadJobs, registerWorkerCrashGuard, runTrackedJob, SESSION_ID_ENV @@ -337,7 +338,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(reapDeadJobs(workspaceRoot, listJobs(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) { @@ -936,7 +937,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(reapDeadJobs(workspaceRoot, listJobs(workspaceRoot)))); const candidate = findLatestResumableTaskJob(jobs); const payload = { diff --git a/plugins/codex/scripts/stop-review-gate-hook.mjs b/plugins/codex/scripts/stop-review-gate-hook.mjs index 2346bdcf4..55756b0f7 100644 --- a/plugins/codex/scripts/stop-review-gate-hook.mjs +++ b/plugins/codex/scripts/stop-review-gate-hook.mjs @@ -10,7 +10,7 @@ import { getCodexAvailability } from "./lib/codex.mjs"; import { loadPromptTemplate, interpolateTemplate } from "./lib/prompts.mjs"; import { getConfig, listJobs } from "./lib/state.mjs"; import { sortJobsNewestFirst } from "./lib/job-control.mjs"; -import { SESSION_ID_ENV } from "./lib/tracked-jobs.mjs"; +import { reapDeadJobs, 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(reapDeadJobs(workspaceRoot, listJobs(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 8f276835b..6a03725ff 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -570,6 +570,48 @@ test("task-resume-candidate returns the latest rescue thread from the current se assert.equal(payload.candidate.threadId, "thr_current"); }); +test("task-resume-candidate reaps a crashed running task so it becomes resumable", () => { + const workspace = makeTempDir(); + const stateDir = resolveStateDir(workspace); + const jobsDir = path.join(stateDir, "jobs"); + fs.mkdirSync(jobsDir, { recursive: true }); + + // A pid that has already exited: process.kill(pid, 0) will throw ESRCH. + const deadPid = run(process.execPath, ["-e", ""]).pid; + const crashedJob = { + id: "task-crashed", + status: "running", + phase: "delegating", + title: "Codex Task", + jobClass: "task", + sessionId: "sess-current", + threadId: "thr_crashed", + summary: "Investigate the crash", + pid: deadPid, + updatedAt: "2026-03-24T20:00:00.000Z" + }; + fs.writeFileSync(path.join(jobsDir, "task-crashed.json"), `${JSON.stringify(crashedJob, null, 2)}\n`, "utf8"); + fs.writeFileSync( + path.join(stateDir, "state.json"), + `${JSON.stringify({ version: 1, config: { stopReviewGate: false }, jobs: [crashedJob] }, null, 2)}\n`, + "utf8" + ); + + const result = run("node", [SCRIPT, "task-resume-candidate", "--json"], { + cwd: workspace, + env: { ...process.env, CODEX_COMPANION_SESSION_ID: "sess-current" } + }); + + assert.equal(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + // Without the reaper the job would still read as "running" and be skipped, + // leaving the resume probe with no candidate. + assert.equal(payload.available, true); + assert.equal(payload.candidate.id, "task-crashed"); + assert.equal(payload.candidate.status, "failed"); + assert.equal(payload.candidate.threadId, "thr_crashed"); +}); + test("task --resume-last does not resume a task from another Claude session", () => { const repo = makeTempDir(); const binDir = makeTempDir(); From d7aec137b7d63e900e41f84def2e4019e70e8e2c Mon Sep 17 00:00:00 2001 From: CBEPX <458940+CBEPX@users.noreply.github.com> Date: Fri, 28 Aug 2026 01:43:00 +0300 Subject: [PATCH 06/51] docs: v1.2.0 plan Co-Authored-By: Claude Fable 5 --- .../2026-08-28-codex-plugin-cc-v1.2.0.md | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-28-codex-plugin-cc-v1.2.0.md diff --git a/docs/superpowers/plans/2026-08-28-codex-plugin-cc-v1.2.0.md b/docs/superpowers/plans/2026-08-28-codex-plugin-cc-v1.2.0.md new file mode 100644 index 000000000..803cf9bac --- /dev/null +++ b/docs/superpowers/plans/2026-08-28-codex-plugin-cc-v1.2.0.md @@ -0,0 +1,131 @@ +# codex-plugin-cc v1.2.0 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 the rescue flow a single `node` call (companion-side await), restore the narrow `Bash(node:*)` grant, close the remaining lifecycle gaps (own-jobs-only SessionEnd, PID liveness reaping, bounded turns), and add the missing live-broker SessionEnd test. + +**Architecture:** New companion subcommand behaviour `task --await` (launch as a tracked background job, poll until terminal or `--await-timeout-ms`, print the result; on timeout print a resumable hint and exit 3) plus `--prompt-stdin` (raw, untokenized stdin = prompt) so a slash-command body is exactly one `node …` invocation with one quoted heredoc. Rescue command/agent bodies collapse to that call; `allowed-tools` goes back to `Bash(node:*)`. Lifecycle: cherry-pick upstream #355 (SessionEnd terminates only jobs it owns), #425 (PID liveness → reap zombie `running` jobs), #376 (bounded `captureTurn` with a configurable turn budget) — resolving against the fork's `disableBroker` cold-resume path and v1.1.1 shutdown changes. + +**Tech Stack:** Node ≥18.18, ESM `.mjs`, `node --test`, fake Codex fixture, `gh`. + +**Spec:** memory `codex-plugin-cc-fork-backlog` (v1.2 items); v1.1.0/v1.1.1 review residuals (random heredoc delimiters instruction-only; `allowed-tools: Bash` too broad; no test that SessionEnd kills a live broker). + +## Global Constraints + +- Repo `/Users/g.mehrenin/project/personal/codex-plugin-cc`, `origin`=CBEPX, `upstream`=openai. Branch `release/v1.2.0` from `main` (6672679 = v1.1.1). +- Gate (zsh reserves `status`, use `st`): `npm test > /tmp/npm-test.log 2>&1; st=$?; rg -e 'ℹ (tests|pass|fail)' -e '^not ok' /tmp/npm-test.log; test "$st" -eq 0` → `fail 0` (150 at base); `sleep 10; pgrep -f codex-plugin-test- | wc -l` → 0; `npm run build`; `npm run check-version`; `claude plugin validate . --strict` before the release commit. +- Tooling rule (user): never `grep` — ripgrep `rg`. No `git add -A`. Trailer `Co-Authored-By: Claude Fable 5 `. No push until the controller says so. +- Upstream PR merges: `git fetch upstream pull/N/head:pr/N && git merge --no-ff --no-edit pr/N`; keep-both on conflicts; the fork's `withAppServer(cwd, fn, clientOptions)`, `disableBroker` cold resume, `assertThreadIsFree`, `buildThreadConfig`, `--args-stdin`, v1.1.1 broker shutdown/ownership code must survive — verify by reading after each merge. +- Shell bodies of commands/agents: exactly one `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" …` invocation per Bash block, prose only via a quoted heredoc on stdin, flags on the command line; `allowed-tools: Bash(node:*)`. + +--- + +### Task 1: `task --await` + `--prompt-stdin` in the companion + +**Files:** +- Modify: `plugins/codex/scripts/codex-companion.mjs` — `handleTask` (new options `await`, `await-timeout-ms`, `prompt-stdin`), `printUsage`, reuse `enqueueBackgroundTask`, `waitForSingleJobSnapshot`/`handleStatus` internals, `handleResult` rendering; `readTaskPrompt` (prompt from raw stdin when `--prompt-stdin`). +- Test: `tests/runtime.test.mjs`, `tests/args.test.mjs` (booleanOptions/valueOptions additions), `tests/commands.test.mjs` untouched here. + +**Interfaces:** +- Consumes: `enqueueBackgroundTask(cwd, job, request)` (returns jobId; job record persisted before spawn since v1.1.0), `waitForSingleJobSnapshot(workspaceRoot, jobId, { timeoutMs, pollIntervalMs })`, `renderTaskResult(job)` / the `result` path, `isActiveJobStatus(status)`, `readStdinIfPiped()`. +- Produces: + - `task --await [--await-timeout-ms ] …` → enqueue a background job, then wait like `status --wait`; when the job reaches a terminal status print exactly what `result ` prints and exit 0 (failed job → its error text, exit 1); when `--await-timeout-ms` (default 540000) elapses first print `Still running: job . Re-run: node "