state.json job index: unguarded read-modify-write race loses concurrent jobs and deletes their artifacts

[piero-dev25]

Summary

scripts/lib/state.mjs persists the per-workspace job index (state.json) with an unguarded read-modify-write (loadState → mutate → fs.writeFileSync, no lock, no atomic rename, no compare-and-swap). When two or more companion invocations register or update jobs on the same workspace root concurrently, the last writer wins and every other writer's job entry is silently dropped from the index.

Because saveState also treats "job missing from the retained list" as "job pruned" (state.mjs ~lines 105–112), the winning writer then deletes the losing jobs' .json payload and .log files via removeJobFile/removeFileIfExists — so the lost work isn't just unindexed, its artifacts are destroyed.

Downstream, /codex:status, /codex:result, and /codex:cancel resolve jobs exclusively through this index (job-control.mjs, matchJobReference), so a completed --background task can become permanently unretrievable through the documented interface with no error surfaced anywhere.

Reproduction (plugin v1.0.6, codex-cli 0.143.0, macOS, Node 22)

// upsert-one.mjs — calls the plugin's real upsertJob
import { upsertJob } from "<plugin-root>/scripts/lib/state.mjs";
const [cwd, id] = process.argv.slice(2);
upsertJob(cwd, { id, status: "running", kind: "repro" });

mkdir -p /tmp/race/ws && cd /tmp/race/ws && git init -q
export CLAUDE_PLUGIN_DATA=/tmp/race/plugin-data

# Sequential — control
for i in $(seq 1 20); do node upsert-one.mjs "$PWD" "seq-$i"; done
# → listJobs(): 20 of 20 survive

rm -rf /tmp/race/plugin-data

# Concurrent
for i in $(seq 1 20); do node upsert-one.mjs "$PWD" "con-$i" & done; wait
# → listJobs(): 1 of 20 survives

Observed across three independent runs on this machine: sequential = 20/20; concurrent = 1/20, 1/20, 1/20.

Impact

• Two Claude Code sessions (or one session plus a background job) sharing a checkout and using /codex:rescue --background, /codex:review --background, or any concurrent task invocations can silently lose each other's job registrations.
• Status transitions racing against registrations have the same window: a runTrackedJob completion update can be overwritten by an unrelated writer, or vice versa.
• The pruning side effect deletes the losing job's payload/log files, so the result is unrecoverable even by hand.
• Nothing fails loudly — the caller sees No job found for <id> later, or simply never finds the job.

Suggested direction

Any of these would close the window:

1. Atomic replace: write to state.json.tmp.<pid> then fs.renameSync — removes torn reads, though not lost updates.
2. Advisory lock (e.g. mkdir-based or proper-lockfile) around updateState's load→mutate→save critical section — removes lost updates.
3. Treat the per-job jobs/<id>.json files as the source of truth and rebuild the index from a directory scan, reserving state.json for config only — sidesteps the shared-file contention entirely and makes the pruning deletion safe.

Found during an automated concurrency audit of the plugin before adopting it for multi-session workflows. Happy to provide the full harness or re-run against a patched branch.

[hsms4710-pixel]

I opened #460 to fix this. The CI workflow is currently waiting on maintainer approval to run (first-time contributor). Could a maintainer approve the workflow run so the tests can execute? Happy to address any feedback.