From c76e9ec46ebac6a30c8fb05d6b48043b020a8dc9 Mon Sep 17 00:00:00 2001
From: rajasekar-venkatesan
 <22398308+rajasekar-venkatesan@users.noreply.github.com>
Date: Wed, 26 Aug 2026 10:43:51 +0800
Subject: [PATCH 1/9] Fix job records lost on concurrent background task
 launches

updateState() did a lockless read-modify-write of state.json: concurrent
`codex-companion.mjs task --background` launches each read the same base
state, appended only their own job, and clobbered siblings on write.
saveState()'s prune then deleted the orphaned siblings' job files, so most
concurrently launched agents vanished from /codex:status and /codex:result.

Serialize the read-modify-write with a cross-process O_EXCL lockfile
(withStateLock, with jittered backoff and stale-lock stealing) and make the
state write atomic (temp file + rename) so lockless readers never observe a
torn file.

Stress test (12 concurrent upsertJob processes): 2/12 tracked before, 12/12
after.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 plugins/codex/scripts/lib/state.mjs | 52 ++++++++++++++++++++++++++---
 tests/state.test.mjs                | 38 +++++++++++++++++++++
 2 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs
index 2da23498f..04c80cc9f 100644
--- a/plugins/codex/scripts/lib/state.mjs
+++ b/plugins/codex/scripts/lib/state.mjs
@@ -111,14 +111,58 @@ export function saveState(cwd, state) {
     removeFileIfExists(job.logFile);
   }
 
-  fs.writeFileSync(resolveStateFile(cwd), `${JSON.stringify(nextState, null, 2)}\n`, "utf8");
+  const stateFile = resolveStateFile(cwd);
+  const tmpFile = `${stateFile}.${process.pid}.tmp`;
+  fs.writeFileSync(tmpFile, `${JSON.stringify(nextState, null, 2)}\n`, "utf8");
+  fs.renameSync(tmpFile, stateFile); // atomic replace; readers never see a partial file
   return nextState;
 }
 
+// Blocking sleep for a sync context (no busy-wait) via Atomics.wait.
+function sleepSync(ms) {
+  Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
+}
+
+// Cross-process lock around the state.json read-modify-write. Without it,
+// concurrent `task --background` launches each read the same base state, add
+// only their own job, and clobber siblings on write (and saveState's prune
+// then deletes the "orphan" job files). Serializing the RMW fixes both.
+function withStateLock(cwd, fn) {
+  ensureStateDir(cwd);
+  const lockFile = path.join(resolveStateDir(cwd), "state.lock");
+  const deadline = Date.now() + 15000;
+  let fd;
+  for (;;) {
+    try {
+      fd = fs.openSync(lockFile, "wx"); // O_CREAT | O_EXCL
+      break;
+    } catch (err) {
+      if (err.code !== "EEXIST") throw err;
+      // Steal a stale lock left by a crashed process.
+      try {
+        if (Date.now() - fs.statSync(lockFile).mtimeMs > 10000) {
+          fs.unlinkSync(lockFile);
+          continue;
+        }
+      } catch {}
+      if (Date.now() > deadline) throw new Error("Timed out acquiring Codex state lock");
+      sleepSync(20 + Math.floor(Math.random() * 30)); // jittered backoff
+    }
+  }
+  try {
+    return fn();
+  } finally {
+    try { fs.closeSync(fd); } catch {}
+    try { fs.unlinkSync(lockFile); } catch {}
+  }
+}
+
 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 saveState(cwd, state);
+  });
 }
 
 export function generateJobId(prefix = "job") {
diff --git a/tests/state.test.mjs b/tests/state.test.mjs
index 0f8f57cea..632acaf6d 100644
--- a/tests/state.test.mjs
+++ b/tests/state.test.mjs
@@ -4,9 +4,15 @@ 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 } from "./helpers.mjs";
 import { resolveJobFile, resolveJobLogFile, resolveStateDir, resolveStateFile, saveState } from "../plugins/codex/scripts/lib/state.mjs";
 
+const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..");
+const STATE_URL = pathToFileURL(path.join(ROOT, "plugins", "codex", "scripts", "lib", "state.mjs")).href;
+
 test("resolveStateDir uses a temp-backed per-workspace directory", () => {
   const workspace = makeTempDir();
   const stateDir = resolveStateDir(workspace);
@@ -103,3 +109,35 @@ test("saveState prunes dropped job artifacts when indexed jobs exceed the cap",
       .sort()
   );
 });
+
+test("updateState serializes concurrent job writes without losing records", async () => {
+  const workspace = makeTempDir();
+  const jobCount = 12;
+  const worker =
+    `import { upsertJob } from ${JSON.stringify(STATE_URL)};\n` +
+    "const [cwd, id] = process.argv.slice(1);\n" +
+    "upsertJob(cwd, { id, status: \"queued\", jobClass: \"task\", summary: id });\n";
+
+  await Promise.all(
+    Array.from({ length: jobCount }, (_, index) =>
+      new Promise((resolve, reject) => {
+        const child = spawn(
+          process.execPath,
+          ["--input-type=module", "-e", worker, workspace, `job-${index}`],
+          { stdio: "ignore" }
+        );
+        child.on("exit", (code) =>
+          code === 0 ? resolve() : reject(new Error(`worker ${index} exited ${code}`))
+        );
+      })
+    )
+  );
+
+  const saved = JSON.parse(fs.readFileSync(resolveStateFile(workspace), "utf8"));
+  const trackedIds = new Set(saved.jobs.map((job) => job.id));
+  const missing = Array.from({ length: jobCount }, (_, index) => `job-${index}`).filter(
+    (id) => !trackedIds.has(id)
+  );
+
+  assert.deepEqual(missing, [], "every concurrently launched job must be tracked");
+});

From af579cf6db029f463dddd966904280da7c6703a4 Mon Sep 17 00:00:00 2001
From: rajasekar-venkatesan
 <22398308+rajasekar-venkatesan@users.noreply.github.com>
Date: Wed, 26 Aug 2026 12:31:15 +0800
Subject: [PATCH 2/9] Make stale-lock reclaim atomic and ownership-safe

Addresses review feedback: the previous stale-lock cleanup did a
stat-then-unlink on the lock pathname, which is not atomic. Two racing
launches could both observe the same stale lock; after one unlinked it and
acquired a fresh lock, the other's unconditional unlink could remove that
fresh lock and let a second writer into the read-modify-write concurrently,
recreating the lost-job corruption this change prevents.

Reclaim the stale lock with rename() instead, which has exactly-one-winner
semantics: concurrent reclaimers target the same inode and only one succeeds;
losers get ENOENT and fall back to re-contending on open(O_EXCL). Stamp a
unique owner id into the lock and only unlink it on release if the contents
still match, so a process can never delete a lock another process now holds.

Add a regression test that seeds a stale lock and fires concurrent writers,
asserting no job records are dropped and the lock is released.
---
 plugins/codex/scripts/lib/state.mjs | 27 ++++++++++++++-----
 tests/state.test.mjs                | 42 +++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs
index 04c80cc9f..54111288a 100644
--- a/plugins/codex/scripts/lib/state.mjs
+++ b/plugins/codex/scripts/lib/state.mjs
@@ -130,18 +130,29 @@ function sleepSync(ms) {
 function withStateLock(cwd, fn) {
   ensureStateDir(cwd);
   const lockFile = path.join(resolveStateDir(cwd), "state.lock");
+  // Unique per acquisition: stamped into the lock so only the true owner ever
+  // removes it, and so a reclaimer's scratch path can't collide.
+  const ownerId = `${process.pid}-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}`;
   const deadline = Date.now() + 15000;
-  let fd;
   for (;;) {
     try {
-      fd = fs.openSync(lockFile, "wx"); // O_CREAT | O_EXCL
+      const fd = fs.openSync(lockFile, "wx"); // O_CREAT | O_EXCL
+      fs.writeSync(fd, ownerId);
+      fs.closeSync(fd);
       break;
     } catch (err) {
       if (err.code !== "EEXIST") throw err;
-      // Steal a stale lock left by a crashed process.
+      // Reclaim a stale lock left by a crashed process. Do it atomically:
+      // rename() has exactly-one-winner semantics, so if several launches race
+      // to reclaim the same stale lock, only one succeeds and the losers get
+      // ENOENT and fall back to re-contending on open(O_EXCL). Never unlink the
+      // lock pathname directly here -- by the time we did, it could already be a
+      // fresh lock held by another process (the P2 the reviewer flagged).
       try {
         if (Date.now() - fs.statSync(lockFile).mtimeMs > 10000) {
-          fs.unlinkSync(lockFile);
+          const scratch = `${lockFile}.stale.${ownerId}`;
+          fs.renameSync(lockFile, scratch); // only one racer wins this
+          fs.unlinkSync(scratch);
           continue;
         }
       } catch {}
@@ -152,8 +163,12 @@ function withStateLock(cwd, fn) {
   try {
     return fn();
   } finally {
-    try { fs.closeSync(fd); } catch {}
-    try { fs.unlinkSync(lockFile); } catch {}
+    // Only remove the lock if we still own it: if fn() ever outran the stale
+    // timeout and another process reclaimed the lock, its contents no longer
+    // match ownerId and we must not delete the lock it now holds.
+    try {
+      if (fs.readFileSync(lockFile, "utf8") === ownerId) fs.unlinkSync(lockFile);
+    } catch {}
   }
 }
 
diff --git a/tests/state.test.mjs b/tests/state.test.mjs
index 632acaf6d..005ad32db 100644
--- a/tests/state.test.mjs
+++ b/tests/state.test.mjs
@@ -141,3 +141,45 @@ test("updateState serializes concurrent job writes without losing records", asyn
 
   assert.deepEqual(missing, [], "every concurrently launched job must be tracked");
 });
+
+test("updateState reclaims a stale lock without losing records under concurrency", async () => {
+  const workspace = makeTempDir();
+
+  // Simulate a crashed holder: a lock file older than the 10s stale threshold.
+  const stateDir = resolveStateDir(workspace);
+  fs.mkdirSync(stateDir, { recursive: true });
+  const lockFile = path.join(stateDir, "state.lock");
+  fs.writeFileSync(lockFile, "dead-owner");
+  const stale = new Date(Date.now() - 60_000);
+  fs.utimesSync(lockFile, stale, stale);
+
+  const jobCount = 10;
+  const worker =
+    `import { upsertJob } from ${JSON.stringify(STATE_URL)};\n` +
+    "const [cwd, id] = process.argv.slice(1);\n" +
+    "upsertJob(cwd, { id, status: \"queued\", jobClass: \"task\", summary: id });\n";
+
+  await Promise.all(
+    Array.from({ length: jobCount }, (_, index) =>
+      new Promise((resolve, reject) => {
+        const child = spawn(
+          process.execPath,
+          ["--input-type=module", "-e", worker, workspace, `job-${index}`],
+          { stdio: "ignore" }
+        );
+        child.on("exit", (code) =>
+          code === 0 ? resolve() : reject(new Error(`worker ${index} exited ${code}`))
+        );
+      })
+    )
+  );
+
+  const saved = JSON.parse(fs.readFileSync(resolveStateFile(workspace), "utf8"));
+  const trackedIds = new Set(saved.jobs.map((job) => job.id));
+  const missing = Array.from({ length: jobCount }, (_, index) => `job-${index}`).filter(
+    (id) => !trackedIds.has(id)
+  );
+
+  assert.deepEqual(missing, [], "stale-lock reclaim must not drop concurrent job records");
+  assert.equal(fs.existsSync(lockFile), false, "lock file should be released after all writers finish");
+});

From 7d7326a5e9e9af7a3d6ef6f9ab45c6067b46bdbe Mon Sep 17 00:00:00 2001
From: rajasekar-venkatesan
 <22398308+rajasekar-venkatesan@users.noreply.github.com>
Date: Wed, 26 Aug 2026 13:23:10 +0800
Subject: [PATCH 3/9] Make lock reclaim safe by owner liveness and atomic
 publication

Follow-up review found the rename-based reclaim still had a TOCTOU: a delayed
reclaimer could act on the lock pathname after another process had recreated a
fresh lock there. Reworked the lock so no reclaim decision is ever made against
a live owner, and so a lock is never observed in a half-published state.

- Reclaim is now gated on owner liveness: the lock stamps its holder's PID and a
  waiter only reclaims when process.kill(pid, 0) shows the owner is dead. A live
  owner (even a slow or briefly-suspended one) is never reclaimed, so the removal
  can never race a lock another process still holds. mtime is demoted to a coarse
  10-minute backstop that only covers PID reuse of a dead owner (preserving
  liveness without letting a live holder be reclaimed in any realistic case).

- Locks are published atomically via write-temp + linkSync instead of
  openSync(O_EXCL) followed by a separate writeSync. linkSync is atomic and fails
  EEXIST like O_EXCL, but the file carries its full owner id the instant it
  appears, closing the window where a concurrent reader could see an empty lock
  and treat a just-created live lock as abandoned. Applied to both the state lock
  and the serialized reclaim lock.

- Reclaim is still serialized through a second lock; that lock is atomically
  published, PID-stamped, cleared only when its own holder is dead, and released
  only by its owner.

The residual (a holder suspended past the 10-minute backstop, or PID reuse of a
dead owner within it) is the timeout limit inherent to any pure-fs PID lock and
is documented in the code.
---
 plugins/codex/scripts/lib/state.mjs | 138 ++++++++++++++++++++++------
 tests/state.test.mjs                |   7 +-
 2 files changed, 113 insertions(+), 32 deletions(-)

diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs
index 54111288a..dbbf99581 100644
--- a/plugins/codex/scripts/lib/state.mjs
+++ b/plugins/codex/scripts/lib/state.mjs
@@ -123,6 +123,103 @@ function sleepSync(ms) {
   Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
 }
 
+// Absolute-backstop timeout. A lock is normally reclaimed the instant its owner's
+// PID is dead; this only forces reclaim of a lock whose PID still looks alive, to
+// preserve liveness in the rare case a dead owner's PID was recycled by an
+// unrelated live process. It is deliberately far longer than any real critical
+// section (a state.json read-modify-write is milliseconds), so a genuinely live
+// holder is never reclaimed by it -- only one wedged/suspended past 10 minutes,
+// which is indistinguishable from dead.
+const LOCK_BACKSTOP_MS = 600000;
+
+// Lock files live under a per-workspace OS temp dir (see resolveStateDir), i.e. a
+// single host, so a PID read from a lock refers to a process on this machine and
+// process.kill(pid, 0) is a valid liveness probe.
+function ownerAlive(id) {
+  const pid = Number.parseInt(String(id).split(".")[0], 10);
+  if (!Number.isInteger(pid) || pid <= 0) return false; // empty/garbled => not a live owner
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch (err) {
+    return err.code === "EPERM"; // exists but not ours (still alive); ESRCH => dead
+  }
+}
+
+function isAbandoned(id, mtimeMs) {
+  if (!ownerAlive(id)) return true;                    // dead owner -> reclaimable
+  return Date.now() - mtimeMs > LOCK_BACKSTOP_MS;      // else only the far backstop
+}
+
+// Publish a lock atomically: write the owner id into a unique temp file, then
+// hard-link it onto the fixed path. linkSync is atomic and fails EEXIST if the
+// path is already held, exactly like O_EXCL -- but unlike open()+write() the file
+// has its full content the instant it appears at the path, so a concurrent reader
+// can never observe an empty lock and mistake a just-created live lock for an
+// abandoned one. Returns true if claimed, false if already held.
+function claimLock(lockFile, ownerId) {
+  const tmp = `${lockFile}.tmp.${ownerId}`;
+  fs.writeFileSync(tmp, ownerId);
+  try {
+    fs.linkSync(tmp, lockFile);
+    return true;
+  } catch (err) {
+    if (err.code === "EEXIST") return false;
+    throw err;
+  } finally {
+    try { fs.unlinkSync(tmp); } catch {}
+  }
+}
+
+// Reclaim a lock only when its owner is gone, never a live one.
+//
+// Gated on PID liveness: a live owner -- even one working slowly or briefly
+// frozen -- keeps a live PID and is never reclaimed, so this can only ever remove
+// a lock whose owner is truly dead. A dead owner can neither release nor recreate
+// its lock, and no other process can claim the path while it still exists, so
+// removing it here cannot race a freshly acquired lock. (The earlier mtime-only
+// reclaim was unsafe precisely because it could fire against a live owner and
+// unlink a lock another process had recreated in the meantime.)
+//
+// Removal is serialized through a second reclaim lock so two reclaimers can't
+// both act; that lock is atomically published, PID-stamped, only cleared when its
+// holder is dead, and released only by its own owner.
+function reclaimIfAbandoned(lockFile, reclaimFile, selfId) {
+  let content, st;
+  try {
+    content = fs.readFileSync(lockFile, "utf8");
+    st = fs.statSync(lockFile);
+  } catch {
+    return; // already gone
+  }
+  if (!isAbandoned(content, st.mtimeMs)) return; // live owner -> wait, don't touch
+  if (!claimLock(reclaimFile, selfId)) {
+    try {
+      // A reclaim lock whose own holder died is safe to drop; a live one is left
+      // alone (its PID is alive, so this never removes a reclaim in progress).
+      const rc = fs.readFileSync(reclaimFile, "utf8");
+      const rst = fs.statSync(reclaimFile);
+      if (isAbandoned(rc, rst.mtimeMs)) fs.unlinkSync(reclaimFile);
+    } catch {}
+    return;
+  }
+  try {
+    // Re-verify under the reclaim lock. The owner is still gone (a dead PID cannot
+    // come back), and the path can't have been re-claimed while it exists, so
+    // unlinking here cannot delete a live lock.
+    const c2 = fs.readFileSync(lockFile, "utf8");
+    const s2 = fs.statSync(lockFile);
+    if (isAbandoned(c2, s2.mtimeMs)) fs.unlinkSync(lockFile);
+  } catch {
+    // lock vanished between checks -- fine, nothing to reclaim
+  } finally {
+    // Release the reclaim lock only if it is still ours.
+    try {
+      if (fs.readFileSync(reclaimFile, "utf8") === selfId) fs.unlinkSync(reclaimFile);
+    } catch {}
+  }
+}
+
 // Cross-process lock around the state.json read-modify-write. Without it,
 // concurrent `task --background` launches each read the same base state, add
 // only their own job, and clobber siblings on write (and saveState's prune
@@ -130,42 +227,23 @@ function sleepSync(ms) {
 function withStateLock(cwd, fn) {
   ensureStateDir(cwd);
   const lockFile = path.join(resolveStateDir(cwd), "state.lock");
-  // Unique per acquisition: stamped into the lock so only the true owner ever
-  // removes it, and so a reclaimer's scratch path can't collide.
-  const ownerId = `${process.pid}-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}`;
+  const reclaimFile = `${lockFile}.reclaim`;
+  // "<pid>.<time>.<rand>": the PID lets other processes probe our liveness; the
+  // full string is stamped into the lock so only the true owner removes it.
+  const ownerId = `${process.pid}.${Date.now().toString(36)}.${Math.random().toString(36).slice(2, 8)}`;
   const deadline = Date.now() + 15000;
   for (;;) {
-    try {
-      const fd = fs.openSync(lockFile, "wx"); // O_CREAT | O_EXCL
-      fs.writeSync(fd, ownerId);
-      fs.closeSync(fd);
-      break;
-    } catch (err) {
-      if (err.code !== "EEXIST") throw err;
-      // Reclaim a stale lock left by a crashed process. Do it atomically:
-      // rename() has exactly-one-winner semantics, so if several launches race
-      // to reclaim the same stale lock, only one succeeds and the losers get
-      // ENOENT and fall back to re-contending on open(O_EXCL). Never unlink the
-      // lock pathname directly here -- by the time we did, it could already be a
-      // fresh lock held by another process (the P2 the reviewer flagged).
-      try {
-        if (Date.now() - fs.statSync(lockFile).mtimeMs > 10000) {
-          const scratch = `${lockFile}.stale.${ownerId}`;
-          fs.renameSync(lockFile, scratch); // only one racer wins this
-          fs.unlinkSync(scratch);
-          continue;
-        }
-      } catch {}
-      if (Date.now() > deadline) throw new Error("Timed out acquiring Codex state lock");
-      sleepSync(20 + Math.floor(Math.random() * 30)); // jittered backoff
-    }
+    if (claimLock(lockFile, ownerId)) break;
+    reclaimIfAbandoned(lockFile, reclaimFile, ownerId);
+    if (Date.now() > deadline) throw new Error("Timed out acquiring Codex state lock");
+    sleepSync(20 + Math.floor(Math.random() * 30)); // jittered backoff
   }
   try {
     return fn();
   } finally {
-    // Only remove the lock if we still own it: if fn() ever outran the stale
-    // timeout and another process reclaimed the lock, its contents no longer
-    // match ownerId and we must not delete the lock it now holds.
+    // Only remove the lock if we still own it: if we were ever reclaimed (e.g.
+    // frozen past the fallback timeout), the contents no longer match ownerId and
+    // we must not delete the lock another process now holds.
     try {
       if (fs.readFileSync(lockFile, "utf8") === ownerId) fs.unlinkSync(lockFile);
     } catch {}
diff --git a/tests/state.test.mjs b/tests/state.test.mjs
index 005ad32db..80b36cfcc 100644
--- a/tests/state.test.mjs
+++ b/tests/state.test.mjs
@@ -149,8 +149,11 @@ test("updateState reclaims a stale lock without losing records under concurrency
   const stateDir = resolveStateDir(workspace);
   fs.mkdirSync(stateDir, { recursive: true });
   const lockFile = path.join(stateDir, "state.lock");
-  fs.writeFileSync(lockFile, "dead-owner");
-  const stale = new Date(Date.now() - 60_000);
+  // Stamp a dead PID as the owner so the liveness-based reclaim path fires. PID 1
+  // reused is possible but never dead; use a high, almost-certainly-dead PID and
+  // also backdate the mtime so the coarse fallback covers any PID-reuse fluke.
+  fs.writeFileSync(lockFile, "2147483646.dead.owner");
+  const stale = new Date(Date.now() - 120_000);
   fs.utimesSync(lockFile, stale, stale);
 
   const jobCount = 10;

From cb7a730719431c1ffd32ba1a97a4377cbf5b498e Mon Sep 17 00:00:00 2001
From: rajasekar-venkatesan
 <22398308+rajasekar-venkatesan@users.noreply.github.com>
Date: Wed, 26 Aug 2026 13:47:27 +0800
Subject: [PATCH 4/9] Reclaim by process-instance identity, not a time backstop

Review flagged that the 10-minute mtime backstop could still expire a lock
whose PID is alive (a holder suspended past the backstop), letting it resume
concurrently with a new owner. Removed all time-based expiry.

- A lock is reclaimed only when the exact process *instance* that wrote it is
  gone. The owner id now embeds a start-time token (Linux /proc/<pid>/stat
  field 22; otherwise `/bin/ps -o lstart` with TZ/locale pinned so a stamper and
  a checker render it identically, and never mixing the two sources). isAbandoned
  is true only for a dead PID, or a live PID whose current start token differs
  from the stamp (PID reuse). A live instance is never reclaimed, no matter how
  long it holds the lock. An unverifiable ("0") stamp is never token-reclaimed.

- Reclaim no longer uses a second "reclaim" lock (whose own stale cleanup had a
  TOCTOU). It captures the abandoned lock with an atomic rename (exactly one
  winner; losers get ENOENT and re-contend), confirms byte-for-byte that it
  captured the instance it judged, and restores rather than deletes anything it
  didn't. A pre-rename re-read keeps the subprocess-backed liveness probe out of
  the capture window.

Residual, documented in code: fully atomic conditional-delete needs an OS
advisory lock (flock), which Node's fs builtins don't expose; the remaining
window is ~2 adjacent syscalls and fails as a bounded acquire-timeout error, not
silent corruption or permanent deadlock.
---
 plugins/codex/scripts/lib/state.mjs | 193 ++++++++++++++++++----------
 tests/state.test.mjs                |  10 +-
 2 files changed, 131 insertions(+), 72 deletions(-)

diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs
index dbbf99581..fa478fc70 100644
--- a/plugins/codex/scripts/lib/state.mjs
+++ b/plugins/codex/scripts/lib/state.mjs
@@ -1,3 +1,4 @@
+import { spawnSync } from "node:child_process";
 import { createHash } from "node:crypto";
 import fs from "node:fs";
 import os from "node:os";
@@ -123,32 +124,82 @@ function sleepSync(ms) {
   Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
 }
 
-// Absolute-backstop timeout. A lock is normally reclaimed the instant its owner's
-// PID is dead; this only forces reclaim of a lock whose PID still looks alive, to
-// preserve liveness in the rare case a dead owner's PID was recycled by an
-// unrelated live process. It is deliberately far longer than any real critical
-// section (a state.json read-modify-write is milliseconds), so a genuinely live
-// holder is never reclaimed by it -- only one wedged/suspended past 10 minutes,
-// which is indistinguishable from dead.
-const LOCK_BACKSTOP_MS = 600000;
-
+// A per-process, restart-stable token identifying a specific process *instance*
+// (not just its PID), so PID reuse can be told apart from the original owner.
 // Lock files live under a per-workspace OS temp dir (see resolveStateDir), i.e. a
-// single host, so a PID read from a lock refers to a process on this machine and
-// process.kill(pid, 0) is a valid liveness probe.
-function ownerAlive(id) {
-  const pid = Number.parseInt(String(id).split(".")[0], 10);
-  if (!Number.isInteger(pid) || pid <= 0) return false; // empty/garbled => not a live owner
+// single host, so the PID refers to a process on this machine.
+//
+// The source is chosen by platform and never mixed: two renderings of the same
+// live process must compare equal, so we must not stamp with one source and check
+// with another. Returns null when the process is gone or its start time can't be
+// read (callers treat null conservatively -- never as "different instance").
+//   - Linux: /proc/<pid>/stat field 22 is the process start time (clock ticks
+//     since boot); read directly, no subprocess.
+//   - else (macOS/BSD): `ps -o lstart` is the start timestamp, stable for the
+//     process lifetime. The env is pinned (TZ/locale) because lstart is rendered
+//     with strftime and would otherwise differ between a stamper and a checker
+//     running under different TZ/LC settings. spawnSync is only reached on the
+//     (rare) reclaim path, never on the uncontended fast path.
+function processStartToken(pid) {
+  if (!Number.isInteger(pid) || pid <= 0) return null;
+  if (process.platform === "linux") {
+    try {
+      const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8");
+      // comm (field 2) may contain spaces/parens; the numeric fields start after
+      // the last ')'. starttime is field 22 => index 19 of that remainder.
+      const rest = stat.slice(stat.lastIndexOf(")") + 2).split(/\s+/);
+      if (rest[19]) return `L:${rest[19]}`;
+    } catch {}
+    return null; // no cross-source fallback -- see note above
+  }
+  try {
+    const r = spawnSync("/bin/ps", ["-o", "lstart=", "-p", String(pid)], {
+      encoding: "utf8",
+      env: { ...process.env, TZ: "UTC0", LC_ALL: "C", LANG: "C" },
+    });
+    if (r.status === 0) {
+      const s = (r.stdout || "").trim();
+      if (s) return `P:${s}`;
+    }
+  } catch {}
+  return null;
+}
+
+// The lock owner is identified as "<pid>.<startTokenHex>.<time>.<rand>". Compute
+// the current process's identity once per acquisition.
+function selfOwnerId() {
+  const tok = processStartToken(process.pid);
+  const tag = tok ? Buffer.from(tok).toString("hex") : "0";
+  return `${process.pid}.${tag}.${Date.now().toString(36)}.${Math.random().toString(36).slice(2, 8)}`;
+}
+
+// True only when the exact process instance that wrote `id` is gone -- never for
+// a live owner, no matter how long it has been holding the lock. This is what
+// lets reclaim run without any time-based expiry: a suspended-but-alive holder is
+// never reclaimed, and a dead owner whose PID was recycled is detected because
+// the recycled process reports a different start token.
+function isAbandoned(id) {
+  const parts = String(id).split(".");
+  const pid = Number.parseInt(parts[0], 10);
+  if (!Number.isInteger(pid) || pid <= 0) return true; // empty/garbled -> not a live owner
+  let alive;
   try {
     process.kill(pid, 0);
-    return true;
+    alive = true;
   } catch (err) {
-    return err.code === "EPERM"; // exists but not ours (still alive); ESRCH => dead
+    alive = err.code === "EPERM"; // exists but not ours (still alive); ESRCH => dead
   }
-}
-
-function isAbandoned(id, mtimeMs) {
-  if (!ownerAlive(id)) return true;                    // dead owner -> reclaimable
-  return Date.now() - mtimeMs > LOCK_BACKSTOP_MS;      // else only the far backstop
+  if (!alive) return true; // owner process is gone
+  // PID is alive. Only declare it abandoned if we can PROVE it is a different
+  // instance. If the owner's stamp is unverifiable ("0"), or we can't read the
+  // current start token, treat the live PID as the same instance and do NOT
+  // reclaim -- otherwise a checker that *can* probe would delete a live owner's
+  // lock whose owner merely failed to self-probe at stamp time. (pid-death
+  // reclaim above still applies, so this never causes a permanent deadlock.)
+  if (!parts[1] || parts[1] === "0") return false;
+  const cur = processStartToken(pid);
+  if (cur === null) return false;
+  return Buffer.from(cur).toString("hex") !== parts[1]; // different instance => PID reuse => owner gone
 }
 
 // Publish a lock atomically: write the owner id into a unique temp file, then
@@ -171,52 +222,62 @@ function claimLock(lockFile, ownerId) {
   }
 }
 
-// Reclaim a lock only when its owner is gone, never a live one.
+// Reclaim a lock only when its owner instance is gone, never a live one.
 //
-// Gated on PID liveness: a live owner -- even one working slowly or briefly
-// frozen -- keeps a live PID and is never reclaimed, so this can only ever remove
-// a lock whose owner is truly dead. A dead owner can neither release nor recreate
-// its lock, and no other process can claim the path while it still exists, so
-// removing it here cannot race a freshly acquired lock. (The earlier mtime-only
-// reclaim was unsafe precisely because it could fire against a live owner and
-// unlink a lock another process had recreated in the meantime.)
+// isAbandoned is true only for a dead owner (or a PID recycled by a different
+// instance), so this never fires against the live owner. Removal is atomic via
+// capture-by-rename: renameSync of the fixed path has exactly-one-winner
+// semantics, so when several launches race to reclaim the same lock only one
+// captures it and the losers get ENOENT and fall back to re-contending. The
+// captor then confirms the file it captured is byte-for-byte the id it judged
+// abandoned (the id embeds time+rand, so this is unique); if instead it captured
+// a lock that had been recreated in the meantime -- i.e. possibly a live one --
+// it restores it rather than deleting it. This needs no second lock, so there is
+// no reclaim-lock to itself go stale and be cleaned up unsafely.
 //
-// Removal is serialized through a second reclaim lock so two reclaimers can't
-// both act; that lock is atomically published, PID-stamped, only cleared when its
-// holder is dead, and released only by its own owner.
-function reclaimIfAbandoned(lockFile, reclaimFile, selfId) {
-  let content, st;
+// Residual (fundamental to pure-fs locking): between the re-read and the rename
+// below the lock could be reclaimed by someone else and freshly re-claimed by a
+// live owner; renaming then captures that live lock, and if a third process
+// claims the momentarily-absent path the restore relink loses (EEXIST) and the
+// captured lock is dropped. The pre-rename re-read shrinks that window to ~2
+// adjacent syscalls with no subprocess in it; isAbandoned (which may spawn `ps`)
+// runs only *before* the re-read. Closing it completely needs an atomic
+// conditional-delete / OS advisory lock (flock), which Node's fs builtins do not
+// expose. The failure mode is bounded -- a losing contender times out within the
+// 15s acquire deadline (an error, not silent corruption) -- never a permanent
+// deadlock, since a dead owner is always reclaimable on the next pass.
+function reclaimIfAbandoned(lockFile, selfId) {
+  let seen;
   try {
-    content = fs.readFileSync(lockFile, "utf8");
-    st = fs.statSync(lockFile);
+    seen = fs.readFileSync(lockFile, "utf8");
   } catch {
     return; // already gone
   }
-  if (!isAbandoned(content, st.mtimeMs)) return; // live owner -> wait, don't touch
-  if (!claimLock(reclaimFile, selfId)) {
-    try {
-      // A reclaim lock whose own holder died is safe to drop; a live one is left
-      // alone (its PID is alive, so this never removes a reclaim in progress).
-      const rc = fs.readFileSync(reclaimFile, "utf8");
-      const rst = fs.statSync(reclaimFile);
-      if (isAbandoned(rc, rst.mtimeMs)) fs.unlinkSync(reclaimFile);
-    } catch {}
-    return;
+  if (!isAbandoned(seen)) return; // live owner -> wait, don't touch
+  const tomb = `${lockFile}.rip.${selfId}`;
+  // Re-read immediately before capturing so isAbandoned's (possibly subprocess-
+  // backed) probe is not inside the capture window: only proceed if the lock is
+  // still the exact abandoned instance we judged.
+  try {
+    if (fs.readFileSync(lockFile, "utf8") !== seen) return;
+  } catch {
+    return; // vanished -- re-contend
   }
   try {
-    // Re-verify under the reclaim lock. The owner is still gone (a dead PID cannot
-    // come back), and the path can't have been re-claimed while it exists, so
-    // unlinking here cannot delete a live lock.
-    const c2 = fs.readFileSync(lockFile, "utf8");
-    const s2 = fs.statSync(lockFile);
-    if (isAbandoned(c2, s2.mtimeMs)) fs.unlinkSync(lockFile);
+    fs.renameSync(lockFile, tomb); // atomic: exactly one reclaimer captures the path
   } catch {
-    // lock vanished between checks -- fine, nothing to reclaim
-  } finally {
-    // Release the reclaim lock only if it is still ours.
-    try {
-      if (fs.readFileSync(reclaimFile, "utf8") === selfId) fs.unlinkSync(reclaimFile);
-    } catch {}
+    return; // lost the race (ENOENT) -- another reclaimer took it; re-contend
+  }
+  try {
+    if (fs.readFileSync(tomb, "utf8") === seen) {
+      fs.unlinkSync(tomb); // captured the exact abandoned instance we judged -> drop it
+    } else {
+      // Captured a lock recreated after our read -> may be live; put it back.
+      try { fs.linkSync(tomb, lockFile); } catch {}
+      fs.unlinkSync(tomb);
+    }
+  } catch {
+    try { fs.unlinkSync(tomb); } catch {}
   }
 }
 
@@ -227,23 +288,23 @@ function reclaimIfAbandoned(lockFile, reclaimFile, selfId) {
 function withStateLock(cwd, fn) {
   ensureStateDir(cwd);
   const lockFile = path.join(resolveStateDir(cwd), "state.lock");
-  const reclaimFile = `${lockFile}.reclaim`;
-  // "<pid>.<time>.<rand>": the PID lets other processes probe our liveness; the
-  // full string is stamped into the lock so only the true owner removes it.
-  const ownerId = `${process.pid}.${Date.now().toString(36)}.${Math.random().toString(36).slice(2, 8)}`;
+  // Identifies this exact process instance (pid + start-time), so others can tell
+  // a live owner from a recycled PID; the full string is stamped into the lock so
+  // only the true owner removes it on release.
+  const ownerId = selfOwnerId();
   const deadline = Date.now() + 15000;
   for (;;) {
     if (claimLock(lockFile, ownerId)) break;
-    reclaimIfAbandoned(lockFile, reclaimFile, ownerId);
+    reclaimIfAbandoned(lockFile, ownerId);
     if (Date.now() > deadline) throw new Error("Timed out acquiring Codex state lock");
     sleepSync(20 + Math.floor(Math.random() * 30)); // jittered backoff
   }
   try {
     return fn();
   } finally {
-    // Only remove the lock if we still own it: if we were ever reclaimed (e.g.
-    // frozen past the fallback timeout), the contents no longer match ownerId and
-    // we must not delete the lock another process now holds.
+    // Only remove the lock if we still own it: if we were ever reclaimed, the
+    // contents no longer match ownerId and we must not delete the lock another
+    // process now holds.
     try {
       if (fs.readFileSync(lockFile, "utf8") === ownerId) fs.unlinkSync(lockFile);
     } catch {}
diff --git a/tests/state.test.mjs b/tests/state.test.mjs
index 80b36cfcc..01cf0bdf1 100644
--- a/tests/state.test.mjs
+++ b/tests/state.test.mjs
@@ -149,12 +149,10 @@ test("updateState reclaims a stale lock without losing records under concurrency
   const stateDir = resolveStateDir(workspace);
   fs.mkdirSync(stateDir, { recursive: true });
   const lockFile = path.join(stateDir, "state.lock");
-  // Stamp a dead PID as the owner so the liveness-based reclaim path fires. PID 1
-  // reused is possible but never dead; use a high, almost-certainly-dead PID and
-  // also backdate the mtime so the coarse fallback covers any PID-reuse fluke.
-  fs.writeFileSync(lockFile, "2147483646.dead.owner");
-  const stale = new Date(Date.now() - 120_000);
-  fs.utimesSync(lockFile, stale, stale);
+  // Stamp a dead PID as the owner ("<pid>.<startTag>.<time>.<rand>") so the
+  // instance-liveness reclaim path fires: process.kill on this high, almost-
+  // certainly-dead PID throws ESRCH, so the lock is treated as abandoned.
+  fs.writeFileSync(lockFile, "2147483646.0.dead.owner");
 
   const jobCount = 10;
   const worker =

From 11ce4cc58e537e17e9ecbeeff3a9f57e91b6a1af Mon Sep 17 00:00:00 2001
From: rajasekar-venkatesan
 <22398308+rajasekar-venkatesan@users.noreply.github.com>
Date: Wed, 26 Aug 2026 14:04:24 +0800
Subject: [PATCH 5/9] Lock session cleanup and treat zombie owners as abandoned

Two review follow-ups:

- Session-end cleanup bypassed the lock. cleanupSessionJobs did loadState() then
  the exported saveState() directly, so a concurrent upsertJob (task launch)
  could be clobbered and its just-added job/log files pruned. It now runs through
  the locked updateState() read-modify-write, collecting the running jobs to kill
  inside the locked mutation. Process teardown runs in a finally after the lock is
  released, so a failed state write (including the 15s lock-acquire timeout) can
  neither leak this session's processes nor abort the rest of session shutdown
  (broker teardown); if the locked update fails, the pids are identified via a
  best-effort unlocked *read* (never an unlocked write).

- Zombie lock owners were treated as live. A process that has exited but not been
  reaped still answers process.kill(pid, 0) and keeps its start token, so it
  looked like a live owner forever and blocked every writer until the parent
  reaped it. isAbandoned now treats a zombie as gone: Linux reads the state field
  of /proc/<pid>/stat; elsewhere it checks `ps -o state` (matches "Z"/"Z+"/"ZN").

Also corrected the reclaim residual comment: the sub-syscall capture window can,
worst case, permit a brief two-writer overlap, not only an acquire timeout.
---
 plugins/codex/scripts/lib/state.mjs           | 32 +++++++++-
 .../codex/scripts/session-lifecycle-hook.mjs  | 61 +++++++++++++------
 2 files changed, 70 insertions(+), 23 deletions(-)

diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs
index fa478fc70..ba355cf02 100644
--- a/plugins/codex/scripts/lib/state.mjs
+++ b/plugins/codex/scripts/lib/state.mjs
@@ -165,6 +165,31 @@ function processStartToken(pid) {
   return null;
 }
 
+// A process that has exited but not yet been reaped by its parent is a zombie:
+// process.kill(pid, 0) still succeeds and its start token is unchanged, so it
+// would otherwise look like a live owner forever. Detect it so its lock is
+// reclaimed instead of blocking every writer until the parent reaps it.
+function isZombie(pid) {
+  if (process.platform === "linux") {
+    try {
+      const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8");
+      // state (field 3) is the first token after the last ')'.
+      return stat.slice(stat.lastIndexOf(")") + 2).split(/\s+/)[0] === "Z";
+    } catch {
+      return false;
+    }
+  }
+  try {
+    const r = spawnSync("/bin/ps", ["-o", "state=", "-p", String(pid)], {
+      encoding: "utf8",
+      env: { ...process.env, LC_ALL: "C" },
+    });
+    return r.status === 0 && (r.stdout || "").trim().startsWith("Z");
+  } catch {
+    return false;
+  }
+}
+
 // The lock owner is identified as "<pid>.<startTokenHex>.<time>.<rand>". Compute
 // the current process's identity once per acquisition.
 function selfOwnerId() {
@@ -190,6 +215,7 @@ function isAbandoned(id) {
     alive = err.code === "EPERM"; // exists but not ours (still alive); ESRCH => dead
   }
   if (!alive) return true; // owner process is gone
+  if (isZombie(pid)) return true; // exited but unreaped -> effectively gone
   // PID is alive. Only declare it abandoned if we can PROVE it is a different
   // instance. If the owner's stamp is unverifiable ("0"), or we can't read the
   // current start token, treat the live PID as the same instance and do NOT
@@ -243,9 +269,9 @@ function claimLock(lockFile, ownerId) {
 // adjacent syscalls with no subprocess in it; isAbandoned (which may spawn `ps`)
 // runs only *before* the re-read. Closing it completely needs an atomic
 // conditional-delete / OS advisory lock (flock), which Node's fs builtins do not
-// expose. The failure mode is bounded -- a losing contender times out within the
-// 15s acquire deadline (an error, not silent corruption) -- never a permanent
-// deadlock, since a dead owner is always reclaimable on the next pass.
+// expose. Within that ~2-syscall window the worst case is a brief overlap (two
+// writers) or a losing contender that errors out at the 15s acquire deadline;
+// never a permanent deadlock, since a dead owner is always reclaimable next pass.
 function reclaimIfAbandoned(lockFile, selfId) {
   let seen;
   try {
diff --git a/plugins/codex/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs
index 778571e6c..670d12484 100644
--- a/plugins/codex/scripts/session-lifecycle-hook.mjs
+++ b/plugins/codex/scripts/session-lifecycle-hook.mjs
@@ -13,7 +13,7 @@ import {
   sendBrokerShutdown,
   teardownBrokerSession
 } from "./lib/broker-lifecycle.mjs";
-import { loadState, resolveStateFile, saveState } from "./lib/state.mjs";
+import { loadState, resolveStateFile, updateState } from "./lib/state.mjs";
 import { TRANSCRIPT_PATH_ENV } from "./lib/claude-session-transfer.mjs";
 import { resolveWorkspaceRoot } from "./lib/workspace.mjs";
 
@@ -50,28 +50,49 @@ 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 stillRunning = job.status === "queued" || job.status === "running";
-    if (!stillRunning) {
-      continue;
+  // Drop this session's jobs through the locked read-modify-write so a concurrent
+  // upsertJob (task launch) can't be clobbered by a stale snapshot, and capture
+  // which running jobs to terminate. Process teardown runs in `finally`, after the
+  // lock is released, so a failed state write (e.g. the 15s lock-acquire timeout)
+  // can never leak this session's processes, and never aborts the rest of session
+  // shutdown (broker teardown) -- session cleanup is best-effort.
+  const isRunning = (job) => job.status === "queued" || job.status === "running";
+  const toTerminate = [];
+  try {
+    updateState(workspaceRoot, (state) => {
+      for (const job of state.jobs) {
+        if (job.sessionId === sessionId && isRunning(job)) {
+          toTerminate.push(job.pid ?? Number.NaN);
+        }
+      }
+      state.jobs = state.jobs.filter((job) => job.sessionId !== sessionId);
+    });
+  } catch {
+    // The locked update failed (e.g. lock-acquire timeout). Still identify this
+    // session's processes so we can tear them down -- via a best-effort unlocked
+    // read only. We deliberately do NOT write state here: an unlocked save is the
+    // very clobber this lock prevents; the stale records are removed on a later
+    // locked pass.
+    if (toTerminate.length === 0) {
+      try {
+        for (const job of loadState(workspaceRoot).jobs) {
+          if (job.sessionId === sessionId && isRunning(job)) {
+            toTerminate.push(job.pid ?? Number.NaN);
+          }
+        }
+      } catch {
+        // Nothing more we can do; fall through to whatever we collected.
+      }
     }
-    try {
-      terminateProcessTree(job.pid ?? Number.NaN);
-    } catch {
-      // Ignore teardown failures during session shutdown.
+  } finally {
+    for (const pid of toTerminate) {
+      try {
+        terminateProcessTree(pid);
+      } catch {
+        // Ignore teardown failures during session shutdown.
+      }
     }
   }
-
-  saveState(workspaceRoot, {
-    ...state,
-    jobs: state.jobs.filter((job) => job.sessionId !== sessionId)
-  });
 }
 
 function handleSessionStart(input) {

From 40032c5384941fa00d58a25160c1795feccdd459 Mon Sep 17 00:00:00 2001
From: rajasekar-venkatesan
 <22398308+rajasekar-venkatesan@users.noreply.github.com>
Date: Wed, 26 Aug 2026 15:01:17 +0800
Subject: [PATCH 6/9] Replace the state lock with lock-free per-job files

Repeated review rounds showed a pure-Node-fs cross-process lock cannot be made
crash-safe (no flock/advisory-lock in fs builtins), so every stale-takeover
variant left a residual. Remove the shared state.json jobs[] array and the lock
entirely: store one record per job at jobs/<id>.json and derive the list by
scanning that directory. Concurrent launches (and session cleanup) now touch
DIFFERENT files, so a stale snapshot clobbering a sibling job is impossible by
construction, not merely serialized.

- upsertJob/writeJobFile merge a patch into jobs/<id>.json via unique-temp +
  rename (atomic publish; readers never see a torn file). listJobs scans the dir,
  skipping ENOENT/parse failures. state.json now holds only config.
- Prune keeps the newest MAX_JOBS and evicts only terminal jobs, non-terminal
  jobs whose pid is dead, or pid-less records stuck past a 60s grace -- never a
  live queued/running job -- and removes the payload and both log paths.
- Legacy state.json jobs[] arrays are migrated into per-job files (additive
  merge, existing payload wins) then state.json is rewritten config-only.
- Session cleanup terminates this session's workers, waits for exit (escalating
  to SIGKILL), then deletes their files -- no lock, no locked read-modify-write.
- Enqueue writes the queued record before spawning (fixing a "No stored job
  found" race) and the worker owns all later lifecycle states; the worker skips a
  task already cancelled during its startup window. Removed a stale-snapshot
  re-write in the progress updater that could resurrect a cancelled job.

Job ids are validated before being used as paths. Guarded the per-job reads
against prune/cleanup TOCTOU. Tests updated to the per-job-file model.
---
 plugins/codex/scripts/codex-companion.mjs     |  33 +-
 plugins/codex/scripts/lib/job-control.mjs     |   8 +-
 plugins/codex/scripts/lib/state.mjs           | 505 +++++++-----------
 plugins/codex/scripts/lib/tracked-jobs.mjs    |  23 +-
 .../codex/scripts/session-lifecycle-hook.mjs  | 108 ++--
 tests/runtime.test.mjs                        |  32 +-
 tests/state.test.mjs                          | 179 +++----
 7 files changed, 404 insertions(+), 484 deletions(-)

diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs
index 83df468ad..c3498fd6f 100644
--- a/plugins/codex/scripts/codex-companion.mjs
+++ b/plugins/codex/scripts/codex-companion.mjs
@@ -685,17 +685,38 @@ function enqueueBackgroundTask(cwd, job, request) {
   const { logFile } = createTrackedProgress(job);
   appendLogLine(logFile, "Queued for background execution.");
 
-  const child = spawnDetachedTaskWorker(cwd, job.id);
+  // Publish the queued record BEFORE spawning the worker, so a fast worker always
+  // finds its job (previously it could start and fail with "No stored job found").
   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);
+
+  let child;
+  try {
+    child = spawnDetachedTaskWorker(cwd, job.id);
+  } catch (error) {
+    // Spawn failed synchronously: mark the record failed so it does not linger as
+    // a pending job.
+    writeJobFile(job.workspaceRoot, job.id, {
+      ...queuedRecord,
+      status: "failed",
+      phase: "failed",
+      errorMessage: error instanceof Error ? error.message : String(error)
+    });
+    throw error;
+  }
+
+  // Record only the child's pid (a pid-only patch, not a stale full snapshot), so
+  // a cancel/cleanup arriving before the worker publishes "running" can still
+  // terminate it. The worker owns every subsequent lifecycle transition; this
+  // merges into whatever the worker has already published without reverting it.
+  upsertJob(job.workspaceRoot, { id: job.id, pid: child.pid ?? null });
 
   return {
     payload: {
@@ -851,6 +872,12 @@ async function handleTaskWorker(argv) {
     throw new Error(`No stored job found for ${options["job-id"]}.`);
   }
 
+  // Honor a cancellation that landed during our startup window (before we could
+  // publish a pid): if the record is already cancelled, do not start the task.
+  if (storedJob.status === "cancelled") {
+    return;
+  }
+
   const request = storedJob.request;
   if (!request || typeof request !== "object") {
     throw new Error(`Stored job ${options["job-id"]} is missing its task request payload.`);
diff --git a/plugins/codex/scripts/lib/job-control.mjs b/plugins/codex/scripts/lib/job-control.mjs
index ad152c157..4ab08ff02 100644
--- a/plugins/codex/scripts/lib/job-control.mjs
+++ b/plugins/codex/scripts/lib/job-control.mjs
@@ -181,11 +181,13 @@ export function enrichJob(job, options = {}) {
 }
 
 export function readStoredJob(workspaceRoot, jobId) {
-  const jobFile = resolveJobFile(workspaceRoot, jobId);
-  if (!fs.existsSync(jobFile)) {
+  // Guarded read: the per-job file can be pruned or session-cleaned between the
+  // existence check and the read, so treat any read/parse failure as "absent".
+  try {
+    return readJobFile(resolveJobFile(workspaceRoot, jobId));
+  } catch {
     return null;
   }
-  return readJobFile(jobFile);
 }
 
 function matchJobReference(jobs, reference, predicate = () => true) {
diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs
index ba355cf02..d0d67e283 100644
--- a/plugins/codex/scripts/lib/state.mjs
+++ b/plugins/codex/scripts/lib/state.mjs
@@ -1,4 +1,3 @@
-import { spawnSync } from "node:child_process";
 import { createHash } from "node:crypto";
 import fs from "node:fs";
 import os from "node:os";
@@ -12,20 +11,31 @@ 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 QUEUED_GRACE_MS = 60000; // a pid-less queued record stuck longer than this is a crashed enqueuer
+const TERMINAL_STATUSES = new Set(["completed", "failed", "cancelled"]);
+
+// A job id becomes a filename, so reject anything that could escape the jobs dir.
+function isValidJobId(id) {
+  return typeof id === "string" && id.length > 0 && !id.includes("/") && !id.includes("\\") && id !== "." && id !== "..";
+}
+
+// Job state is stored one file per job under <stateDir>/jobs/<id>.json, and the
+// job list is derived by scanning that directory. There is deliberately NO shared
+// index and therefore NO cross-process lock: concurrent `task --background`
+// launches (and session cleanup) operate on DIFFERENT files and cannot clobber
+// each other, so the "a stale snapshot of a shared array overwrites a sibling
+// job" corruption is impossible by construction rather than merely serialized.
+// A rename publishes a whole record atomically, so a reader never observes a
+// torn/partial file. Only `config` (rarely written, by /setup) lives in
+// state.json. See PR openai/codex-plugin-cc#689 for the history behind this.
 
 function nowIso() {
   return new Date().toISOString();
 }
 
-function defaultState() {
-  return {
-    version: STATE_VERSION,
-    config: {
-      stopReviewGate: false
-    },
-    jobs: []
-  };
-}
+// ---------------------------------------------------------------------------
+// Paths
+// ---------------------------------------------------------------------------
 
 export function resolveStateDir(cwd) {
   const workspaceRoot = resolveWorkspaceRoot(cwd);
@@ -56,360 +66,235 @@ export function ensureStateDir(cwd) {
   fs.mkdirSync(resolveJobsDir(cwd), { recursive: true });
 }
 
-export function loadState(cwd) {
-  const stateFile = resolveStateFile(cwd);
-  if (!fs.existsSync(stateFile)) {
-    return defaultState();
-  }
-
-  try {
-    const parsed = JSON.parse(fs.readFileSync(stateFile, "utf8"));
-    return {
-      ...defaultState(),
-      ...parsed,
-      config: {
-        ...defaultState().config,
-        ...(parsed.config ?? {})
-      },
-      jobs: Array.isArray(parsed.jobs) ? parsed.jobs : []
-    };
-  } catch {
-    return defaultState();
-  }
-}
-
-function pruneJobs(jobs) {
-  return [...jobs]
-    .sort((left, right) => String(right.updatedAt ?? "").localeCompare(String(left.updatedAt ?? "")))
-    .slice(0, MAX_JOBS);
-}
-
-function removeFileIfExists(filePath) {
-  if (filePath && fs.existsSync(filePath)) {
-    fs.unlinkSync(filePath);
-  }
-}
-
-export function saveState(cwd, state) {
-  const previousJobs = loadState(cwd).jobs;
+export function resolveJobLogFile(cwd, jobId) {
   ensureStateDir(cwd);
-  const nextJobs = pruneJobs(state.jobs ?? []);
-  const nextState = {
-    version: STATE_VERSION,
-    config: {
-      ...defaultState().config,
-      ...(state.config ?? {})
-    },
-    jobs: nextJobs
-  };
-
-  const retainedIds = new Set(nextJobs.map((job) => job.id));
-  for (const job of previousJobs) {
-    if (retainedIds.has(job.id)) {
-      continue;
-    }
-    removeJobFile(resolveJobFile(cwd, job.id));
-    removeFileIfExists(job.logFile);
-  }
-
-  const stateFile = resolveStateFile(cwd);
-  const tmpFile = `${stateFile}.${process.pid}.tmp`;
-  fs.writeFileSync(tmpFile, `${JSON.stringify(nextState, null, 2)}\n`, "utf8");
-  fs.renameSync(tmpFile, stateFile); // atomic replace; readers never see a partial file
-  return nextState;
+  return path.join(resolveJobsDir(cwd), `${jobId}.log`);
 }
 
-// Blocking sleep for a sync context (no busy-wait) via Atomics.wait.
-function sleepSync(ms) {
-  Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
+export function resolveJobFile(cwd, jobId) {
+  ensureStateDir(cwd);
+  return path.join(resolveJobsDir(cwd), `${jobId}.json`);
 }
 
-// A per-process, restart-stable token identifying a specific process *instance*
-// (not just its PID), so PID reuse can be told apart from the original owner.
-// Lock files live under a per-workspace OS temp dir (see resolveStateDir), i.e. a
-// single host, so the PID refers to a process on this machine.
-//
-// The source is chosen by platform and never mixed: two renderings of the same
-// live process must compare equal, so we must not stamp with one source and check
-// with another. Returns null when the process is gone or its start time can't be
-// read (callers treat null conservatively -- never as "different instance").
-//   - Linux: /proc/<pid>/stat field 22 is the process start time (clock ticks
-//     since boot); read directly, no subprocess.
-//   - else (macOS/BSD): `ps -o lstart` is the start timestamp, stable for the
-//     process lifetime. The env is pinned (TZ/locale) because lstart is rendered
-//     with strftime and would otherwise differ between a stamper and a checker
-//     running under different TZ/LC settings. spawnSync is only reached on the
-//     (rare) reclaim path, never on the uncontended fast path.
-function processStartToken(pid) {
-  if (!Number.isInteger(pid) || pid <= 0) return null;
-  if (process.platform === "linux") {
-    try {
-      const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8");
-      // comm (field 2) may contain spaces/parens; the numeric fields start after
-      // the last ')'. starttime is field 22 => index 19 of that remainder.
-      const rest = stat.slice(stat.lastIndexOf(")") + 2).split(/\s+/);
-      if (rest[19]) return `L:${rest[19]}`;
-    } catch {}
-    return null; // no cross-source fallback -- see note above
-  }
-  try {
-    const r = spawnSync("/bin/ps", ["-o", "lstart=", "-p", String(pid)], {
-      encoding: "utf8",
-      env: { ...process.env, TZ: "UTC0", LC_ALL: "C", LANG: "C" },
-    });
-    if (r.status === 0) {
-      const s = (r.stdout || "").trim();
-      if (s) return `P:${s}`;
-    }
-  } catch {}
-  return null;
+export function generateJobId(prefix = "job") {
+  const random = Math.random().toString(36).slice(2, 8);
+  return `${prefix}-${Date.now().toString(36)}-${random}`;
 }
 
-// A process that has exited but not yet been reaped by its parent is a zombie:
-// process.kill(pid, 0) still succeeds and its start token is unchanged, so it
-// would otherwise look like a live owner forever. Detect it so its lock is
-// reclaimed instead of blocking every writer until the parent reaps it.
-function isZombie(pid) {
-  if (process.platform === "linux") {
-    try {
-      const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8");
-      // state (field 3) is the first token after the last ')'.
-      return stat.slice(stat.lastIndexOf(")") + 2).split(/\s+/)[0] === "Z";
-    } catch {
-      return false;
-    }
-  }
-  try {
-    const r = spawnSync("/bin/ps", ["-o", "state=", "-p", String(pid)], {
-      encoding: "utf8",
-      env: { ...process.env, LC_ALL: "C" },
-    });
-    return r.status === 0 && (r.stdout || "").trim().startsWith("Z");
-  } catch {
-    return false;
-  }
-}
+// ---------------------------------------------------------------------------
+// Atomic writes
+// ---------------------------------------------------------------------------
 
-// The lock owner is identified as "<pid>.<startTokenHex>.<time>.<rand>". Compute
-// the current process's identity once per acquisition.
-function selfOwnerId() {
-  const tok = processStartToken(process.pid);
-  const tag = tok ? Buffer.from(tok).toString("hex") : "0";
-  return `${process.pid}.${tag}.${Date.now().toString(36)}.${Math.random().toString(36).slice(2, 8)}`;
+function uniqueTmp(file) {
+  return `${file}.tmp.${process.pid}.${Math.random().toString(36).slice(2, 8)}`;
 }
 
-// True only when the exact process instance that wrote `id` is gone -- never for
-// a live owner, no matter how long it has been holding the lock. This is what
-// lets reclaim run without any time-based expiry: a suspended-but-alive holder is
-// never reclaimed, and a dead owner whose PID was recycled is detected because
-// the recycled process reports a different start token.
-function isAbandoned(id) {
-  const parts = String(id).split(".");
-  const pid = Number.parseInt(parts[0], 10);
-  if (!Number.isInteger(pid) || pid <= 0) return true; // empty/garbled -> not a live owner
-  let alive;
+// Publish `value` at `file` atomically. The temp lives in the same directory (so
+// rename is a same-filesystem atomic replace) and its name carries `.tmp.` so it
+// is never mistaken for a job file by listJobs.
+function atomicWriteJson(file, value) {
+  const tmp = uniqueTmp(file);
   try {
-    process.kill(pid, 0);
-    alive = true;
+    fs.writeFileSync(tmp, `${JSON.stringify(value, null, 2)}\n`, "utf8");
+    fs.renameSync(tmp, file);
   } catch (err) {
-    alive = err.code === "EPERM"; // exists but not ours (still alive); ESRCH => dead
+    try { fs.unlinkSync(tmp); } catch {} // never leave a partial temp behind
+    throw err;
   }
-  if (!alive) return true; // owner process is gone
-  if (isZombie(pid)) return true; // exited but unreaped -> effectively gone
-  // PID is alive. Only declare it abandoned if we can PROVE it is a different
-  // instance. If the owner's stamp is unverifiable ("0"), or we can't read the
-  // current start token, treat the live PID as the same instance and do NOT
-  // reclaim -- otherwise a checker that *can* probe would delete a live owner's
-  // lock whose owner merely failed to self-probe at stamp time. (pid-death
-  // reclaim above still applies, so this never causes a permanent deadlock.)
-  if (!parts[1] || parts[1] === "0") return false;
-  const cur = processStartToken(pid);
-  if (cur === null) return false;
-  return Buffer.from(cur).toString("hex") !== parts[1]; // different instance => PID reuse => owner gone
 }
 
-// Publish a lock atomically: write the owner id into a unique temp file, then
-// hard-link it onto the fixed path. linkSync is atomic and fails EEXIST if the
-// path is already held, exactly like O_EXCL -- but unlike open()+write() the file
-// has its full content the instant it appears at the path, so a concurrent reader
-// can never observe an empty lock and mistake a just-created live lock for an
-// abandoned one. Returns true if claimed, false if already held.
-function claimLock(lockFile, ownerId) {
-  const tmp = `${lockFile}.tmp.${ownerId}`;
-  fs.writeFileSync(tmp, ownerId);
+// ---------------------------------------------------------------------------
+// Liveness (used only to keep pruning from evicting a live job)
+// ---------------------------------------------------------------------------
+
+function pidAlive(pid) {
+  if (!Number.isInteger(pid) || pid <= 0) return false;
   try {
-    fs.linkSync(tmp, lockFile);
+    process.kill(pid, 0);
     return true;
   } catch (err) {
-    if (err.code === "EEXIST") return false;
-    throw err;
-  } finally {
-    try { fs.unlinkSync(tmp); } catch {}
+    return err.code === "EPERM"; // exists but not ours (still alive); ESRCH => dead
   }
 }
 
-// Reclaim a lock only when its owner instance is gone, never a live one.
-//
-// isAbandoned is true only for a dead owner (or a PID recycled by a different
-// instance), so this never fires against the live owner. Removal is atomic via
-// capture-by-rename: renameSync of the fixed path has exactly-one-winner
-// semantics, so when several launches race to reclaim the same lock only one
-// captures it and the losers get ENOENT and fall back to re-contending. The
-// captor then confirms the file it captured is byte-for-byte the id it judged
-// abandoned (the id embeds time+rand, so this is unique); if instead it captured
-// a lock that had been recreated in the meantime -- i.e. possibly a live one --
-// it restores it rather than deleting it. This needs no second lock, so there is
-// no reclaim-lock to itself go stale and be cleaned up unsafely.
-//
-// Residual (fundamental to pure-fs locking): between the re-read and the rename
-// below the lock could be reclaimed by someone else and freshly re-claimed by a
-// live owner; renaming then captures that live lock, and if a third process
-// claims the momentarily-absent path the restore relink loses (EEXIST) and the
-// captured lock is dropped. The pre-rename re-read shrinks that window to ~2
-// adjacent syscalls with no subprocess in it; isAbandoned (which may spawn `ps`)
-// runs only *before* the re-read. Closing it completely needs an atomic
-// conditional-delete / OS advisory lock (flock), which Node's fs builtins do not
-// expose. Within that ~2-syscall window the worst case is a brief overlap (two
-// writers) or a losing contender that errors out at the 15s acquire deadline;
-// never a permanent deadlock, since a dead owner is always reclaimable next pass.
-function reclaimIfAbandoned(lockFile, selfId) {
-  let seen;
+// ---------------------------------------------------------------------------
+// Legacy migration: older installs kept a jobs[] index array in state.json, with
+// some fields (startedAt, completedAt, summary, ...) living only in that index
+// and others only in the jobs/<id>.json payload. Fold each index entry INTO its
+// per-job file so the single record has both, then rewrite state.json config-only.
+// The merge is additive: an existing payload wins on conflicting keys (it is the
+// authoritative/newer record) and the index only fills in keys the payload lacks.
+// Idempotent; after the first run state.json has no jobs array and this no-ops.
+// ---------------------------------------------------------------------------
+
+function migrateLegacyState(cwd) {
+  const stateFile = resolveStateFile(cwd);
+  let parsed;
   try {
-    seen = fs.readFileSync(lockFile, "utf8");
+    parsed = JSON.parse(fs.readFileSync(stateFile, "utf8"));
   } catch {
-    return; // already gone
+    return; // no state.json (or unreadable) -> nothing to migrate
   }
-  if (!isAbandoned(seen)) return; // live owner -> wait, don't touch
-  const tomb = `${lockFile}.rip.${selfId}`;
-  // Re-read immediately before capturing so isAbandoned's (possibly subprocess-
-  // backed) probe is not inside the capture window: only proceed if the lock is
-  // still the exact abandoned instance we judged.
-  try {
-    if (fs.readFileSync(lockFile, "utf8") !== seen) return;
-  } catch {
-    return; // vanished -- re-contend
+  if (!parsed || !Array.isArray(parsed.jobs) || parsed.jobs.length === 0) {
+    return;
   }
-  try {
-    fs.renameSync(lockFile, tomb); // atomic: exactly one reclaimer captures the path
-  } catch {
-    return; // lost the race (ENOENT) -- another reclaimer took it; re-contend
+  ensureStateDir(cwd);
+  for (const job of parsed.jobs) {
+    if (!job || !isValidJobId(job.id)) continue;
+    const existing = readJobRecord(cwd, job.id);
+    atomicWriteJson(resolveJobFile(cwd, job.id), { ...job, ...(existing ?? {}) });
   }
   try {
-    if (fs.readFileSync(tomb, "utf8") === seen) {
-      fs.unlinkSync(tomb); // captured the exact abandoned instance we judged -> drop it
-    } else {
-      // Captured a lock recreated after our read -> may be live; put it back.
-      try { fs.linkSync(tomb, lockFile); } catch {}
-      fs.unlinkSync(tomb);
-    }
+    atomicWriteJson(stateFile, {
+      version: STATE_VERSION,
+      config: { stopReviewGate: false, ...(parsed.config ?? {}) }
+    });
   } catch {
-    try { fs.unlinkSync(tomb); } catch {}
+    // A concurrent migrator may have already rewritten it; harmless.
   }
 }
 
-// Cross-process lock around the state.json read-modify-write. Without it,
-// concurrent `task --background` launches each read the same base state, add
-// only their own job, and clobber siblings on write (and saveState's prune
-// then deletes the "orphan" job files). Serializing the RMW fixes both.
-function withStateLock(cwd, fn) {
-  ensureStateDir(cwd);
-  const lockFile = path.join(resolveStateDir(cwd), "state.lock");
-  // Identifies this exact process instance (pid + start-time), so others can tell
-  // a live owner from a recycled PID; the full string is stamped into the lock so
-  // only the true owner removes it on release.
-  const ownerId = selfOwnerId();
-  const deadline = Date.now() + 15000;
-  for (;;) {
-    if (claimLock(lockFile, ownerId)) break;
-    reclaimIfAbandoned(lockFile, ownerId);
-    if (Date.now() > deadline) throw new Error("Timed out acquiring Codex state lock");
-    sleepSync(20 + Math.floor(Math.random() * 30)); // jittered backoff
-  }
+// ---------------------------------------------------------------------------
+// Config (state.json holds config only)
+// ---------------------------------------------------------------------------
+
+function readConfig(cwd) {
   try {
-    return fn();
-  } finally {
-    // Only remove the lock if we still own it: if we were ever reclaimed, the
-    // contents no longer match ownerId and we must not delete the lock another
-    // process now holds.
-    try {
-      if (fs.readFileSync(lockFile, "utf8") === ownerId) fs.unlinkSync(lockFile);
-    } catch {}
+    const parsed = JSON.parse(fs.readFileSync(resolveStateFile(cwd), "utf8"));
+    return { stopReviewGate: false, ...(parsed?.config ?? {}) };
+  } catch {
+    return { stopReviewGate: false };
   }
 }
 
-export function updateState(cwd, mutate) {
-  return withStateLock(cwd, () => {
-    const state = loadState(cwd);
-    mutate(state);
-    return saveState(cwd, state);
-  });
+export function getConfig(cwd) {
+  migrateLegacyState(cwd);
+  return readConfig(cwd);
 }
 
-export function generateJobId(prefix = "job") {
-  const random = Math.random().toString(36).slice(2, 8);
-  return `${prefix}-${Date.now().toString(36)}-${random}`;
+export function setConfig(cwd, key, value) {
+  ensureStateDir(cwd);
+  migrateLegacyState(cwd);
+  const config = { ...readConfig(cwd), [key]: value };
+  atomicWriteJson(resolveStateFile(cwd), { version: STATE_VERSION, config });
+  return config;
 }
 
-export function upsertJob(cwd, jobPatch) {
-  return updateState(cwd, (state) => {
-    const timestamp = nowIso();
-    const existingIndex = state.jobs.findIndex((job) => job.id === jobPatch.id);
-    if (existingIndex === -1) {
-      state.jobs.unshift({
-        createdAt: timestamp,
-        updatedAt: timestamp,
-        ...jobPatch
-      });
-      return;
-    }
-    state.jobs[existingIndex] = {
-      ...state.jobs[existingIndex],
-      ...jobPatch,
-      updatedAt: timestamp
-    };
-  });
+// ---------------------------------------------------------------------------
+// Job records
+// ---------------------------------------------------------------------------
+
+function isJobFileName(name) {
+  return name.endsWith(".json") && !name.includes(".tmp.");
 }
 
-export function listJobs(cwd) {
-  return loadState(cwd).jobs;
+function readJobRecord(cwd, jobId) {
+  try {
+    return JSON.parse(fs.readFileSync(resolveJobFile(cwd, jobId), "utf8"));
+  } catch {
+    return null;
+  }
 }
 
-export function setConfig(cwd, key, value) {
-  return updateState(cwd, (state) => {
-    state.config = {
-      ...state.config,
-      [key]: value
-    };
-  });
+// Read every job record. Robust to concurrent create/delete/rename: a name that
+// vanished mid-scan (ENOENT) or a record captured mid-write (never happens with
+// atomic rename, but parse-guarded anyway) is simply skipped.
+function readAllJobs(cwd) {
+  let names;
+  try {
+    names = fs.readdirSync(resolveJobsDir(cwd));
+  } catch {
+    return []; // jobs dir not created yet
+  }
+  const jobs = [];
+  const dir = resolveJobsDir(cwd);
+  for (const name of names) {
+    if (!isJobFileName(name)) continue;
+    try {
+      jobs.push(JSON.parse(fs.readFileSync(path.join(dir, name), "utf8")));
+    } catch {
+      // deleted mid-scan or unparseable -> skip
+    }
+  }
+  return jobs;
 }
 
-export function getConfig(cwd) {
-  return loadState(cwd).config;
+// A record is reclaimable only if it is terminal, or a non-terminal job whose
+// owner is provably gone. A just-queued job (no pid yet) is PENDING, not dead, so
+// it must not be evicted -- except as a liveness backstop if it has been stuck far
+// longer than a normal queue->spawn transition (a crashed enqueuer).
+function isEvictable(job) {
+  if (typeof job.id !== "string") return false;
+  if (TERMINAL_STATUSES.has(job.status)) return true;
+  if (Number.isInteger(job.pid) && job.pid > 0) return !pidAlive(job.pid);
+  return Date.now() - Date.parse(job.updatedAt ?? "") > QUEUED_GRACE_MS;
 }
 
-export function writeJobFile(cwd, jobId, payload) {
+// Keep the newest MAX_JOBS records; evict the oldest reclaimable ones so a live
+// queued/running job is never made undiscoverable (which would break
+// status/cancel/session cleanup). Deletes the payload, its recorded log, and the
+// conventional log. Idempotent under concurrency; re-checks each candidate right
+// before deleting so a job that transitioned to live since the scan is spared.
+function pruneJobs(cwd) {
+  const jobs = readAllJobs(cwd);
+  const overflow = jobs.length - MAX_JOBS;
+  if (overflow <= 0) return;
+  const evictable = jobs
+    .filter(isEvictable)
+    .sort((a, b) => String(a.updatedAt ?? "").localeCompare(String(b.updatedAt ?? "")));
+  let removed = 0;
+  for (const job of evictable) {
+    if (removed >= overflow) break;
+    const current = readJobRecord(cwd, job.id);
+    if (current && !isEvictable(current)) continue; // became live/pending since scan
+    let gone = false;
+    try {
+      fs.unlinkSync(resolveJobFile(cwd, job.id));
+      gone = true;
+    } catch (err) {
+      gone = err.code === "ENOENT"; // already removed by another pruner -> counts
+    }
+    if (!gone) continue; // couldn't remove (e.g. EACCES) -> don't count against overflow
+    if (typeof job.logFile === "string") { try { fs.unlinkSync(job.logFile); } catch {} }
+    try { fs.unlinkSync(resolveJobLogFile(cwd, job.id)); } catch {}
+    removed += 1;
+  }
+}
+
+// Merge `patch` into job <id>'s record and publish it atomically. This is the
+// single write path for both a full record and an incremental patch; merging
+// (rather than overwriting) means a field written by one call site is never lost
+// by a later call that omits it (e.g. a `summary` added after the payload write).
+function mergeJobRecord(cwd, jobId, patch) {
   ensureStateDir(cwd);
-  const jobFile = resolveJobFile(cwd, jobId);
-  fs.writeFileSync(jobFile, `${JSON.stringify(payload, null, 2)}\n`, "utf8");
-  return jobFile;
+  const existing = readJobRecord(cwd, jobId) ?? {};
+  const now = nowIso();
+  const record = {
+    ...existing,
+    ...patch,
+    id: jobId,
+    createdAt: existing.createdAt ?? patch.createdAt ?? now,
+    updatedAt: now
+  };
+  atomicWriteJson(resolveJobFile(cwd, jobId), record);
+  pruneJobs(cwd);
+  return record;
 }
 
-export function readJobFile(jobFile) {
-  return JSON.parse(fs.readFileSync(jobFile, "utf8"));
+export function upsertJob(cwd, patch) {
+  return mergeJobRecord(cwd, patch.id, patch);
 }
 
-function removeJobFile(jobFile) {
-  if (fs.existsSync(jobFile)) {
-    fs.unlinkSync(jobFile);
-  }
+export function writeJobFile(cwd, jobId, payload) {
+  mergeJobRecord(cwd, jobId, payload);
+  return resolveJobFile(cwd, jobId);
 }
 
-export function resolveJobLogFile(cwd, jobId) {
-  ensureStateDir(cwd);
-  return path.join(resolveJobsDir(cwd), `${jobId}.log`);
+export function readJobFile(jobFile) {
+  return JSON.parse(fs.readFileSync(jobFile, "utf8"));
 }
 
-export function resolveJobFile(cwd, jobId) {
-  ensureStateDir(cwd);
-  return path.join(resolveJobsDir(cwd), `${jobId}.json`);
+export function listJobs(cwd) {
+  migrateLegacyState(cwd);
+  return readAllJobs(cwd);
 }
diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs
index 902869012..383d97002 100644
--- a/plugins/codex/scripts/lib/tracked-jobs.mjs
+++ b/plugins/codex/scripts/lib/tracked-jobs.mjs
@@ -99,18 +99,11 @@ export function createJobProgressUpdater(workspaceRoot, jobId) {
       return;
     }
 
+    // upsertJob merges the patch into the single per-job record atomically, so the
+    // former re-read + full-snapshot writeJobFile is not only redundant now but
+    // unsafe: passing a stale snapshot could resurrect a concurrently cancelled or
+    // completed job back to "running".
     upsertJob(workspaceRoot, patch);
-
-    const jobFile = resolveJobFile(workspaceRoot, jobId);
-    if (!fs.existsSync(jobFile)) {
-      return;
-    }
-
-    const storedJob = readJobFile(jobFile);
-    writeJobFile(workspaceRoot, jobId, {
-      ...storedJob,
-      ...patch
-    });
   };
 }
 
@@ -132,11 +125,13 @@ export function createProgressReporter({ stderr = false, logFile = null, onEvent
 }
 
 function readStoredJobOrNull(workspaceRoot, jobId) {
-  const jobFile = resolveJobFile(workspaceRoot, jobId);
-  if (!fs.existsSync(jobFile)) {
+  // Guarded read: the per-job file can be pruned or session-cleaned between an
+  // existence check and the read, so treat any read/parse failure as "absent".
+  try {
+    return readJobFile(resolveJobFile(workspaceRoot, jobId));
+  } catch {
     return null;
   }
-  return readJobFile(jobFile);
 }
 
 export async function runTrackedJob(job, runner, options = {}) {
diff --git a/plugins/codex/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs
index 670d12484..847bdf8de 100644
--- a/plugins/codex/scripts/session-lifecycle-hook.mjs
+++ b/plugins/codex/scripts/session-lifecycle-hook.mjs
@@ -13,7 +13,13 @@ import {
   sendBrokerShutdown,
   teardownBrokerSession
 } from "./lib/broker-lifecycle.mjs";
-import { loadState, resolveStateFile, updateState } from "./lib/state.mjs";
+import {
+  listJobs,
+  resolveJobFile,
+  resolveJobLogFile,
+  resolveJobsDir,
+  resolveStateFile
+} from "./lib/state.mjs";
 import { TRANSCRIPT_PATH_ENV } from "./lib/claude-session-transfer.mjs";
 import { resolveWorkspaceRoot } from "./lib/workspace.mjs";
 
@@ -39,59 +45,81 @@ function appendEnvVar(name, value) {
   fs.appendFileSync(process.env.CLAUDE_ENV_FILE, `export ${name}=${shellEscape(value)}\n`, "utf8");
 }
 
+function pidAlive(pid) {
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch (err) {
+    return err.code === "EPERM";
+  }
+}
+
+// Block (bounded) until every pid has exited, or capMs elapses. Workers install no
+// SIGTERM handler so they normally die within a few ms; the cap prevents a hang.
+function waitForExit(pids, capMs) {
+  const deadline = Date.now() + capMs;
+  let remaining = pids.slice();
+  while (remaining.length > 0 && Date.now() < deadline) {
+    remaining = remaining.filter(pidAlive);
+    if (remaining.length > 0) {
+      Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 50);
+    }
+  }
+}
+
 function cleanupSessionJobs(cwd, sessionId) {
   if (!cwd || !sessionId) {
     return;
   }
 
   const workspaceRoot = resolveWorkspaceRoot(cwd);
-  const stateFile = resolveStateFile(workspaceRoot);
-  if (!fs.existsSync(stateFile)) {
+  if (!fs.existsSync(resolveStateFile(workspaceRoot)) && !fs.existsSync(resolveJobsDir(workspaceRoot))) {
     return;
   }
 
-  // Drop this session's jobs through the locked read-modify-write so a concurrent
-  // upsertJob (task launch) can't be clobbered by a stale snapshot, and capture
-  // which running jobs to terminate. Process teardown runs in `finally`, after the
-  // lock is released, so a failed state write (e.g. the 15s lock-acquire timeout)
-  // can never leak this session's processes, and never aborts the rest of session
-  // shutdown (broker teardown) -- session cleanup is best-effort.
+  // Job state is one file per job with no shared lock, so cleanup needs no locked
+  // read-modify-write. Terminate this session's live workers FIRST, wait briefly
+  // for them to actually exit, THEN delete their records -- so a still-dying worker
+  // cannot re-create a record we just removed.
   const isRunning = (job) => job.status === "queued" || job.status === "running";
-  const toTerminate = [];
+  let jobs = [];
   try {
-    updateState(workspaceRoot, (state) => {
-      for (const job of state.jobs) {
-        if (job.sessionId === sessionId && isRunning(job)) {
-          toTerminate.push(job.pid ?? Number.NaN);
-        }
-      }
-      state.jobs = state.jobs.filter((job) => job.sessionId !== sessionId);
-    });
+    jobs = listJobs(workspaceRoot).filter((job) => job.sessionId === sessionId);
   } catch {
-    // The locked update failed (e.g. lock-acquire timeout). Still identify this
-    // session's processes so we can tear them down -- via a best-effort unlocked
-    // read only. We deliberately do NOT write state here: an unlocked save is the
-    // very clobber this lock prevents; the stale records are removed on a later
-    // locked pass.
-    if (toTerminate.length === 0) {
-      try {
-        for (const job of loadState(workspaceRoot).jobs) {
-          if (job.sessionId === sessionId && isRunning(job)) {
-            toTerminate.push(job.pid ?? Number.NaN);
-          }
-        }
-      } catch {
-        // Nothing more we can do; fall through to whatever we collected.
-      }
+    return;
+  }
+
+  const pids = [];
+  for (const job of jobs) {
+    if (!isRunning(job)) continue;
+    const pid = job.pid;
+    if (Number.isInteger(pid) && pid > 0) pids.push(pid);
+    try {
+      terminateProcessTree(pid ?? Number.NaN);
+    } catch {
+      // Ignore teardown failures during session shutdown.
     }
-  } finally {
-    for (const pid of toTerminate) {
-      try {
-        terminateProcessTree(pid);
-      } catch {
-        // Ignore teardown failures during session shutdown.
-      }
+  }
+  waitForExit(pids, 2000);
+  // Escalate to SIGKILL for any worker that ignored SIGTERM, so it cannot survive
+  // to re-create a record we are about to delete, then give it a moment to die.
+  const survivors = pids.filter(pidAlive);
+  for (const pid of survivors) {
+    try { process.kill(pid, "SIGKILL"); } catch {}
+  }
+  if (survivors.length > 0) {
+    waitForExit(survivors, 500);
+  }
+
+  for (const job of jobs) {
+    if (typeof job.id !== "string") continue;
+    try { fs.unlinkSync(resolveJobFile(workspaceRoot, job.id)); } catch {}
+    // Remove the log at its recorded path (which need not be jobs/<id>.log) as
+    // well as the conventional path, so no orphan log survives cleanup.
+    if (typeof job.logFile === "string") {
+      try { fs.unlinkSync(job.logFile); } catch {}
     }
+    try { fs.unlinkSync(resolveJobLogFile(workspaceRoot, job.id)); } catch {}
   }
 }
 
diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs
index 8f276835b..a56e9ab90 100644
--- a/tests/runtime.test.mjs
+++ b/tests/runtime.test.mjs
@@ -10,6 +10,22 @@ 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";
 
+// Jobs are stored one file per job under <stateDir>/jobs/<id>.json (no shared
+// index array). Read them back newest-first, the way the CLI lists them.
+function jobsFromStateDir(stateDir) {
+  const jobsDir = path.join(stateDir, "jobs");
+  let names;
+  try {
+    names = fs.readdirSync(jobsDir);
+  } catch {
+    return [];
+  }
+  return names
+    .filter((name) => name.endsWith(".json") && !name.includes(".tmp."))
+    .map((name) => JSON.parse(fs.readFileSync(path.join(jobsDir, name), "utf8")))
+    .sort((a, b) => String(b.updatedAt ?? "").localeCompare(String(a.updatedAt ?? "")));
+}
+
 const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..");
 const PLUGIN_ROOT = path.join(ROOT, "plugins", "codex");
 const SCRIPT = path.join(PLUGIN_ROOT, "scripts", "codex-companion.mjs");
@@ -471,7 +487,7 @@ test("review logs reasoning summaries and review output to the job log", () => {
 
   assert.equal(result.status, 0, result.stderr);
   const stateDir = resolveStateDir(repo);
-  const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8"));
+  const state = { jobs: jobsFromStateDir(stateDir) };
   const log = fs.readFileSync(state.jobs[0].logFile, "utf8");
   assert.match(log, /Reasoning summary/);
   assert.match(log, /Reviewed the changed files and checked the likely regression paths/);
@@ -800,7 +816,7 @@ test("task logs reasoning summaries and assistant messages to the job log", () =
 
   assert.equal(result.status, 0, result.stderr);
   const stateDir = resolveStateDir(repo);
-  const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8"));
+  const state = { jobs: jobsFromStateDir(stateDir) };
   const log = fs.readFileSync(state.jobs[0].logFile, "utf8");
   assert.match(log, /Reasoning summary/);
   assert.match(log, /Inspected the prompt, gathered evidence, and checked the highest-risk paths first/);
@@ -824,7 +840,7 @@ test("task logs subagent reasoning and messages with a subagent prefix", () => {
 
   assert.equal(result.status, 0, result.stderr);
   const stateDir = resolveStateDir(repo);
-  const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8"));
+  const state = { jobs: jobsFromStateDir(stateDir) };
   const log = fs.readFileSync(state.jobs[0].logFile, "utf8");
   assert.match(log, /Starting subagent design-challenger via collaboration tool: wait\./);
   assert.match(log, /Subagent design-challenger reasoning:/);
@@ -1624,7 +1640,7 @@ test("cancel stops an active background job and marks it cancelled", async (t) =
     }
   });
 
-  const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8"));
+  const state = { jobs: jobsFromStateDir(stateDir) };
   const cancelled = state.jobs.find((job) => job.id === "task-live");
   assert.equal(cancelled.status, "cancelled");
   assert.equal(cancelled.pid, null);
@@ -1685,7 +1701,7 @@ test("cancel without a job id ignores active jobs from other Claude sessions", (
   assert.equal(cancel.status, 1);
   assert.match(cancel.stderr, /No active Codex jobs to cancel for this session\./);
 
-  const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8"));
+  const state = { jobs: jobsFromStateDir(stateDir) };
   assert.equal(state.jobs[0].status, "running");
 });
 
@@ -1733,7 +1749,7 @@ test("cancel with a job id can still target an active job from another Claude se
   assert.equal(cancel.status, 0, cancel.stderr);
   assert.equal(JSON.parse(cancel.stdout).jobId, "task-other");
 
-  const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8"));
+  const state = { jobs: jobsFromStateDir(stateDir) };
   assert.equal(state.jobs[0].status, "cancelled");
 });
 
@@ -1760,7 +1776,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 state = { jobs: jobsFromStateDir(stateDir) };
     const job = state.jobs.find((candidate) => candidate.id === jobId);
     if (job?.status === "running" && job.threadId && job.turnId) {
       return job;
@@ -1917,7 +1933,7 @@ test("session end fully cleans up jobs for the ending session", async (t) => {
     }
   });
 
-  const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8"));
+  const state = { jobs: jobsFromStateDir(stateDir) };
   assert.deepEqual(state.jobs.map((job) => job.id), ["review-other"]);
   const otherJob = state.jobs[0];
   assert.equal(otherJob.logFile, otherSessionLog);
diff --git a/tests/state.test.mjs b/tests/state.test.mjs
index 01cf0bdf1..3df2b8b13 100644
--- a/tests/state.test.mjs
+++ b/tests/state.test.mjs
@@ -8,7 +8,16 @@ import { spawn } from "node:child_process";
 import { fileURLToPath, pathToFileURL } from "node:url";
 
 import { makeTempDir } from "./helpers.mjs";
-import { resolveJobFile, resolveJobLogFile, resolveStateDir, resolveStateFile, saveState } from "../plugins/codex/scripts/lib/state.mjs";
+import {
+  ensureStateDir,
+  listJobs,
+  resolveJobFile,
+  resolveJobLogFile,
+  resolveStateDir,
+  resolveStateFile,
+  upsertJob,
+  writeJobFile
+} from "../plugins/codex/scripts/lib/state.mjs";
 
 const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..");
 const STATE_URL = pathToFileURL(path.join(ROOT, "plugins", "codex", "scripts", "lib", "state.mjs")).href;
@@ -46,73 +55,22 @@ test("resolveStateDir uses CLAUDE_PLUGIN_DATA when it is provided", () => {
   }
 });
 
-test("saveState prunes dropped job artifacts when indexed jobs exceed the cap", () => {
+test("upsertJob merges a patch into a job's record without dropping fields", () => {
   const workspace = makeTempDir();
-  const stateFile = resolveStateFile(workspace);
-  fs.mkdirSync(path.dirname(stateFile), { recursive: true });
-
-  const jobs = Array.from({ length: 51 }, (_, index) => {
-    const jobId = `job-${index}`;
-    const updatedAt = new Date(Date.UTC(2026, 0, 1, 0, index, 0)).toISOString();
-    const logFile = resolveJobLogFile(workspace, jobId);
-    const jobFile = resolveJobFile(workspace, jobId);
-    fs.writeFileSync(logFile, `log ${jobId}\n`, "utf8");
-    fs.writeFileSync(jobFile, JSON.stringify({ id: jobId, status: "completed" }, null, 2), "utf8");
-    return {
-      id: jobId,
-      status: "completed",
-      logFile,
-      updatedAt,
-      createdAt: updatedAt
-    };
-  });
-
-  fs.writeFileSync(
-    stateFile,
-    `${JSON.stringify(
-      {
-        version: 1,
-        config: { stopReviewGate: false },
-        jobs
-      },
-      null,
-      2
-    )}\n`,
-    "utf8"
-  );
-
-  saveState(workspace, {
-    version: 1,
-    config: { stopReviewGate: false },
-    jobs
-  });
-
-  const prunedJobFile = resolveJobFile(workspace, "job-0");
-  const prunedLogFile = resolveJobLogFile(workspace, "job-0");
-  const retainedJobFile = resolveJobFile(workspace, "job-50");
-  const retainedLogFile = resolveJobLogFile(workspace, "job-50");
-  const jobsDir = path.dirname(prunedJobFile);
-
-  assert.equal(fs.existsSync(retainedJobFile), true);
-  assert.equal(fs.existsSync(retainedLogFile), true);
-
-  const savedState = JSON.parse(fs.readFileSync(stateFile, "utf8"));
-  assert.equal(savedState.jobs.length, 50);
-  assert.deepEqual(
-    savedState.jobs.map((job) => job.id),
-    Array.from({ length: 50 }, (_, index) => `job-${50 - index}`)
-  );
-  assert.deepEqual(
-    fs.readdirSync(jobsDir).sort(),
-    Array.from({ length: 50 }, (_, index) => `job-${index + 1}`)
-      .flatMap((jobId) => [`${jobId}.json`, `${jobId}.log`])
-      .sort()
-  );
+  writeJobFile(workspace, "j1", { id: "j1", status: "running", sessionId: "s1", pid: 123 });
+  upsertJob(workspace, { id: "j1", status: "completed", summary: "done" });
+
+  const job = listJobs(workspace).find((entry) => entry.id === "j1");
+  assert.equal(job.status, "completed");
+  assert.equal(job.summary, "done");
+  assert.equal(job.sessionId, "s1"); // field from the earlier write is preserved
+  assert.ok(job.createdAt);
+  assert.ok(job.updatedAt);
 });
 
-test("updateState serializes concurrent job writes without losing records", async () => {
+test("concurrent upserts of different jobs never lose a record (no lock)", async () => {
   const workspace = makeTempDir();
-  const jobCount = 12;
+  const jobCount = 16;
   const worker =
     `import { upsertJob } from ${JSON.stringify(STATE_URL)};\n` +
     "const [cwd, id] = process.argv.slice(1);\n" +
@@ -133,54 +91,63 @@ test("updateState serializes concurrent job writes without losing records", asyn
     )
   );
 
-  const saved = JSON.parse(fs.readFileSync(resolveStateFile(workspace), "utf8"));
-  const trackedIds = new Set(saved.jobs.map((job) => job.id));
+  const ids = new Set(listJobs(workspace).map((job) => job.id));
   const missing = Array.from({ length: jobCount }, (_, index) => `job-${index}`).filter(
-    (id) => !trackedIds.has(id)
+    (id) => !ids.has(id)
   );
-
-  assert.deepEqual(missing, [], "every concurrently launched job must be tracked");
+  assert.deepEqual(missing, [], "every concurrently launched job must be present");
 });
 
-test("updateState reclaims a stale lock without losing records under concurrency", async () => {
+test("prune evicts oldest terminal jobs but keeps live and non-terminal ones over the cap", () => {
   const workspace = makeTempDir();
-
-  // Simulate a crashed holder: a lock file older than the 10s stale threshold.
-  const stateDir = resolveStateDir(workspace);
-  fs.mkdirSync(stateDir, { recursive: true });
-  const lockFile = path.join(stateDir, "state.lock");
-  // Stamp a dead PID as the owner ("<pid>.<startTag>.<time>.<rand>") so the
-  // instance-liveness reclaim path fires: process.kill on this high, almost-
-  // certainly-dead PID throws ESRCH, so the lock is treated as abandoned.
-  fs.writeFileSync(lockFile, "2147483646.0.dead.owner");
-
-  const jobCount = 10;
-  const worker =
-    `import { upsertJob } from ${JSON.stringify(STATE_URL)};\n` +
-    "const [cwd, id] = process.argv.slice(1);\n" +
-    "upsertJob(cwd, { id, status: \"queued\", jobClass: \"task\", summary: id });\n";
-
-  await Promise.all(
-    Array.from({ length: jobCount }, (_, index) =>
-      new Promise((resolve, reject) => {
-        const child = spawn(
-          process.execPath,
-          ["--input-type=module", "-e", worker, workspace, `job-${index}`],
-          { stdio: "ignore" }
-        );
-        child.on("exit", (code) =>
-          code === 0 ? resolve() : reject(new Error(`worker ${index} exited ${code}`))
-        );
-      })
-    )
+  ensureStateDir(workspace);
+
+  // 50 old terminal jobs (completed), staggered updatedAt so ordering is defined.
+  for (let i = 0; i < 50; i += 1) {
+    const id = `done-${String(i).padStart(2, "0")}`;
+    const ts = new Date(Date.UTC(2026, 0, 1, 0, i, 0)).toISOString();
+    fs.writeFileSync(resolveJobFile(workspace, id), JSON.stringify({ id, status: "completed", updatedAt: ts, createdAt: ts }));
+    fs.writeFileSync(resolveJobLogFile(workspace, id), `log ${id}\n`);
+  }
+  // A running job owned by THIS (alive) process, with the oldest timestamp of all.
+  const liveTs = new Date(Date.UTC(2025, 0, 1)).toISOString();
+  fs.writeFileSync(
+    resolveJobFile(workspace, "live"),
+    JSON.stringify({ id: "live", status: "running", pid: process.pid, updatedAt: liveTs, createdAt: liveTs })
   );
 
-  const saved = JSON.parse(fs.readFileSync(resolveStateFile(workspace), "utf8"));
-  const trackedIds = new Set(saved.jobs.map((job) => job.id));
-  const missing = Array.from({ length: jobCount }, (_, index) => `job-${index}`).filter(
-    (id) => !trackedIds.has(id)
-  );
+  // 52 files now (> MAX_JOBS=50). One more write triggers prune (overflow 2).
+  upsertJob(workspace, { id: "trigger", status: "completed" });
 
-  assert.deepEqual(missing, [], "stale-lock reclaim must not drop concurrent job records");
-  assert.equal(fs.existsSync(lockFile), false, "lock file should be released after all writers finish");
+  const ids = new Set(listJobs(workspace).map((job) => job.id));
+  assert.equal(ids.has("live"), true, "a live running job must never be pruned, even as the oldest");
+  // The two oldest *terminal* jobs are evicted (done-00, done-01), payload + log.
+  assert.equal(ids.has("done-00"), false);
+  assert.equal(ids.has("done-01"), false);
+  assert.equal(fs.existsSync(resolveJobLogFile(workspace, "done-00")), false, "evicted job's log is removed too");
+  assert.equal(ids.has("done-02"), true, "newer terminal jobs are kept");
+});
+
+test("legacy state.json jobs[] array migrates to per-job files on read", () => {
+  const workspace = makeTempDir();
+  ensureStateDir(workspace);
+  const legacy = {
+    version: 1,
+    config: { stopReviewGate: true },
+    jobs: [
+      { id: "old-a", status: "completed", sessionId: "s", updatedAt: "2026-01-01T00:00:00.000Z" },
+      { id: "old-b", status: "failed", sessionId: "s", updatedAt: "2026-01-01T00:01:00.000Z" }
+    ]
+  };
+  fs.writeFileSync(resolveStateFile(workspace), JSON.stringify(legacy, null, 2));
+
+  const ids = new Set(listJobs(workspace).map((job) => job.id));
+  assert.equal(ids.has("old-a"), true);
+  assert.equal(ids.has("old-b"), true);
+  assert.equal(fs.existsSync(resolveJobFile(workspace, "old-a")), true, "legacy entry materialized as a per-job file");
+
+  // state.json is rewritten config-only (no jobs array left to re-migrate/resurrect).
+  const rewritten = JSON.parse(fs.readFileSync(resolveStateFile(workspace), "utf8"));
+  assert.equal(Array.isArray(rewritten.jobs), false);
+  assert.equal(rewritten.config.stopReviewGate, true);
 });

From e1b659188a90526a0925dc1613e6343f0a0cbba1 Mon Sep 17 00:00:00 2001
From: rajasekar-venkatesan
 <22398308+rajasekar-venkatesan@users.noreply.github.com>
Date: Wed, 26 Aug 2026 17:26:15 +0800
Subject: [PATCH 7/9] Make cancellation lock-free and race-safe via immutable
 markers

Replace the mutable-record cancellation path (which lost updates and could
resurrect cancelled jobs under the lock-free per-file store) with a
single-writer record plus immutable cancel/session markers and a
publish-pid / check-marker handshake.

- Record is single-writer: the parent no longer patches the child pid after
  spawn; the worker publishes its own pid with the running record. The
  pre-spawn queued write is an atomic wx claim (createJsonExclusive), so a
  duplicate id cannot spawn a second worker; mergeJobRecord is a plain
  additive merge again.
- Cancellation is an immutable jobs/<id>.cancelled marker (atomic wx create);
  readAllJobs overlays it so a cancellation can never be masked by a racing
  worker write. A session-<hash>.ended marker closes the enqueue-after-scan
  window.
- Handshake: cancel/cleanup create the marker BEFORE reading the pid; the
  worker publishes its pid BEFORE re-checking the marker, so at least one side
  observes the other -- the worker is killed or it self-aborts.
- Session cleanup writes the session marker before its raw scan, keeps a
  booting/still-runnable worker's record+marker, and deletes only
  provably-dead or terminal jobs. prune evicts on the raw record and never
  age-evicts a pid-less non-terminal job. Legacy migration is create-exclusive.
---
 plugins/codex/scripts/codex-companion.mjs     |  90 +++++--
 plugins/codex/scripts/lib/state.mjs           | 244 +++++++++++++++---
 plugins/codex/scripts/lib/tracked-jobs.mjs    |  45 ++--
 .../codex/scripts/session-lifecycle-hook.mjs  | 121 ++++++---
 tests/runtime.test.mjs                        |  49 ++--
 tests/session-cleanup.test.mjs                |  54 ++++
 tests/state.test.mjs                          |  81 ++++++
 7 files changed, 539 insertions(+), 145 deletions(-)
 create mode 100644 tests/session-cleanup.test.mjs

diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs
index c3498fd6f..ac40d8217 100644
--- a/plugins/codex/scripts/codex-companion.mjs
+++ b/plugins/codex/scripts/codex-companion.mjs
@@ -27,11 +27,15 @@ import { collectReviewContext, ensureGitRepository, resolveReviewTarget } from "
 import { binaryAvailable, terminateProcessTree } from "./lib/process.mjs";
 import { loadPromptTemplate, interpolateTemplate } from "./lib/prompts.mjs";
 import {
+  claimJobRecord,
   generateJobId,
   getConfig,
+  isJobCancelled,
+  isSessionEnded,
   listJobs,
+  markJobCancelled,
+  readJobPid,
   setConfig,
-  upsertJob,
   writeJobFile
 } from "./lib/state.mjs";
 import {
@@ -682,11 +686,20 @@ function spawnDetachedTaskWorker(cwd, jobId) {
 }
 
 function enqueueBackgroundTask(cwd, job, request) {
+  // Refuse to enqueue into a session that has already ended: otherwise a task launched
+  // after session cleanup's one-shot scan would be neither tombstoned nor terminated.
+  if (isSessionEnded(job.workspaceRoot, job.sessionId)) {
+    throw new Error("This session has ended; cannot launch a new background task.");
+  }
+
   const { logFile } = createTrackedProgress(job);
   appendLogLine(logFile, "Queued for background execution.");
 
-  // Publish the queued record BEFORE spawning the worker, so a fast worker always
-  // finds its job (previously it could start and fail with "No stored job found").
+  // CLAIM the queued record BEFORE spawning the worker, with an atomic create (never an
+  // overwrite). This both makes the record findable by a fast worker AND enforces
+  // single-writer ownership: if the id already exists (an astronomically unlikely
+  // duplicate from generateJobId, or a double launch), we refuse rather than spawn a
+  // second worker onto the same record.
   const queuedRecord = {
     ...job,
     status: "queued",
@@ -695,11 +708,12 @@ function enqueueBackgroundTask(cwd, job, request) {
     logFile,
     request
   };
-  writeJobFile(job.workspaceRoot, job.id, queuedRecord);
+  if (!claimJobRecord(job.workspaceRoot, job.id, queuedRecord)) {
+    throw new Error(`Job id ${job.id} already exists; refusing to launch a duplicate worker.`);
+  }
 
-  let child;
   try {
-    child = spawnDetachedTaskWorker(cwd, job.id);
+    spawnDetachedTaskWorker(cwd, job.id);
   } catch (error) {
     // Spawn failed synchronously: mark the record failed so it does not linger as
     // a pending job.
@@ -712,11 +726,12 @@ function enqueueBackgroundTask(cwd, job, request) {
     throw error;
   }
 
-  // Record only the child's pid (a pid-only patch, not a stale full snapshot), so
-  // a cancel/cleanup arriving before the worker publishes "running" can still
-  // terminate it. The worker owns every subsequent lifecycle transition; this
-  // merges into whatever the worker has already published without reverting it.
-  upsertJob(job.workspaceRoot, { id: job.id, pid: child.pid ?? null });
+  // The worker owns the record from here: it publishes `running` with its own pid as
+  // its first act (see runTrackedJob). The parent deliberately does NOT patch the pid
+  // in, so the per-job record has a SINGLE writer -- no concurrent read-modify-write,
+  // hence no lost update. A cancel/session-end arriving before the worker publishes
+  // `running` is honored by the immutable cancel/session-ended markers, which the
+  // worker consults at startup and again right after it publishes its pid.
 
   return {
     payload: {
@@ -872,9 +887,21 @@ async function handleTaskWorker(argv) {
     throw new Error(`No stored job found for ${options["job-id"]}.`);
   }
 
-  // Honor a cancellation that landed during our startup window (before we could
-  // publish a pid): if the record is already cancelled, do not start the task.
-  if (storedJob.status === "cancelled") {
+  // Honor a cancellation or a session end that landed during our startup window
+  // (before we could publish a pid): the immutable cancel marker and the session-ended
+  // marker are the durable, race-free signals, so consult them (not just the record,
+  // which a stale reader could have missed). We are the single writer, so record a
+  // cancelled terminal state before returning (keeps the raw record truthful; the marker
+  // overlay would show cancelled regardless). runTrackedJob re-checks the markers again
+  // after it publishes our pid, closing the rest of the window.
+  if (storedJob.status === "cancelled" || isJobCancelled(workspaceRoot, options["job-id"]) || isSessionEnded(workspaceRoot, storedJob.sessionId)) {
+    writeJobFile(workspaceRoot, options["job-id"], {
+      ...storedJob,
+      status: "cancelled",
+      phase: "cancelled",
+      pid: null,
+      completedAt: nowIso()
+    });
     return;
   }
 
@@ -1010,7 +1037,23 @@ async function handleCancel(argv) {
     );
   }
 
-  terminateProcessTree(job.pid ?? Number.NaN);
+  // Publish the immutable cancel marker BEFORE terminating, so a worker that publishes
+  // its pid concurrently is guaranteed to see the marker on its post-pid re-check and
+  // self-abort (the mirror of runTrackedJob's handshake). The marker is the RACE-SAFE
+  // authority: readers overlay it, so even if a worker writes `running`/`completed`
+  // after this, the job still reads as cancelled and can never be resurrected.
+  //
+  // Ordering is load-bearing: create the marker FIRST, THEN read the pid to kill. The
+  // worker does the mirror (publish pid, then read marker), so at least one side always
+  // observes the other -- we kill the worker, or it self-aborts. Reading a stale pid
+  // from before the marker would let a worker slip through both checks.
+  markJobCancelled(workspaceRoot, job.id, "Cancelled by user.");
+  // Read the pid FRESH, after the marker. No fallback to the pre-marker snapshot pid: a
+  // fresh null means the worker either never published or already exited, and killing a
+  // stale pid risks signalling an unrelated process that reused it. If it's null, the
+  // worker will honor the marker on its own post-pid re-check.
+  const killPid = readJobPid(workspaceRoot, job.id);
+  terminateProcessTree(killPid ?? Number.NaN);
   appendLogLine(job.logFile, "Cancelled by user.");
 
   const completedAt = nowIso();
@@ -1023,19 +1066,10 @@ async function handleCancel(argv) {
     errorMessage: "Cancelled by user."
   };
 
-  writeJobFile(workspaceRoot, job.id, {
-    ...existing,
-    ...nextJob,
-    cancelledAt: completedAt
-  });
-  upsertJob(workspaceRoot, {
-    id: job.id,
-    status: "cancelled",
-    phase: "cancelled",
-    pid: null,
-    errorMessage: "Cancelled by user.",
-    completedAt
-  });
+  // We deliberately do NOT write the record here. The marker is the authority: every
+  // read overlays it to cancelled, and the worker (the record's single writer) records
+  // the terminal cancelled state when it observes the marker. A cancel-command record
+  // write would reintroduce a second writer racing the worker.
 
   const payload = {
     jobId: job.id,
diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs
index d0d67e283..bf8ee5aca 100644
--- a/plugins/codex/scripts/lib/state.mjs
+++ b/plugins/codex/scripts/lib/state.mjs
@@ -11,9 +11,12 @@ 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 QUEUED_GRACE_MS = 60000; // a pid-less queued record stuck longer than this is a crashed enqueuer
 const TERMINAL_STATUSES = new Set(["completed", "failed", "cancelled"]);
 
+export function isTerminalStatus(status) {
+  return TERMINAL_STATUSES.has(status);
+}
+
 // A job id becomes a filename, so reject anything that could escape the jobs dir.
 function isValidJobId(id) {
   return typeof id === "string" && id.length > 0 && !id.includes("/") && !id.includes("\\") && id !== "." && id !== "..";
@@ -76,6 +79,67 @@ export function resolveJobFile(cwd, jobId) {
   return path.join(resolveJobsDir(cwd), `${jobId}.json`);
 }
 
+// Cancellation is expressed as a SEPARATE, immutable marker file, never by mutating
+// the worker-owned record. `<id>.cancelled` is created atomically (O_EXCL) and never
+// overwritten, so a cancellation can never be lost to a racing record write. Readers
+// overlay it (a record with a live marker reads as cancelled) and the worker honors
+// it. This is the compare-and-swap primitive the mutable-record design lacked.
+export function resolveJobCancelFile(cwd, jobId) {
+  ensureStateDir(cwd);
+  return path.join(resolveJobsDir(cwd), `${jobId}.cancelled`);
+}
+
+// A per-session "ended" marker (also immutable, atomic-create). It closes the window
+// where a task is enqueued AFTER session cleanup's one-shot directory scan: enqueue
+// refuses, and a worker started in that window aborts, because both consult it.
+// Hash (not sanitize) the session id into the marker filename, so distinct ids like
+// "a/b" and "a-b" cannot collide onto the same marker.
+function sessionEndedBasename(sessionId) {
+  const hash = createHash("sha256").update(String(sessionId)).digest("hex").slice(0, 32);
+  return `session-${hash}.ended`;
+}
+
+export function resolveSessionEndedFile(cwd, sessionId) {
+  ensureStateDir(cwd);
+  return path.join(resolveJobsDir(cwd), sessionEndedBasename(sessionId));
+}
+
+// Create `file` atomically iff absent (wx). Returns true if we created it, false if it
+// already existed. Any other error propagates. Used for the immutable markers.
+function createMarkerFile(file, payload) {
+  try {
+    fs.writeFileSync(file, `${JSON.stringify({ ...payload, at: nowIso() }, null, 2)}\n`, { encoding: "utf8", flag: "wx" });
+    return true;
+  } catch (err) {
+    if (err.code === "EEXIST") return false; // already marked; idempotent
+    throw err;
+  }
+}
+
+export function markJobCancelled(cwd, jobId, reason) {
+  if (!isValidJobId(jobId)) return false;
+  return createMarkerFile(resolveJobCancelFile(cwd, jobId), { reason: reason ?? "Cancelled." });
+}
+
+export function isJobCancelled(cwd, jobId) {
+  if (!isValidJobId(jobId)) return false;
+  return fs.existsSync(resolveJobCancelFile(cwd, jobId));
+}
+
+export function markSessionEnded(cwd, sessionId) {
+  if (!sessionId) return false;
+  return createMarkerFile(resolveSessionEndedFile(cwd, sessionId), { sessionId: String(sessionId) });
+}
+
+export function isSessionEnded(cwd, sessionId) {
+  if (!sessionId) return false;
+  try {
+    return fs.existsSync(resolveSessionEndedFile(cwd, sessionId));
+  } catch {
+    return false;
+  }
+}
+
 export function generateJobId(prefix = "job") {
   const random = Math.random().toString(36).slice(2, 8);
   return `${prefix}-${Date.now().toString(36)}-${random}`;
@@ -103,6 +167,40 @@ function atomicWriteJson(file, value) {
   }
 }
 
+// Atomically create `file` iff it does not already exist, and return true; return false
+// if it exists. Uses temp + hardlink (link fails with EEXIST if the target exists) so the
+// published file is never torn. This is the single-writer CLAIM primitive: the first
+// caller to create a job record owns it; a second caller (a duplicate id, a double worker
+// launch, a racing migrator) gets false and must not proceed as owner.
+function createJsonExclusive(file, value) {
+  const tmp = uniqueTmp(file);
+  try {
+    fs.writeFileSync(tmp, `${JSON.stringify(value, null, 2)}\n`, "utf8");
+    try {
+      fs.linkSync(tmp, file);
+      return true;
+    } catch (err) {
+      if (err.code === "EEXIST") return false;
+      throw err;
+    }
+  } finally {
+    try { fs.unlinkSync(tmp); } catch {}
+  }
+}
+
+// Claim job <id>'s record for the first time. Returns true on success, false if the
+// record already exists (another enqueuer/worker owns it -- caller must not proceed).
+export function claimJobRecord(cwd, jobId, payload) {
+  ensureStateDir(cwd);
+  const now = nowIso();
+  return createJsonExclusive(resolveJobFile(cwd, jobId), {
+    ...payload,
+    id: jobId,
+    createdAt: payload.createdAt ?? now,
+    updatedAt: now
+  });
+}
+
 // ---------------------------------------------------------------------------
 // Liveness (used only to keep pruning from evicting a live job)
 // ---------------------------------------------------------------------------
@@ -118,12 +216,13 @@ function pidAlive(pid) {
 }
 
 // ---------------------------------------------------------------------------
-// Legacy migration: older installs kept a jobs[] index array in state.json, with
-// some fields (startedAt, completedAt, summary, ...) living only in that index
-// and others only in the jobs/<id>.json payload. Fold each index entry INTO its
-// per-job file so the single record has both, then rewrite state.json config-only.
-// The merge is additive: an existing payload wins on conflicting keys (it is the
-// authoritative/newer record) and the index only fills in keys the payload lacks.
+// Legacy migration: older installs kept a jobs[] index array in state.json. Materialize
+// each index entry as its per-job file, then rewrite state.json config-only. The write
+// is CREATE-EXCLUSIVE: if a per-job file already exists it is the authoritative record
+// and migration must never overwrite it -- otherwise migration would be a second writer
+// racing a live worker (a stale index entry could clobber a freshly published `running`
+// or `completed`). Since cleanup now migrates too, this can run in a hook process
+// concurrently with workers, so create-only is required, not merely tidy.
 // Idempotent; after the first run state.json has no jobs array and this no-ops.
 // ---------------------------------------------------------------------------
 
@@ -141,8 +240,7 @@ function migrateLegacyState(cwd) {
   ensureStateDir(cwd);
   for (const job of parsed.jobs) {
     if (!job || !isValidJobId(job.id)) continue;
-    const existing = readJobRecord(cwd, job.id);
-    atomicWriteJson(resolveJobFile(cwd, job.id), { ...job, ...(existing ?? {}) });
+    createJsonExclusive(resolveJobFile(cwd, job.id), job); // never overwrite a live record
   }
   try {
     atomicWriteJson(stateFile, {
@@ -196,15 +294,50 @@ function readJobRecord(cwd, jobId) {
   }
 }
 
-// Read every job record. Robust to concurrent create/delete/rename: a name that
-// vanished mid-scan (ENOENT) or a record captured mid-write (never happens with
-// atomic rename, but parse-guarded anyway) is simply skipped.
-function readAllJobs(cwd) {
+// The RAW (non-overlaid) pid currently on the record, or null. A canceller reads this
+// AFTER creating the cancel marker (never before), so the marker/pid handshake holds:
+// if the worker had already published its pid, we see it here and kill it; if not, the
+// worker will see our marker on its post-pid re-check and self-abort.
+export function readJobPid(cwd, jobId) {
+  const pid = readJobRecord(cwd, jobId)?.pid;
+  return Number.isInteger(pid) && pid > 0 ? pid : null;
+}
+
+// Overlay the immutable markers onto a record so a caller observes a cancellation
+// atomically with the record and can never see a cancelled job as live:
+//  - a `<id>.cancelled` marker forces cancelled, authoritatively ("marked => cancelled,
+//    full stop", overriding even a raced completion);
+//  - a `session-<hash>.ended` marker forces cancelled for that session's still-LIVE
+//    (queued/running) jobs -- a genuinely finished job keeps its terminal outcome.
+// Completion payload (result/rendered) is dropped so a cancelled job never exposes a
+// half-result. Side-effect free.
+function overlayJob(job, cancelledIds, endedNames) {
+  if (!job || typeof job.id !== "string" || job.status === "cancelled") return job;
+  const byCancel = cancelledIds.has(job.id);
+  const bySession =
+    !byCancel && job.sessionId != null &&
+    endedNames.has(sessionEndedBasename(job.sessionId)) &&
+    !TERMINAL_STATUSES.has(job.status);
+  if (!byCancel && !bySession) return job;
+  const { result, rendered, ...rest } = job;
+  return { ...rest, status: "cancelled", phase: "cancelled", pid: null };
+}
+
+// Scan the jobs dir ONCE: return the raw records plus the marker sets. Robust to
+// concurrent create/delete/rename: a name that vanished mid-scan (ENOENT) or a record
+// captured mid-write (parse-guarded) is skipped.
+function scanJobsDir(cwd) {
   let names;
   try {
     names = fs.readdirSync(resolveJobsDir(cwd));
   } catch {
-    return []; // jobs dir not created yet
+    return { jobs: [], cancelledIds: new Set(), endedNames: new Set() };
+  }
+  const cancelledIds = new Set();
+  const endedNames = new Set();
+  for (const name of names) {
+    if (name.endsWith(".cancelled")) cancelledIds.add(name.slice(0, -".cancelled".length));
+    else if (name.startsWith("session-") && name.endsWith(".ended")) endedNames.add(name);
   }
   const jobs = [];
   const dir = resolveJobsDir(cwd);
@@ -216,27 +349,59 @@ function readAllJobs(cwd) {
       // deleted mid-scan or unparseable -> skip
     }
   }
-  return jobs;
+  return { jobs, cancelledIds, endedNames };
+}
+
+// The marker-overlaid view -- what every consumer (status/cancel/result/session cleanup)
+// should see.
+function readAllJobs(cwd) {
+  const { jobs, cancelledIds, endedNames } = scanJobsDir(cwd);
+  return jobs.map((job) => overlayJob(job, cancelledIds, endedNames));
 }
 
-// A record is reclaimable only if it is terminal, or a non-terminal job whose
-// owner is provably gone. A just-queued job (no pid yet) is PENDING, not dead, so
-// it must not be evicted -- except as a liveness backstop if it has been stuck far
-// longer than a normal queue->spawn transition (a crashed enqueuer).
+// A record is reclaimable ONLY if it is terminal, or a running job whose owner pid is
+// provably dead. A pid-less non-terminal job (queued, no pid published yet) is NEVER
+// age-evicted: a worker could still be booting -- even paused for a long time by machine
+// sleep, load, or a debugger -- and prune's re-check + unlink is a non-atomic
+// check-then-act, so age-evicting such a record races the worker's `running` publish and
+// (worse) can strand its cancel tombstone. Leaving an abandoned queued record until it
+// turns terminal or its worker publishes+dies is the safe choice (a rare, bounded leak).
 function isEvictable(job) {
   if (typeof job.id !== "string") return false;
   if (TERMINAL_STATUSES.has(job.status)) return true;
   if (Number.isInteger(job.pid) && job.pid > 0) return !pidAlive(job.pid);
-  return Date.now() - Date.parse(job.updatedAt ?? "") > QUEUED_GRACE_MS;
+  return false; // pid-less non-terminal -> a worker may still be booting; never age-evict
+}
+
+// Remove a job's record, its cancel marker, and both its recorded and conventional
+// logs. Returns true if the record was removed (or was already gone).
+export function deleteJobFiles(cwd, job) {
+  let gone = false;
+  try {
+    fs.unlinkSync(resolveJobFile(cwd, job.id));
+    gone = true;
+  } catch (err) {
+    gone = err.code === "ENOENT"; // already removed by another actor -> counts
+  }
+  if (!gone) return false; // couldn't remove (e.g. EACCES)
+  try { fs.unlinkSync(resolveJobCancelFile(cwd, job.id)); } catch {}
+  if (typeof job.logFile === "string") { try { fs.unlinkSync(job.logFile); } catch {} }
+  try { fs.unlinkSync(resolveJobLogFile(cwd, job.id)); } catch {}
+  return true;
 }
 
 // Keep the newest MAX_JOBS records; evict the oldest reclaimable ones so a live
 // queued/running job is never made undiscoverable (which would break
-// status/cancel/session cleanup). Deletes the payload, its recorded log, and the
-// conventional log. Idempotent under concurrency; re-checks each candidate right
-// before deleting so a job that transitioned to live since the scan is spared.
+// status/cancel/session cleanup). Eviction is decided on the RAW record, NOT the cancel
+// overlay: a job that merely carries a cancel marker but whose raw record is still
+// queued/running is NOT evictable while a worker could still be booting (a pid-less
+// queued record only ages out after QUEUED_GRACE_MS; a booting worker boots in seconds
+// and honors the marker long before then). Deleting the record+marker of such a job
+// would let the booting worker re-create it unmarked -- the resurrection this avoids.
+// Session-ended markers are intentionally NOT GC'd here: a safe generation-aware sweep
+// is out of scope, and one tiny empty file per session is a negligible, race-free leak.
 function pruneJobs(cwd) {
-  const jobs = readAllJobs(cwd);
+  const jobs = scanJobsDir(cwd).jobs; // RAW records
   const overflow = jobs.length - MAX_JOBS;
   if (overflow <= 0) return;
   const evictable = jobs
@@ -245,19 +410,9 @@ function pruneJobs(cwd) {
   let removed = 0;
   for (const job of evictable) {
     if (removed >= overflow) break;
-    const current = readJobRecord(cwd, job.id);
-    if (current && !isEvictable(current)) continue; // became live/pending since scan
-    let gone = false;
-    try {
-      fs.unlinkSync(resolveJobFile(cwd, job.id));
-      gone = true;
-    } catch (err) {
-      gone = err.code === "ENOENT"; // already removed by another pruner -> counts
-    }
-    if (!gone) continue; // couldn't remove (e.g. EACCES) -> don't count against overflow
-    if (typeof job.logFile === "string") { try { fs.unlinkSync(job.logFile); } catch {} }
-    try { fs.unlinkSync(resolveJobLogFile(cwd, job.id)); } catch {}
-    removed += 1;
+    const current = readJobRecord(cwd, job.id); // RAW re-check
+    if (current && !isEvictable(current)) continue; // became live/pending since scan -> spare
+    if (deleteJobFiles(cwd, job)) removed += 1;
   }
 }
 
@@ -265,6 +420,12 @@ function pruneJobs(cwd) {
 // single write path for both a full record and an incremental patch; merging
 // (rather than overwriting) means a field written by one call site is never lost
 // by a later call that omits it (e.g. a `summary` added after the payload write).
+//
+// The record has a SINGLE writer -- the worker owns queued->running->terminal (the
+// enqueue's pre-spawn queued write happens-before the worker exists). Cancellation
+// does NOT write here; it uses the immutable `<id>.cancelled` marker instead. So there
+// is no concurrent read-modify-write on this file and a plain additive merge is safe:
+// no cross-process lost update is possible. Returns the published record.
 function mergeJobRecord(cwd, jobId, patch) {
   ensureStateDir(cwd);
   const existing = readJobRecord(cwd, jobId) ?? {};
@@ -298,3 +459,12 @@ export function listJobs(cwd) {
   migrateLegacyState(cwd);
   return readAllJobs(cwd);
 }
+
+// The RAW records (no marker overlay), but still migrated from any legacy state.json
+// jobs[] index so a fresh install and an upgraded one look the same. Session cleanup
+// needs this: having just written the session-ended marker, the overlaid view would
+// hide the very jobs it must kill.
+export function readAllJobsRaw(cwd) {
+  migrateLegacyState(cwd);
+  return scanJobsDir(cwd).jobs;
+}
diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs
index 383d97002..5564120d8 100644
--- a/plugins/codex/scripts/lib/tracked-jobs.mjs
+++ b/plugins/codex/scripts/lib/tracked-jobs.mjs
@@ -1,7 +1,7 @@
 import fs from "node:fs";
 import process from "node:process";
 
-import { readJobFile, resolveJobFile, resolveJobLogFile, upsertJob, writeJobFile } from "./state.mjs";
+import { isJobCancelled, isSessionEnded, readJobFile, resolveJobFile, resolveJobLogFile, upsertJob } from "./state.mjs";
 
 export const SESSION_ID_ENV = "CODEX_COMPANION_SESSION_ID";
 
@@ -143,41 +143,48 @@ export async function runTrackedJob(job, runner, options = {}) {
     pid: process.pid,
     logFile: options.logFile ?? job.logFile ?? null
   };
-  writeJobFile(job.workspaceRoot, job.id, runningRecord);
+  // Cancel handshake (the linearizable half). We PUBLISH our pid first, THEN re-check
+  // the cancel marker. Cancellation does the mirror: create the marker first, THEN read
+  // the pid. Because each side writes its flag before reading the other's, at least one
+  // side always observes the other -- so either the canceller finds our pid and kills
+  // us, or we find the marker here and abort. No orphan worker, no lost cancellation.
   upsertJob(job.workspaceRoot, runningRecord);
+  if (isJobCancelled(job.workspaceRoot, job.id) || isSessionEnded(job.workspaceRoot, job.sessionId)) {
+    appendLogLine(options.logFile ?? job.logFile ?? null, "Job cancelled before start; not starting.");
+    upsertJob(job.workspaceRoot, { id: job.id, status: "cancelled", phase: "cancelled", pid: null, completedAt: nowIso() });
+    return { aborted: true, status: "cancelled" };
+  }
 
   try {
     const execution = await runner();
     const completionStatus = execution.exitStatus === 0 ? "completed" : "failed";
     const completedAt = nowIso();
-    writeJobFile(job.workspaceRoot, job.id, {
+    upsertJob(job.workspaceRoot, {
       ...runningRecord,
       status: completionStatus,
       threadId: execution.threadId ?? null,
       turnId: execution.turnId ?? null,
+      summary: execution.summary,
       pid: null,
       phase: completionStatus === "completed" ? "done" : "failed",
       completedAt,
       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);
+    // Guard the final log append: the terminal record is already published, so a logging
+    // failure must NOT fall into the catch below and write a second (failed) lifecycle
+    // record over it. The terminal upsertJob above is the worker's last record write.
+    try {
+      appendLogBlock(options.logFile ?? job.logFile ?? null, "Final output", execution.rendered);
+    } catch {
+      // logging is best-effort; never revert a published terminal state
+    }
     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, {
+    upsertJob(job.workspaceRoot, {
       ...existing,
       status: "failed",
       phase: "failed",
@@ -186,14 +193,6 @@ export async function runTrackedJob(job, runner, options = {}) {
       completedAt,
       logFile: options.logFile ?? job.logFile ?? existing.logFile ?? null
     });
-    upsertJob(job.workspaceRoot, {
-      id: job.id,
-      status: "failed",
-      phase: "failed",
-      pid: null,
-      errorMessage,
-      completedAt
-    });
     throw error;
   }
 }
diff --git a/plugins/codex/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs
index 847bdf8de..fb154d90b 100644
--- a/plugins/codex/scripts/session-lifecycle-hook.mjs
+++ b/plugins/codex/scripts/session-lifecycle-hook.mjs
@@ -1,7 +1,9 @@
 #!/usr/bin/env node
 
+import { spawnSync } from "node:child_process";
 import fs from "node:fs";
 import process from "node:process";
+import { fileURLToPath } from "node:url";
 
 import { terminateProcessTree } from "./lib/process.mjs";
 import { BROKER_ENDPOINT_ENV } from "./lib/app-server.mjs";
@@ -14,9 +16,11 @@ import {
   teardownBrokerSession
 } from "./lib/broker-lifecycle.mjs";
 import {
-  listJobs,
-  resolveJobFile,
-  resolveJobLogFile,
+  deleteJobFiles,
+  markJobCancelled,
+  markSessionEnded,
+  readAllJobsRaw,
+  readJobPid,
   resolveJobsDir,
   resolveStateFile
 } from "./lib/state.mjs";
@@ -54,6 +58,22 @@ function pidAlive(pid) {
   }
 }
 
+// A pid that is still RUNNABLE (can execute code), as opposed to a zombie/defunct that
+// `kill(pid, 0)` still reports as existing but which can never resurrect a record. Used
+// only to decide whether to DELETE a job's record. It must FAIL SAFE: any uncertainty
+// returns true (keep the record+tombstone), because wrongly classifying a live worker as
+// non-runnable would delete a tombstone the worker could still overwrite. Only a
+// conclusive `ps` result reporting a zombie state returns false.
+function pidRunnable(pid) {
+  if (!pidAlive(pid)) return false; // ESRCH => definitely gone
+  const out = spawnSync("ps", ["-o", "state=", "-p", String(pid)], { encoding: "utf8" });
+  // Treat launch failure / non-zero exit / no output / read error as UNCERTAIN -> runnable.
+  if (out.error || out.status !== 0) return true;
+  const state = (out.stdout ?? "").trim();
+  if (state === "") return true; // ambiguous (some ps print nothing transiently) -> keep
+  return state[0].toUpperCase() !== "Z"; // conclusive Z/Z+ zombie -> not runnable
+}
+
 // Block (bounded) until every pid has exited, or capMs elapses. Workers install no
 // SIGTERM handler so they normally die within a few ms; the cap prevents a hang.
 function waitForExit(pids, capMs) {
@@ -67,7 +87,7 @@ function waitForExit(pids, capMs) {
   }
 }
 
-function cleanupSessionJobs(cwd, sessionId) {
+export function cleanupSessionJobs(cwd, sessionId) {
   if (!cwd || !sessionId) {
     return;
   }
@@ -77,32 +97,53 @@ function cleanupSessionJobs(cwd, sessionId) {
     return;
   }
 
-  // Job state is one file per job with no shared lock, so cleanup needs no locked
-  // read-modify-write. Terminate this session's live workers FIRST, wait briefly
-  // for them to actually exit, THEN delete their records -- so a still-dying worker
-  // cannot re-create a record we just removed.
-  const isRunning = (job) => job.status === "queued" || job.status === "running";
+  // 1. Publish the session-ended marker FIRST, BEFORE scanning. This is what makes the
+  // scan race-free against a task being enqueued concurrently: if a worker slips past
+  // its own marker checks (reads them before this marker exists), then it must have
+  // published its pid before this marker, hence before the scan below -- so the scan
+  // sees its record and kills it. Either the worker honors the marker, or we find and
+  // kill it. It also refuses any later enqueue for this session. This marker is
+  // load-bearing: if it cannot be written (e.g. ENOSPC/EACCES), surface the failure
+  // loudly rather than silently proceeding as if the session were cleanly closed. We
+  // still run the per-job kill below so live workers are stopped regardless.
+  try {
+    markSessionEnded(workspaceRoot, sessionId);
+  } catch (err) {
+    process.stderr.write(`codex: failed to publish session-ended marker for ${sessionId}: ${err instanceof Error ? err.message : String(err)}\n`);
+  }
+
+  // 2. Scan the RAW records (not the overlaid view -- the marker we just wrote would
+  // otherwise hide this session's active jobs from us).
+  const isActive = (job) => job.status === "queued" || job.status === "running";
   let jobs = [];
   try {
-    jobs = listJobs(workspaceRoot).filter((job) => job.sessionId === sessionId);
+    jobs = readAllJobsRaw(workspaceRoot).filter((job) => typeof job.id === "string" && job.sessionId === sessionId);
   } catch {
     return;
   }
+  const active = jobs.filter(isActive);
 
-  const pids = [];
-  for (const job of jobs) {
-    if (!isRunning(job)) continue;
-    const pid = job.pid;
-    if (Number.isInteger(pid) && pid > 0) pids.push(pid);
+  // 3. Publish a per-job cancel marker for each active job BEFORE reading its pid, so a
+  // worker that publishes its pid concurrently sees the marker on its post-pid re-check
+  // and self-aborts (the pid-less startup window is safe: no pid to kill, marker stands).
+  for (const job of active) {
+    try { markJobCancelled(workspaceRoot, job.id, "Session ended."); } catch {}
+  }
+
+  // 4. Only NOW read each active job's pid (raw, AFTER its marker exists) and terminate
+  // it, escalating to SIGKILL. Reading the pid after the marker is what makes the
+  // handshake hold: a worker that publishes its pid after this read still sees the marker
+  // on its own re-check and self-aborts.
+  const pidByJob = new Map(active.map((job) => [job.id, readJobPid(workspaceRoot, job.id)]));
+  const pids = [...pidByJob.values()].filter((pid) => pid != null);
+  for (const pid of pids) {
     try {
-      terminateProcessTree(pid ?? Number.NaN);
+      terminateProcessTree(pid);
     } catch {
       // Ignore teardown failures during session shutdown.
     }
   }
   waitForExit(pids, 2000);
-  // Escalate to SIGKILL for any worker that ignored SIGTERM, so it cannot survive
-  // to re-create a record we are about to delete, then give it a moment to die.
   const survivors = pids.filter(pidAlive);
   for (const pid of survivors) {
     try { process.kill(pid, "SIGKILL"); } catch {}
@@ -111,15 +152,22 @@ function cleanupSessionJobs(cwd, sessionId) {
     waitForExit(survivors, 500);
   }
 
+  // 5. Delete records, EXCEPT those we cannot prove are worker-free: an active job for
+  // which we found NO pid (a worker may still be booting), or one whose pid is STILL
+  // alive after SIGKILL (an unkillable/stuck worker). Those are left as record + marker
+  // (overlay => cancelled; prune GCs once the raw record ages out or turns terminal);
+  // the booting worker honors the marker and aborts. Everything else -- a job we killed
+  // and confirmed gone, or an already-finished job -- is safe to remove; deleteJobFiles
+  // removes the record, its cancel marker, and its logs together.
+  const stillRunnable = new Set(pids.filter(pidRunnable)); // excludes zombies (dead, unreaped)
+  const keepMarker = new Set(
+    active
+      .filter((job) => pidByJob.get(job.id) == null || stillRunnable.has(pidByJob.get(job.id)))
+      .map((job) => job.id)
+  );
   for (const job of jobs) {
-    if (typeof job.id !== "string") continue;
-    try { fs.unlinkSync(resolveJobFile(workspaceRoot, job.id)); } catch {}
-    // Remove the log at its recorded path (which need not be jobs/<id>.log) as
-    // well as the conventional path, so no orphan log survives cleanup.
-    if (typeof job.logFile === "string") {
-      try { fs.unlinkSync(job.logFile); } catch {}
-    }
-    try { fs.unlinkSync(resolveJobLogFile(workspaceRoot, job.id)); } catch {}
+    if (keepMarker.has(job.id)) continue;
+    deleteJobFiles(workspaceRoot, job);
   }
 }
 
@@ -176,7 +224,20 @@ async function main() {
   }
 }
 
-main().catch((error) => {
-  process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`);
-  process.exit(1);
-});
+// Only run the hook when executed directly (node session-lifecycle-hook.mjs <event>);
+// importing the module (e.g. from tests) must not read stdin or run a lifecycle event.
+function isDirectRun() {
+  if (!process.argv[1]) return false;
+  try {
+    return fs.realpathSync(process.argv[1]) === fs.realpathSync(fileURLToPath(import.meta.url));
+  } catch {
+    return false;
+  }
+}
+
+if (isDirectRun()) {
+  main().catch((error) => {
+    process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`);
+    process.exit(1);
+  });
+}
diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs
index a56e9ab90..997d1ae0f 100644
--- a/tests/runtime.test.mjs
+++ b/tests/runtime.test.mjs
@@ -8,7 +8,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 { resolveStateDir } from "../plugins/codex/scripts/lib/state.mjs";
+import { listJobs, resolveStateDir } from "../plugins/codex/scripts/lib/state.mjs";
 
 // Jobs are stored one file per job under <stateDir>/jobs/<id>.json (no shared
 // index array). Read them back newest-first, the way the CLI lists them.
@@ -1444,6 +1444,7 @@ test("result without a job id prefers the latest finished job from the current C
         id: "review-current",
         status: "completed",
         title: "Codex Review",
+        sessionId: "sess-current",
         threadId: "thr_current",
         result: {
           codex: {
@@ -1464,6 +1465,7 @@ test("result without a job id prefers the latest finished job from the current C
         id: "review-other",
         status: "completed",
         title: "Codex Review",
+        sessionId: "sess-other",
         threadId: "thr_other",
         result: {
           codex: {
@@ -1581,22 +1583,10 @@ test("cancel stops an active background job and marks it cancelled", async (t) =
   });
 
   const logFile = path.join(jobsDir, "task-live.log");
-  const jobFile = path.join(jobsDir, "task-live.json");
   fs.writeFileSync(logFile, "[2026-03-18T15:30:00.000Z] Starting Codex Task.\n", "utf8");
-  fs.writeFileSync(
-    jobFile,
-    JSON.stringify(
-      {
-        id: "task-live",
-        status: "running",
-        title: "Codex Task",
-        logFile
-      },
-      null,
-      2
-    ),
-    "utf8"
-  );
+  // The task-live.json record (with its pid) is materialized from the legacy state.json
+  // jobs[] index by migration; do NOT pre-write a bare file, which create-exclusive
+  // migration would skip, leaving the record without its pid.
   fs.writeFileSync(
     path.join(stateDir, "state.json"),
     `${JSON.stringify(
@@ -1640,13 +1630,13 @@ test("cancel stops an active background job and marks it cancelled", async (t) =
     }
   });
 
-  const state = { jobs: jobsFromStateDir(stateDir) };
-  const cancelled = state.jobs.find((job) => job.id === "task-live");
+  // Cancellation is recorded via an immutable marker + read overlay (not a racy record
+  // write), so the observable state (what listJobs returns) is cancelled and the marker
+  // file exists.
+  const cancelled = listJobs(workspace).find((job) => job.id === "task-live");
   assert.equal(cancelled.status, "cancelled");
   assert.equal(cancelled.pid, null);
-
-  const stored = JSON.parse(fs.readFileSync(jobFile, "utf8"));
-  assert.equal(stored.status, "cancelled");
+  assert.equal(fs.existsSync(path.join(jobsDir, "task-live.cancelled")), true, "cancel marker published");
   assert.match(fs.readFileSync(logFile, "utf8"), /Cancelled by user/);
 });
 
@@ -1749,8 +1739,7 @@ test("cancel with a job id can still target an active job from another Claude se
   assert.equal(cancel.status, 0, cancel.stderr);
   assert.equal(JSON.parse(cancel.stdout).jobId, "task-other");
 
-  const state = { jobs: jobsFromStateDir(stateDir) };
-  assert.equal(state.jobs[0].status, "cancelled");
+  assert.equal(listJobs(workspace).find((job) => job.id === "task-other").status, "cancelled");
 });
 
 test("cancel sends turn interrupt to the shared app-server before killing a brokered task", async () => {
@@ -1837,8 +1826,9 @@ test("session end fully cleans up jobs for the ending session", async (t) => {
   fs.writeFileSync(completedLog, "completed\n", "utf8");
   fs.writeFileSync(runningLog, "running\n", "utf8");
   fs.writeFileSync(otherSessionLog, "other\n", "utf8");
-  fs.writeFileSync(completedJobFile, JSON.stringify({ id: "review-completed" }, null, 2), "utf8");
-  fs.writeFileSync(otherJobFile, JSON.stringify({ id: "review-other" }, null, 2), "utf8");
+  // The per-job .json records are materialized from the legacy state.json jobs[] index
+  // below by migration; we deliberately do NOT pre-write bare {id} files, because
+  // migration is now create-exclusive and would skip (not enrich) an existing file.
 
   const sleeper = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], {
     cwd: repo,
@@ -1846,7 +1836,6 @@ test("session end fully cleans up jobs for the ending session", async (t) => {
     stdio: "ignore"
   });
   sleeper.unref();
-  fs.writeFileSync(runningJobFile, JSON.stringify({ id: "review-running" }, null, 2), "utf8");
 
   t.after(() => {
     try {
@@ -1919,8 +1908,14 @@ test("session end fully cleans up jobs for the ending session", async (t) => {
   assert.equal(result.status, 0, result.stderr);
   assert.equal(fs.existsSync(otherSessionLog), true);
   assert.equal(fs.existsSync(otherJobFile), true);
+  // The ending session's jobs (and their logs) are removed; the other session's job is
+  // untouched. Session end also drops an immutable `session-<hash>.ended` marker that
+  // refuses/aborts a task raced in after this cleanup scan.
+  const remaining = fs.readdirSync(path.dirname(otherJobFile)).sort();
+  const sessionMarkers = remaining.filter((n) => n.startsWith("session-") && n.endsWith(".ended"));
+  assert.equal(sessionMarkers.length, 1, "one session-ended marker was written");
   assert.deepEqual(
-    fs.readdirSync(path.dirname(otherJobFile)).sort(),
+    remaining.filter((n) => !n.startsWith("session-")),
     [path.basename(otherJobFile), path.basename(otherSessionLog)].sort()
   );
 
diff --git a/tests/session-cleanup.test.mjs b/tests/session-cleanup.test.mjs
new file mode 100644
index 000000000..0582c24a0
--- /dev/null
+++ b/tests/session-cleanup.test.mjs
@@ -0,0 +1,54 @@
+import fs from "node:fs";
+import test from "node:test";
+import assert from "node:assert/strict";
+
+import { makeTempDir } from "./helpers.mjs";
+import { isJobCancelled, listJobs, resolveJobFile, upsertJob } from "../plugins/codex/scripts/lib/state.mjs";
+import { resolveWorkspaceRoot } from "../plugins/codex/scripts/lib/workspace.mjs";
+import { cleanupSessionJobs } from "../plugins/codex/scripts/session-lifecycle-hook.mjs";
+
+// A pid that is (practically) guaranteed not to exist, so it reads as a dead worker.
+const DEAD_PID = 2 ** 31 - 1;
+
+test("cleanup marks a queued pid-less job cancelled and keeps it (a booting worker cannot resurrect it)", () => {
+  const cwd = makeTempDir();
+  const workspace = resolveWorkspaceRoot(cwd);
+  // Enqueue wrote the queued record but the worker has not published its pid yet.
+  upsertJob(workspace, { id: "q", status: "queued", phase: "queued", pid: null, sessionId: "s1" });
+
+  cleanupSessionJobs(cwd, "s1");
+
+  assert.equal(isJobCancelled(workspace, "q"), true, "an immutable cancel marker was published");
+  const job = listJobs(workspace).find((entry) => entry.id === "q");
+  assert.ok(job, "record must NOT be deleted while a worker could still be booting");
+  assert.equal(job.status, "cancelled", "the marker overlays the record as cancelled");
+
+  // A worker that already read the queued record then publishes running: the record
+  // says running, but the immutable marker overlays every read back to cancelled.
+  upsertJob(workspace, { id: "q", status: "running", pid: 12345 });
+  const after = listJobs(workspace).find((entry) => entry.id === "q");
+  assert.equal(after.status, "cancelled", "no resurrection to running: the marker wins on every read");
+  assert.equal(after.pid, null, "an overlaid-cancelled job exposes no live pid");
+});
+
+test("cleanup deletes a running job whose worker is provably gone", () => {
+  const cwd = makeTempDir();
+  const workspace = resolveWorkspaceRoot(cwd);
+  upsertJob(workspace, { id: "r", status: "running", phase: "starting", pid: DEAD_PID, sessionId: "s1" });
+
+  cleanupSessionJobs(cwd, "s1");
+
+  assert.equal(fs.existsSync(resolveJobFile(workspace, "r")), false, "a confirmed-dead worker's record is removed");
+});
+
+test("cleanup deletes an already-finished job and ignores other sessions", () => {
+  const cwd = makeTempDir();
+  const workspace = resolveWorkspaceRoot(cwd);
+  upsertJob(workspace, { id: "done", status: "completed", pid: null, sessionId: "s1" });
+  upsertJob(workspace, { id: "other", status: "running", pid: DEAD_PID, sessionId: "s2" });
+
+  cleanupSessionJobs(cwd, "s1");
+
+  assert.equal(fs.existsSync(resolveJobFile(workspace, "done")), false, "finished job of this session is removed");
+  assert.ok(listJobs(workspace).find((entry) => entry.id === "other"), "another session's job is untouched");
+});
diff --git a/tests/state.test.mjs b/tests/state.test.mjs
index 3df2b8b13..a2fe02165 100644
--- a/tests/state.test.mjs
+++ b/tests/state.test.mjs
@@ -10,7 +10,12 @@ import { fileURLToPath, pathToFileURL } from "node:url";
 import { makeTempDir } from "./helpers.mjs";
 import {
   ensureStateDir,
+  isJobCancelled,
+  isSessionEnded,
   listJobs,
+  markJobCancelled,
+  markSessionEnded,
+  readJobPid,
   resolveJobFile,
   resolveJobLogFile,
   resolveStateDir,
@@ -151,3 +156,79 @@ test("legacy state.json jobs[] array migrates to per-job files on read", () => {
   assert.equal(Array.isArray(rewritten.jobs), false);
   assert.equal(rewritten.config.stopReviewGate, true);
 });
+
+test("a no-status patch (e.g. a progress update) leaves status untouched", () => {
+  const workspace = makeTempDir();
+  upsertJob(workspace, { id: "j", status: "running", pid: 42 });
+  upsertJob(workspace, { id: "j", phase: "thinking" });
+
+  const job = listJobs(workspace).find((entry) => entry.id === "j");
+  assert.equal(job.status, "running");
+  assert.equal(job.phase, "thinking");
+});
+
+test("a cancel marker overlays as cancelled and cannot be resurrected by a later running write", () => {
+  const workspace = makeTempDir();
+  upsertJob(workspace, { id: "j", status: "queued", pid: null });
+  assert.equal(markJobCancelled(workspace, "j", "test"), true, "marker created");
+
+  // A racing worker publishes running AFTER the marker (the record itself says running):
+  upsertJob(workspace, { id: "j", status: "running", pid: 99, phase: "starting" });
+
+  const job = listJobs(workspace).find((entry) => entry.id === "j");
+  assert.equal(job.status, "cancelled", "the immutable marker overlays the record");
+  assert.equal(job.pid, null, "an overlaid-cancelled job exposes no live pid");
+  assert.equal(isJobCancelled(workspace, "j"), true);
+});
+
+test("the cancel marker is immutable: a second mark is a no-op, not an error", () => {
+  const workspace = makeTempDir();
+  upsertJob(workspace, { id: "j", status: "running", pid: 1 });
+  assert.equal(markJobCancelled(workspace, "j", "first"), true);
+  assert.equal(markJobCancelled(workspace, "j", "second"), false, "already marked -> false, no throw");
+});
+
+test("a completion that lands after a cancel marker still reads as cancelled", () => {
+  const workspace = makeTempDir();
+  upsertJob(workspace, { id: "j", status: "running", pid: 5 });
+  markJobCancelled(workspace, "j", "cancelled by user");
+  upsertJob(workspace, { id: "j", status: "completed", summary: "done" });
+
+  const job = listJobs(workspace).find((entry) => entry.id === "j");
+  assert.equal(job.status, "cancelled", "cancellation wins over a later completion via the overlay");
+});
+
+test("normal forward transitions still advance (queued -> running -> completed)", () => {
+  const workspace = makeTempDir();
+  upsertJob(workspace, { id: "j", status: "queued", pid: null });
+  upsertJob(workspace, { id: "j", status: "running", pid: 7 });
+  const done = upsertJob(workspace, { id: "j", status: "completed" });
+  assert.equal(done.status, "completed");
+});
+
+test("a no-status progress patch advances phase without touching status", () => {
+  const workspace = makeTempDir();
+  upsertJob(workspace, { id: "j", status: "running", pid: 7, phase: "starting" });
+  const after = upsertJob(workspace, { id: "j", phase: "thinking" });
+  assert.equal(after.status, "running");
+  assert.equal(after.phase, "thinking", "phase update must not be dropped");
+});
+
+test("session-ended marker is create-once and observable", () => {
+  const workspace = makeTempDir();
+  assert.equal(isSessionEnded(workspace, "s1"), false);
+  assert.equal(markSessionEnded(workspace, "s1"), true, "first mark creates it");
+  assert.equal(markSessionEnded(workspace, "s1"), false, "second mark is a no-op");
+  assert.equal(isSessionEnded(workspace, "s1"), true);
+  assert.equal(isSessionEnded(workspace, "s2"), false, "unrelated session is unaffected");
+});
+
+test("readJobPid returns the raw record pid (not overlaid) so a canceller can kill after marking", () => {
+  const workspace = makeTempDir();
+  upsertJob(workspace, { id: "j", status: "running", pid: 4321 });
+  assert.equal(readJobPid(workspace, "j"), 4321);
+  // After marking, listJobs overlays pid:null, but the RAW pid is still readable to kill.
+  markJobCancelled(workspace, "j", "test");
+  assert.equal(readJobPid(workspace, "j"), 4321, "raw pid survives the overlay");
+  assert.equal(listJobs(workspace).find((e) => e.id === "j").pid, null, "overlay hides the pid from readers");
+});

From c8bb48671e35a729c8c8101d902f8f0108234f20 Mon Sep 17 00:00:00 2001
From: rajasekar-venkatesan
 <22398308+rajasekar-venkatesan@users.noreply.github.com>
Date: Wed, 26 Aug 2026 19:14:48 +0800
Subject: [PATCH 8/9] Address round-9 review: legacy-metadata merge,
 session-marker ordering, cancellation hardening

Bot findings:
- P1 migrateLegacyState: fold legacy state.json index-only metadata (summary,
  threadId, timing) into an existing per-job record instead of discarding it, but
  only when the RAW record has no live worker (isEvictable); create-exclusive for a
  missing record, leave a live/booting record untouched.
- P2 cleanupSessionJobs: publish the session-ended marker BEFORE any existsSync
  check or scan, so a SessionEnd racing the first launch can't skip it.

Fresh full-scan hardening:
- /codex:result renders from the overlaid record, not a raw read, so a
  completed-then-cancelled job can't leak its result/rendered.
- overlayJob surfaces the cancel marker's reason/at as errorMessage/cancelledAt/
  completedAt (best-effort; marker existence stays authoritative), still stripping
  the completion payload.
- handleCancel wraps terminateProcessTree; resolveCancelableJob classifies raw
  records so a marked job stays targetable only while running with a live pid (to
  retry a kill that threw) and never shadows a genuinely active job -- honoring both
  the cancel and session-ended markers.
- Session cleanup exits non-zero if a load-bearing marker (session-ended, or a kept
  job's cancel marker) cannot be written.
---
 plugins/codex/scripts/codex-companion.mjs     | 18 ++++-
 plugins/codex/scripts/lib/job-control.mjs     | 33 ++++++++-
 plugins/codex/scripts/lib/state.mjs           | 72 ++++++++++++++-----
 .../codex/scripts/session-lifecycle-hook.mjs  | 46 +++++++-----
 tests/state.test.mjs                          | 68 ++++++++++++++++++
 5 files changed, 196 insertions(+), 41 deletions(-)

diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs
index ac40d8217..600201c85 100644
--- a/plugins/codex/scripts/codex-companion.mjs
+++ b/plugins/codex/scripts/codex-companion.mjs
@@ -969,8 +969,12 @@ function handleResult(argv) {
 
   const cwd = resolveCommandCwd(options);
   const reference = positionals[0] ?? "";
-  const { workspaceRoot, job } = resolveResultJob(cwd, reference);
-  const storedJob = readStoredJob(workspaceRoot, job.id);
+  const { job } = resolveResultJob(cwd, reference);
+  // Render from the OVERLAID record (resolveResultJob returns it via listJobs), NOT a raw
+  // readStoredJob: a cancel marker strips result/rendered, so a job that completed and was
+  // then cancel-marked must not leak its output here. The overlay is the single source of
+  // truth for what a cancelled job exposes.
+  const storedJob = job;
   const payload = {
     job,
     storedJob
@@ -1053,7 +1057,15 @@ async function handleCancel(argv) {
   // stale pid risks signalling an unrelated process that reused it. If it's null, the
   // worker will honor the marker on its own post-pid re-check.
   const killPid = readJobPid(workspaceRoot, job.id);
-  terminateProcessTree(killPid ?? Number.NaN);
+  // Best-effort: a termination failure must NOT abort the command and leave the job
+  // marked-cancelled-but-live with no way to retry. resolveCancelableJob works off the RAW
+  // record, so re-running cancel still finds a marked job whose worker is raw-queued/running
+  // and re-attempts the kill.
+  try {
+    terminateProcessTree(killPid ?? Number.NaN);
+  } catch (error) {
+    appendLogLine(job.logFile, `Termination failed: ${error instanceof Error ? error.message : String(error)}`);
+  }
   appendLogLine(job.logFile, "Cancelled by user.");
 
   const completedAt = nowIso();
diff --git a/plugins/codex/scripts/lib/job-control.mjs b/plugins/codex/scripts/lib/job-control.mjs
index 4ab08ff02..521f1fd72 100644
--- a/plugins/codex/scripts/lib/job-control.mjs
+++ b/plugins/codex/scripts/lib/job-control.mjs
@@ -1,13 +1,24 @@
 import fs from "node:fs";
 
 import { getSessionRuntimeStatus } from "./codex.mjs";
-import { getConfig, listJobs, readJobFile, resolveJobFile } from "./state.mjs";
+import { getConfig, isJobCancelled, isSessionEnded, listJobs, readAllJobsRaw, readJobFile, resolveJobFile } from "./state.mjs";
 import { SESSION_ID_ENV } from "./tracked-jobs.mjs";
 import { resolveWorkspaceRoot } from "./workspace.mjs";
 
 export const DEFAULT_MAX_STATUS_JOBS = 8;
 export const DEFAULT_MAX_PROGRESS_LINES = 4;
 
+// True while pid exists and could still execute code (EPERM = alive but not ours).
+function isPidAlive(pid) {
+  if (!Number.isInteger(pid) || pid <= 0) return false;
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch (err) {
+    return err.code === "EPERM";
+  }
+}
+
 export function sortJobsNewestFirst(jobs) {
   return [...jobs].sort((left, right) => String(right.updatedAt ?? "").localeCompare(String(left.updatedAt ?? "")));
 }
@@ -282,8 +293,24 @@ export function resolveResultJob(cwd, reference) {
 
 export function resolveCancelableJob(cwd, reference, options = {}) {
   const workspaceRoot = resolveWorkspaceRoot(cwd);
-  const jobs = sortJobsNewestFirst(listJobs(workspaceRoot));
-  const activeJobs = jobs.filter((job) => job.status === "queued" || job.status === "running");
+  // Work off RAW records (so a failed-kill retry can still see the true running state),
+  // but classify cancellability precisely so a job that was already cancelled cannot
+  // shadow a genuinely active one:
+  //  - unmarked queued/running  -> cancellable (normal case);
+  //  - MARKED + running + LIVE pid -> cancellable (a previous kill threw; retry it);
+  //  - MARKED + running with a dead pid (killed, died before writing a terminal record),
+  //    or MARKED + queued/pid-less -> NOT cancellable (the marker is already authoritative
+  //    and the worker must honor it; re-selecting it would shadow other active jobs).
+  const jobs = sortJobsNewestFirst(readAllJobsRaw(workspaceRoot));
+  const activeJobs = jobs.filter((job) => {
+    if (job.status !== "queued" && job.status !== "running") return false;
+    // A job is logically cancelled by EITHER its own cancel marker OR its session's
+    // ended marker (both are authoritative in the read overlay). Only a marked job that
+    // is still running with a live pid stays targetable (to retry a kill that threw).
+    const marked = isJobCancelled(workspaceRoot, job.id) || isSessionEnded(workspaceRoot, job.sessionId);
+    if (!marked) return true;
+    return job.status === "running" && isPidAlive(job.pid);
+  });
 
   if (reference) {
     const selected = matchJobReference(activeJobs, reference);
diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs
index bf8ee5aca..3547f2005 100644
--- a/plugins/codex/scripts/lib/state.mjs
+++ b/plugins/codex/scripts/lib/state.mjs
@@ -216,13 +216,18 @@ function pidAlive(pid) {
 }
 
 // ---------------------------------------------------------------------------
-// Legacy migration: older installs kept a jobs[] index array in state.json. Materialize
-// each index entry as its per-job file, then rewrite state.json config-only. The write
-// is CREATE-EXCLUSIVE: if a per-job file already exists it is the authoritative record
-// and migration must never overwrite it -- otherwise migration would be a second writer
-// racing a live worker (a stale index entry could clobber a freshly published `running`
-// or `completed`). Since cleanup now migrates too, this can run in a hook process
-// concurrently with workers, so create-only is required, not merely tidy.
+// Legacy migration: older installs kept a jobs[] index array in state.json, with some
+// fields (startedAt, completedAt, summary, threadId, ...) living only in that index.
+// Materialize each index entry as its per-job file, then rewrite state.json config-only.
+// - No per-job file yet: create it exclusively (claim).
+// - A per-job file exists AND has no live worker (isEvictable: terminal, or dead pid):
+//   fold the index-only fields IN (payload wins on conflicts), so a finished job keeps
+//   its summary/threadId/duration. This is safe precisely because no worker can be
+//   writing that record.
+// - A per-job file exists for a LIVE/booting job (queued pid-less, or running with a
+//   live pid): leave it untouched -- a worker owns it, and legacy index metadata for an
+//   active job is stale/minimal anyway. This is what keeps migration from racing a
+//   worker (it now runs in the session hook too).
 // Idempotent; after the first run state.json has no jobs array and this no-ops.
 // ---------------------------------------------------------------------------
 
@@ -240,7 +245,19 @@ function migrateLegacyState(cwd) {
   ensureStateDir(cwd);
   for (const job of parsed.jobs) {
     if (!job || !isValidJobId(job.id)) continue;
-    createJsonExclusive(resolveJobFile(cwd, job.id), job); // never overwrite a live record
+    // `readJobRecord` is the RAW on-disk payload (NOT the cancel/session overlay), so
+    // isEvictable() classifies the record by its true status: a marked-but-still-running
+    // record still reads `running` here and is correctly treated as owned, not terminal.
+    // The evictable set (raw terminal, or raw running with an ESRCH-dead pid) has no
+    // process that can still write it -- there is no reclaimer of a dead-pid job in this
+    // design -- so folding index metadata in cannot lose a live/booting worker's write.
+    const existing = readJobRecord(cwd, job.id);
+    if (existing == null) {
+      createJsonExclusive(resolveJobFile(cwd, job.id), job);
+    } else if (isEvictable(existing)) {
+      atomicWriteJson(resolveJobFile(cwd, job.id), { ...job, ...existing }); // add index-only fields; payload wins
+    }
+    // else: a live/booting worker owns the record -> leave it alone
   }
   try {
     atomicWriteJson(stateFile, {
@@ -311,36 +328,55 @@ export function readJobPid(cwd, jobId) {
 //    (queued/running) jobs -- a genuinely finished job keeps its terminal outcome.
 // Completion payload (result/rendered) is dropped so a cancelled job never exposes a
 // half-result. Side-effect free.
-function overlayJob(job, cancelledIds, endedNames) {
+function overlayJob(job, cancelledMeta, endedNames) {
   if (!job || typeof job.id !== "string" || job.status === "cancelled") return job;
-  const byCancel = cancelledIds.has(job.id);
+  const byCancel = cancelledMeta.has(job.id);
   const bySession =
     !byCancel && job.sessionId != null &&
     endedNames.has(sessionEndedBasename(job.sessionId)) &&
     !TERMINAL_STATUSES.has(job.status);
   if (!byCancel && !bySession) return job;
+  // Surface the marker's own metadata (best-effort; marker EXISTENCE is authoritative
+  // even if its JSON was unreadable) so a cancel that killed the worker before it could
+  // publish a terminal record still shows a reason and timestamps.
+  const meta = (byCancel ? cancelledMeta.get(job.id) : null) ?? {};
+  const at = typeof meta.at === "string" ? meta.at : null;
   const { result, rendered, ...rest } = job;
-  return { ...rest, status: "cancelled", phase: "cancelled", pid: null };
+  return {
+    ...rest,
+    status: "cancelled",
+    phase: "cancelled",
+    pid: null,
+    errorMessage: rest.errorMessage ?? (typeof meta.reason === "string" ? meta.reason : rest.errorMessage),
+    cancelledAt: rest.cancelledAt ?? at ?? undefined,
+    completedAt: rest.completedAt ?? at ?? undefined
+  };
 }
 
 // Scan the jobs dir ONCE: return the raw records plus the marker sets. Robust to
 // concurrent create/delete/rename: a name that vanished mid-scan (ENOENT) or a record
-// captured mid-write (parse-guarded) is skipped.
+// captured mid-write (parse-guarded) is skipped. `cancelledIds` is a Map id -> marker
+// metadata ({reason, at}, or {} if the marker existed but was unparseable).
 function scanJobsDir(cwd) {
+  const dir = resolveJobsDir(cwd);
   let names;
   try {
-    names = fs.readdirSync(resolveJobsDir(cwd));
+    names = fs.readdirSync(dir);
   } catch {
-    return { jobs: [], cancelledIds: new Set(), endedNames: new Set() };
+    return { jobs: [], cancelledIds: new Map(), endedNames: new Set() };
   }
-  const cancelledIds = new Set();
+  const cancelledIds = new Map();
   const endedNames = new Set();
   for (const name of names) {
-    if (name.endsWith(".cancelled")) cancelledIds.add(name.slice(0, -".cancelled".length));
-    else if (name.startsWith("session-") && name.endsWith(".ended")) endedNames.add(name);
+    if (name.endsWith(".cancelled")) {
+      let meta = {};
+      try { meta = JSON.parse(fs.readFileSync(path.join(dir, name), "utf8")); } catch {}
+      cancelledIds.set(name.slice(0, -".cancelled".length), meta);
+    } else if (name.startsWith("session-") && name.endsWith(".ended")) {
+      endedNames.add(name);
+    }
   }
   const jobs = [];
-  const dir = resolveJobsDir(cwd);
   for (const name of names) {
     if (!isJobFileName(name)) continue;
     try {
diff --git a/plugins/codex/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs
index fb154d90b..18fbcdcd6 100644
--- a/plugins/codex/scripts/session-lifecycle-hook.mjs
+++ b/plugins/codex/scripts/session-lifecycle-hook.mjs
@@ -20,9 +20,7 @@ import {
   markJobCancelled,
   markSessionEnded,
   readAllJobsRaw,
-  readJobPid,
-  resolveJobsDir,
-  resolveStateFile
+  readJobPid
 } from "./lib/state.mjs";
 import { TRANSCRIPT_PATH_ENV } from "./lib/claude-session-transfer.mjs";
 import { resolveWorkspaceRoot } from "./lib/workspace.mjs";
@@ -93,22 +91,25 @@ export function cleanupSessionJobs(cwd, sessionId) {
   }
 
   const workspaceRoot = resolveWorkspaceRoot(cwd);
-  if (!fs.existsSync(resolveStateFile(workspaceRoot)) && !fs.existsSync(resolveJobsDir(workspaceRoot))) {
-    return;
-  }
-
-  // 1. Publish the session-ended marker FIRST, BEFORE scanning. This is what makes the
-  // scan race-free against a task being enqueued concurrently: if a worker slips past
-  // its own marker checks (reads them before this marker exists), then it must have
-  // published its pid before this marker, hence before the scan below -- so the scan
-  // sees its record and kills it. Either the worker honors the marker, or we find and
-  // kill it. It also refuses any later enqueue for this session. This marker is
-  // load-bearing: if it cannot be written (e.g. ENOSPC/EACCES), surface the failure
-  // loudly rather than silently proceeding as if the session were cleanly closed. We
-  // still run the per-job kill below so live workers are stopped regardless.
+  let sessionMarkerFailed = false;
+  const jobMarkerFailed = new Set();
+
+  // 1. Publish the session-ended marker FIRST -- before ANY existence check or scan.
+  // A `!existsSync(state) return` optimization here would be a correctness hole: when
+  // SessionEnd races the very FIRST background launch in a workspace, both paths can be
+  // absent at that check, so cleanup would return without writing the marker and the
+  // launcher's worker would then run after the session ended. Writing the marker first
+  // (it ensures the state dir) closes that race and makes the scan below race-free: if a
+  // worker slips past its own marker checks (reads them before this marker exists), it
+  // must have published its pid before this marker -- hence before the scan -- so the
+  // scan sees its record and kills it. Either the worker honors the marker, or we find
+  // and kill it; it also refuses any later enqueue for this session. Load-bearing: if it
+  // cannot be written (ENOSPC/EACCES), surface the failure loudly rather than silently
+  // proceeding as if the session were cleanly closed; the per-job kill below still runs.
   try {
     markSessionEnded(workspaceRoot, sessionId);
   } catch (err) {
+    sessionMarkerFailed = true;
     process.stderr.write(`codex: failed to publish session-ended marker for ${sessionId}: ${err instanceof Error ? err.message : String(err)}\n`);
   }
 
@@ -127,7 +128,7 @@ export function cleanupSessionJobs(cwd, sessionId) {
   // worker that publishes its pid concurrently sees the marker on its post-pid re-check
   // and self-aborts (the pid-less startup window is safe: no pid to kill, marker stands).
   for (const job of active) {
-    try { markJobCancelled(workspaceRoot, job.id, "Session ended."); } catch {}
+    try { markJobCancelled(workspaceRoot, job.id, "Session ended."); } catch { jobMarkerFailed.add(job.id); }
   }
 
   // 4. Only NOW read each active job's pid (raw, AFTER its marker exists) and terminate
@@ -169,6 +170,17 @@ export function cleanupSessionJobs(cwd, sessionId) {
     if (keepMarker.has(job.id)) continue;
     deleteJobFiles(workspaceRoot, job);
   }
+
+  // 6. Fail loud if a LOAD-BEARING marker could not be written. Best-effort scanning and
+  // killing above still ran, but without the session-ended marker (or the cancel marker of
+  // a job we KEPT because a worker may still be booting) we cannot claim the session was
+  // cleanly closed: a task racing enqueue, or that kept booting worker, could still run.
+  // A non-zero exit surfaces that to the hook runner rather than reporting success.
+  const keptWithFailedMarker = [...keepMarker].some((id) => jobMarkerFailed.has(id));
+  if (sessionMarkerFailed || keptWithFailedMarker) {
+    process.exitCode = 1;
+    process.stderr.write(`codex: session cleanup for ${sessionId} could not durably publish a required marker\n`);
+  }
 }
 
 function handleSessionStart(input) {
diff --git a/tests/state.test.mjs b/tests/state.test.mjs
index a2fe02165..f24ff0413 100644
--- a/tests/state.test.mjs
+++ b/tests/state.test.mjs
@@ -232,3 +232,71 @@ test("readJobPid returns the raw record pid (not overlaid) so a canceller can ki
   assert.equal(readJobPid(workspace, "j"), 4321, "raw pid survives the overlay");
   assert.equal(listJobs(workspace).find((e) => e.id === "j").pid, null, "overlay hides the pid from readers");
 });
+
+test("legacy migration folds index-only metadata into an existing FINISHED per-job file", () => {
+  const workspace = makeTempDir();
+  // A finished per-job payload already exists but lacks the index-only fields.
+  writeJobFile(workspace, "review-done", { id: "review-done", status: "completed", title: "Review" });
+  fs.writeFileSync(
+    resolveStateFile(workspace),
+    JSON.stringify({
+      version: 1,
+      config: { stopReviewGate: false },
+      jobs: [
+        {
+          id: "review-done",
+          status: "completed",
+          title: "Review",
+          summary: "found 2 issues",
+          threadId: "thr_abc",
+          startedAt: "2026-01-01T00:00:00.000Z",
+          completedAt: "2026-01-01T00:01:00.000Z"
+        }
+      ]
+    }, null, 2)
+  );
+
+  const job = listJobs(workspace).find((entry) => entry.id === "review-done");
+  assert.equal(job.status, "completed");
+  assert.equal(job.summary, "found 2 issues", "index-only summary is merged in, not lost");
+  assert.equal(job.threadId, "thr_abc", "index-only threadId is merged in");
+  assert.equal(job.startedAt, "2026-01-01T00:00:00.000Z");
+  assert.equal(job.completedAt, "2026-01-01T00:01:00.000Z");
+});
+
+test("legacy migration does NOT overwrite a LIVE (running, live-pid) per-job file", () => {
+  const workspace = makeTempDir();
+  // A running job owned by THIS (alive) process; its worker is the sole writer.
+  writeJobFile(workspace, "live", { id: "live", status: "running", pid: process.pid, phase: "thinking" });
+  fs.writeFileSync(
+    resolveStateFile(workspace),
+    JSON.stringify({
+      version: 1,
+      config: {},
+      jobs: [{ id: "live", status: "queued", phase: "queued", summary: "stale index" }]
+    }, null, 2)
+  );
+
+  const job = listJobs(workspace).find((entry) => entry.id === "live");
+  assert.equal(job.status, "running", "a live worker's record is never reverted by a stale index");
+  assert.equal(job.phase, "thinking");
+});
+
+test("cancel overlay strips result/rendered and surfaces the marker's reason + timestamp", () => {
+  const workspace = makeTempDir();
+  // A job that finished (has result/rendered) and is THEN cancel-marked.
+  upsertJob(workspace, {
+    id: "j",
+    status: "completed",
+    result: { codex: { stdout: "secret output" } },
+    rendered: "full rendered output"
+  });
+  assert.equal(markJobCancelled(workspace, "j", "Cancelled by user."), true);
+
+  const job = listJobs(workspace).find((entry) => entry.id === "j");
+  assert.equal(job.status, "cancelled");
+  assert.equal(job.result, undefined, "result payload is stripped from a cancelled job");
+  assert.equal(job.rendered, undefined, "rendered output is stripped from a cancelled job");
+  assert.equal(job.errorMessage, "Cancelled by user.", "marker reason surfaces as errorMessage");
+  assert.ok(job.cancelledAt, "marker timestamp surfaces as cancelledAt");
+});

From e73e56cbe882185269eb3083b0db9c76ecfbac33 Mon Sep 17 00:00:00 2001
From: rajasekar-venkatesan
 <22398308+rajasekar-venkatesan@users.noreply.github.com>
Date: Wed, 26 Aug 2026 20:55:02 +0800
Subject: [PATCH 9/9] Round-10 review + fresh-scan hardening: prune window,
 overlay coherence, fail-loud scans

Bot P2 (prune cap): evict only records OUTSIDE the newest-MAX_JOBS window, so a fresh
completion is never deleted when the cap is saturated by non-evictable queued jobs.

Fresh full-scan hardening (adversarial review + independent audit):
- readAllJobs reads each record's cancel/session marker FRESH (after the record), not from
  a stale readdir snapshot, so a marker published between the directory listing and the
  record read is still observed and a cancelled completion can't leak. A null/non-object
  marker body is treated as present (existence is authoritative).
- overlayJob applies to a marked record even if its raw status is already "cancelled", and
  strips `summary` in addition to `result`/`rendered`.
- scanRawJobs: an absent dir (ENOENT) is empty, but EACCES/EIO (on the dir or a visible
  record) propagate; session cleanup fails loud (non-zero exit) on a scan failure instead
  of reporting success. Prune is isolated from the committed upsertJob write, so a GC scan
  failure can never turn a committed `completed` into `failed`.
- migrateLegacyState re-reads the current config before the state.json rewrite and aborts
  the rewrite if that read fails, so a concurrent setConfig isn't clobbered.
- runTrackedJob appends the final-output log BEFORE publishing the terminal record, so the
  terminal write is the worker's last fs touch and cleanup can't orphan a recreated log.
- Resume (--resume-last) only considers completed/failed tasks, never cancelled.
---
 plugins/codex/scripts/codex-companion.mjs     |   7 +-
 plugins/codex/scripts/lib/state.mjs           | 153 +++++++++++-------
 plugins/codex/scripts/lib/tracked-jobs.mjs    |  18 ++-
 .../codex/scripts/session-lifecycle-hook.mjs  |  10 +-
 tests/state.test.mjs                          |  27 +++-
 5 files changed, 144 insertions(+), 71 deletions(-)

diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs
index 600201c85..28794d73e 100644
--- a/plugins/codex/scripts/codex-companion.mjs
+++ b/plugins/codex/scripts/codex-companion.mjs
@@ -308,13 +308,16 @@ function filterJobsForCurrentClaudeSession(jobs) {
 }
 
 function findLatestResumableTaskJob(jobs) {
+  // Only a COMPLETED or FAILED task is resumable. A CANCELLED task must NOT be -- the user
+  // explicitly stopped it, and (because the overlay keeps threadId while marking the job
+  // cancelled) treating "any non-active" as resumable would let a freshly cancelled task
+  // shadow an older genuinely-finished one and resume the wrong Codex thread.
   return (
     jobs.find(
       (job) =>
         job.jobClass === "task" &&
         job.threadId &&
-        job.status !== "queued" &&
-        job.status !== "running"
+        (job.status === "completed" || job.status === "failed")
     ) ?? null
   );
 }
diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs
index 3547f2005..c1833bcd8 100644
--- a/plugins/codex/scripts/lib/state.mjs
+++ b/plugins/codex/scripts/lib/state.mjs
@@ -259,13 +259,29 @@ function migrateLegacyState(cwd) {
     }
     // else: a live/booting worker owns the record -> leave it alone
   }
+  // Rewrite state.json config-only to drop the legacy `jobs` array. Re-read the CURRENT
+  // state fresh rather than reusing the snapshot parsed at the top, so a config committed
+  // by a concurrent setConfig while we migrated per-job files is preserved. If the re-read
+  // FAILS, do NOT rewrite from the stale snapshot (that could revert a concurrent config
+  // update or a rewrite another migrator already did) -- migration is idempotent and
+  // re-runs next time. (A residual TOCTOU between this re-read and the rename remains; it
+  // is the accepted no-CAS class and is much smaller than the pre-read window.)
+  let current;
+  try {
+    current = JSON.parse(fs.readFileSync(stateFile, "utf8"));
+  } catch {
+    return; // couldn't re-read -> leave state.json as-is; retry on the next migration
+  }
+  if (!Array.isArray(current.jobs) || current.jobs.length === 0) {
+    return; // another migrator already dropped the jobs array -> nothing to do
+  }
   try {
     atomicWriteJson(stateFile, {
       version: STATE_VERSION,
-      config: { stopReviewGate: false, ...(parsed.config ?? {}) }
+      config: { stopReviewGate: false, ...(current.config ?? {}) }
     });
   } catch {
-    // A concurrent migrator may have already rewritten it; harmless.
+    // A concurrent migrator may have rewritten it between our re-read and here; harmless.
   }
 }
 
@@ -320,28 +336,39 @@ export function readJobPid(cwd, jobId) {
   return Number.isInteger(pid) && pid > 0 ? pid : null;
 }
 
+// Read job <id>'s cancel marker: its parsed content ({reason, at}), `{}` if the marker
+// exists but is unreadable/non-object (existence is authoritative), or null if there is
+// no marker. A marker whose content parses to a non-object (e.g. literal `null`) must NOT
+// be mistaken for "absent" -- normalize it to `{}`.
+function readCancelMarker(cwd, jobId) {
+  let parsed;
+  try {
+    parsed = JSON.parse(fs.readFileSync(resolveJobCancelFile(cwd, jobId), "utf8"));
+  } catch (err) {
+    return err.code === "ENOENT" ? null : {}; // absent vs present-but-unreadable
+  }
+  return parsed && typeof parsed === "object" ? parsed : {};
+}
+
 // Overlay the immutable markers onto a record so a caller observes a cancellation
 // atomically with the record and can never see a cancelled job as live:
-//  - a `<id>.cancelled` marker forces cancelled, authoritatively ("marked => cancelled,
-//    full stop", overriding even a raced completion);
+//  - a `<id>.cancelled` marker forces cancelled, authoritatively (overriding even a
+//    raced completion, and applying even if the raw record already says cancelled so its
+//    stripping/metadata are always enforced);
 //  - a `session-<hash>.ended` marker forces cancelled for that session's still-LIVE
 //    (queued/running) jobs -- a genuinely finished job keeps its terminal outcome.
-// Completion payload (result/rendered) is dropped so a cancelled job never exposes a
-// half-result. Side-effect free.
-function overlayJob(job, cancelledMeta, endedNames) {
-  if (!job || typeof job.id !== "string" || job.status === "cancelled") return job;
-  const byCancel = cancelledMeta.has(job.id);
-  const bySession =
-    !byCancel && job.sessionId != null &&
-    endedNames.has(sessionEndedBasename(job.sessionId)) &&
-    !TERMINAL_STATUSES.has(job.status);
-  if (!byCancel && !bySession) return job;
-  // Surface the marker's own metadata (best-effort; marker EXISTENCE is authoritative
-  // even if its JSON was unreadable) so a cancel that killed the worker before it could
-  // publish a terminal record still shows a reason and timestamps.
-  const meta = (byCancel ? cancelledMeta.get(job.id) : null) ?? {};
+// The completion payload (result/rendered/summary) is dropped so a cancelled job never
+// exposes any model-generated output. `cancelMeta` MUST be read AFTER the record (see
+// readAllJobs) so the marker/record observation is coherent: cancel/cleanup always
+// publish the marker BEFORE the worker's terminal write, so any record carrying that
+// write is observed together with its marker. Side-effect free.
+function overlayJob(job, cancelMeta, sessionEnded) {
+  if (!job || typeof job.id !== "string") return job;
+  const marked = cancelMeta != null;
+  if (!marked && !sessionEnded) return job;
+  const meta = cancelMeta ?? {};
   const at = typeof meta.at === "string" ? meta.at : null;
-  const { result, rendered, ...rest } = job;
+  const { result, rendered, summary, ...rest } = job;
   return {
     ...rest,
     status: "cancelled",
@@ -353,46 +380,52 @@ function overlayJob(job, cancelledMeta, endedNames) {
   };
 }
 
-// Scan the jobs dir ONCE: return the raw records plus the marker sets. Robust to
-// concurrent create/delete/rename: a name that vanished mid-scan (ENOENT) or a record
-// captured mid-write (parse-guarded) is skipped. `cancelledIds` is a Map id -> marker
-// metadata ({reason, at}, or {} if the marker existed but was unparseable).
-function scanJobsDir(cwd) {
+// Read every RAW job record. `readdirSync` failing with ENOENT means the dir does not
+// exist yet -> genuinely empty; ANY OTHER error (EACCES/EIO/...) is a real, load-bearing
+// scan failure and MUST propagate rather than masquerade as "no jobs" (session cleanup
+// relies on this to not silently skip live workers). A record file that vanished or is
+// mid-write (parse error) is skipped -- that is a per-file transient, not a scan failure.
+function scanRawJobs(cwd) {
   const dir = resolveJobsDir(cwd);
   let names;
   try {
     names = fs.readdirSync(dir);
-  } catch {
-    return { jobs: [], cancelledIds: new Map(), endedNames: new Set() };
-  }
-  const cancelledIds = new Map();
-  const endedNames = new Set();
-  for (const name of names) {
-    if (name.endsWith(".cancelled")) {
-      let meta = {};
-      try { meta = JSON.parse(fs.readFileSync(path.join(dir, name), "utf8")); } catch {}
-      cancelledIds.set(name.slice(0, -".cancelled".length), meta);
-    } else if (name.startsWith("session-") && name.endsWith(".ended")) {
-      endedNames.add(name);
-    }
+  } catch (err) {
+    if (err.code === "ENOENT") return [];
+    throw err;
   }
   const jobs = [];
   for (const name of names) {
     if (!isJobFileName(name)) continue;
+    let raw;
     try {
-      jobs.push(JSON.parse(fs.readFileSync(path.join(dir, name), "utf8")));
+      raw = fs.readFileSync(path.join(dir, name), "utf8");
+    } catch (err) {
+      if (err.code === "ENOENT") continue; // vanished between readdir and read -> skip
+      throw err; // EACCES/EIO on a record we can SEE is a real scan failure -> propagate
+    }
+    try {
+      jobs.push(JSON.parse(raw));
     } catch {
-      // deleted mid-scan or unparseable -> skip
+      // torn/unparseable record -> skip (a transient, not a scan failure)
     }
   }
-  return { jobs, cancelledIds, endedNames };
+  return jobs;
 }
 
 // The marker-overlaid view -- what every consumer (status/cancel/result/session cleanup)
-// should see.
+// should see. Each record's markers are read AFTER the record itself (not from a stale
+// directory snapshot), so a marker published between the directory listing and the record
+// read is still observed.
 function readAllJobs(cwd) {
-  const { jobs, cancelledIds, endedNames } = scanJobsDir(cwd);
-  return jobs.map((job) => overlayJob(job, cancelledIds, endedNames));
+  return scanRawJobs(cwd).map((job) => {
+    if (!job || typeof job.id !== "string") return job;
+    const cancelMeta = readCancelMarker(cwd, job.id);
+    const sessionEnded =
+      cancelMeta == null && job.sessionId != null &&
+      !TERMINAL_STATUSES.has(job.status) && isSessionEnded(cwd, job.sessionId);
+    return overlayJob(job, cancelMeta, sessionEnded);
+  });
 }
 
 // A record is reclaimable ONLY if it is terminal, or a running job whose owner pid is
@@ -437,18 +470,20 @@ export function deleteJobFiles(cwd, job) {
 // Session-ended markers are intentionally NOT GC'd here: a safe generation-aware sweep
 // is out of scope, and one tiny empty file per session is a negligible, race-free leak.
 function pruneJobs(cwd) {
-  const jobs = scanJobsDir(cwd).jobs; // RAW records
-  const overflow = jobs.length - MAX_JOBS;
-  if (overflow <= 0) return;
-  const evictable = jobs
-    .filter(isEvictable)
-    .sort((a, b) => String(a.updatedAt ?? "").localeCompare(String(b.updatedAt ?? "")));
-  let removed = 0;
-  for (const job of evictable) {
-    if (removed >= overflow) break;
+  const jobs = scanRawJobs(cwd); // RAW records
+  if (jobs.length <= MAX_JOBS) return;
+  // Protect the newest MAX_JOBS: a record is a candidate for eviction ONLY if it falls
+  // OUTSIDE that window. Otherwise, when the cap is exceeded entirely by non-evictable
+  // records (e.g. 50 queued pid-less jobs) and one more job just completed, we would
+  // evict that fresh completion -- the only evictable record -- and lose its output.
+  // The cap is soft: a non-evictable (live/booting) record beyond the window is kept
+  // rather than a recent one. Every evictable record outside the window is removed.
+  const byNewest = [...jobs].sort((a, b) => String(b.updatedAt ?? "").localeCompare(String(a.updatedAt ?? "")));
+  const candidates = byNewest.slice(MAX_JOBS).filter(isEvictable); // everything beyond the newest MAX_JOBS
+  for (const job of candidates) {
     const current = readJobRecord(cwd, job.id); // RAW re-check
     if (current && !isEvictable(current)) continue; // became live/pending since scan -> spare
-    if (deleteJobFiles(cwd, job)) removed += 1;
+    deleteJobFiles(cwd, job);
   }
 }
 
@@ -474,7 +509,15 @@ function mergeJobRecord(cwd, jobId, patch) {
     updatedAt: now
   };
   atomicWriteJson(resolveJobFile(cwd, jobId), record);
-  pruneJobs(cwd);
+  // Prune is best-effort GC that runs AFTER the record above is already committed. It must
+  // never throw back into the caller: a prune-scan failure (e.g. EACCES/EIO) must not be
+  // mistaken for a failure of this already-published write (which, in runTrackedJob, would
+  // overwrite a committed `completed` with `failed`).
+  try {
+    pruneJobs(cwd);
+  } catch {
+    // GC failure is non-fatal; the cap is soft and the next write retries prune.
+  }
   return record;
 }
 
@@ -502,5 +545,5 @@ export function listJobs(cwd) {
 // hide the very jobs it must kill.
 export function readAllJobsRaw(cwd) {
   migrateLegacyState(cwd);
-  return scanJobsDir(cwd).jobs;
+  return scanRawJobs(cwd);
 }
diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs
index 5564120d8..b13de40f8 100644
--- a/plugins/codex/scripts/lib/tracked-jobs.mjs
+++ b/plugins/codex/scripts/lib/tracked-jobs.mjs
@@ -159,6 +159,16 @@ export async function runTrackedJob(job, runner, options = {}) {
     const execution = await runner();
     const completionStatus = execution.exitStatus === 0 ? "completed" : "failed";
     const completedAt = nowIso();
+    // Append the final-output log FIRST, then publish the terminal record LAST, so the
+    // terminal write is the worker's last filesystem touch of this job. Otherwise session
+    // cleanup, seeing the terminal record, could delete the log and have this append
+    // recreate an orphan log afterward. The append is best-effort (its own try/catch) so a
+    // logging failure never falls into the lifecycle catch and writes a `failed` record.
+    try {
+      appendLogBlock(options.logFile ?? job.logFile ?? null, "Final output", execution.rendered);
+    } catch {
+      // logging is best-effort; must not affect the published lifecycle state
+    }
     upsertJob(job.workspaceRoot, {
       ...runningRecord,
       status: completionStatus,
@@ -171,14 +181,6 @@ export async function runTrackedJob(job, runner, options = {}) {
       result: execution.payload,
       rendered: execution.rendered
     });
-    // Guard the final log append: the terminal record is already published, so a logging
-    // failure must NOT fall into the catch below and write a second (failed) lifecycle
-    // record over it. The terminal upsertJob above is the worker's last record write.
-    try {
-      appendLogBlock(options.logFile ?? job.logFile ?? null, "Final output", execution.rendered);
-    } catch {
-      // logging is best-effort; never revert a published terminal state
-    }
     return execution;
   } catch (error) {
     const errorMessage = error instanceof Error ? error.message : String(error);
diff --git a/plugins/codex/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs
index 18fbcdcd6..42a0cf0d8 100644
--- a/plugins/codex/scripts/session-lifecycle-hook.mjs
+++ b/plugins/codex/scripts/session-lifecycle-hook.mjs
@@ -114,12 +114,16 @@ export function cleanupSessionJobs(cwd, sessionId) {
   }
 
   // 2. Scan the RAW records (not the overlaid view -- the marker we just wrote would
-  // otherwise hide this session's active jobs from us).
+  // otherwise hide this session's active jobs from us). A scan failure here (EACCES/EIO;
+  // an absent dir returns [] rather than throwing) is load-bearing: we may be leaving a
+  // live worker unscanned/unkilled, so fail LOUD rather than silently returning success.
   const isActive = (job) => job.status === "queued" || job.status === "running";
-  let jobs = [];
+  let jobs;
   try {
     jobs = readAllJobsRaw(workspaceRoot).filter((job) => typeof job.id === "string" && job.sessionId === sessionId);
-  } catch {
+  } catch (err) {
+    process.exitCode = 1;
+    process.stderr.write(`codex: session cleanup for ${sessionId} could not scan jobs: ${err instanceof Error ? err.message : String(err)}\n`);
     return;
   }
   const active = jobs.filter(isActive);
diff --git a/tests/state.test.mjs b/tests/state.test.mjs
index f24ff0413..90c09a141 100644
--- a/tests/state.test.mjs
+++ b/tests/state.test.mjs
@@ -126,13 +126,34 @@ test("prune evicts oldest terminal jobs but keeps live and non-terminal ones ove
 
   const ids = new Set(listJobs(workspace).map((job) => job.id));
   assert.equal(ids.has("live"), true, "a live running job must never be pruned, even as the oldest");
-  // The two oldest *terminal* jobs are evicted (done-00, done-01), payload + log.
-  assert.equal(ids.has("done-00"), false);
-  assert.equal(ids.has("done-01"), false);
+  // Only records OUTSIDE the newest MAX_JOBS window are eviction candidates. With 52
+  // records the two oldest are `done-00` and `live`; `done-00` (terminal) is evicted,
+  // `live` (non-evictable) is kept -> the cap stays soft at 51. `done-01` is inside the
+  // protected newest-50 window, so it survives.
+  assert.equal(ids.has("done-00"), false, "the oldest terminal job (outside the window) is evicted");
   assert.equal(fs.existsSync(resolveJobLogFile(workspace, "done-00")), false, "evicted job's log is removed too");
+  assert.equal(ids.has("done-01"), true, "a job inside the newest-MAX_JOBS window is protected");
   assert.equal(ids.has("done-02"), true, "newer terminal jobs are kept");
 });
 
+test("prune never evicts a just-completed job when the cap is full of non-evictable jobs", () => {
+  const workspace = makeTempDir();
+  ensureStateDir(workspace);
+  // MAX_JOBS (50) queued, pid-less jobs -> all non-evictable.
+  for (let i = 0; i < 50; i += 1) {
+    const id = `q-${String(i).padStart(2, "0")}`;
+    const ts = new Date(Date.UTC(2026, 0, 1, 0, i, 0)).toISOString();
+    fs.writeFileSync(resolveJobFile(workspace, id), JSON.stringify({ id, status: "queued", pid: null, updatedAt: ts, createdAt: ts }));
+  }
+  // A 51st job completes with full output; its write triggers prune.
+  upsertJob(workspace, { id: "fresh", status: "completed", result: { codex: { stdout: "important" } } });
+
+  const job = listJobs(workspace).find((entry) => entry.id === "fresh");
+  assert.ok(job, "the just-completed job is the ONLY evictable record but must not be pruned");
+  assert.equal(job.status, "completed");
+  assert.equal(fs.existsSync(resolveJobFile(workspace, "fresh")), true);
+});
+
 test("legacy state.json jobs[] array migrates to per-job files on read", () => {
   const workspace = makeTempDir();
   ensureStateDir(workspace);