Skip to content

state-drift marks a completed review as failed even when the reviewed diff is unchanged #114

Description

@patriyang

Observation

Found while fixing the consumer-side symptom in obsidian-vault (patriyang/obsidian-vault#753, #754). Filing the plugin-side gap separately; both issues there explicitly declined to prescribe a plugin fix.

A background review whose base ref moves during the run is reported as status: failed, failureClass: state-drift, with a ⚠️ STALE REVIEW banner — even though the review completed and its findings are fully preserved.

Where

  • captureRepoStateIdentity (plugins/codex/scripts/lib/git.mjs:372-385) records the resolved OID of HEAD, plus the base ref's OID in branch mode, at enqueue.
  • describeRepoStateDrift (git.mjs:387-409) re-resolves both at completion and returns a drift reason on any mismatch.
  • buildCompletion (codex-companion.mjs:541-557) then forces exitStatus: 1, which tracked-jobs.mjs:268-271 turns into status: "failed".

Why it is a problem

The comparison is on OIDs, not on the reviewed diff. In a repo where hooks or a cron commit to main continuously, origin/main advances during essentially every review, so state-drift fires on runs whose branch diff is byte-identical to what was reviewed. Measured in the vault repo: 33 commits to origin/main in 60 minutes, nearly all touching only note directories that cannot affect any branch diff.

Consequences:

  1. The signal is always on, so it carries no information. A genuinely aborted review and a healthy one both surface as failed with the same banner.
  2. Post-completion and pre-execution drift are indistinguishable by status, yet they mean opposite things. Post-completion drift preserves the complete rendered review (render.mjs:484 whitelists state-drift so /codex:result prints it); pre-execution drift (assertPinnedState, codex-companion.mjs:529-536) aborts before Codex is invoked and produces no output at all. Callers must currently infer which they got from whether a rendered body came back.
  3. Callers with a review-round budget pay for it. Agents in that repo are capped at 3 rounds, and spurious drift consumes them.

Not prescribing the fix

Directions worth weighing, none obviously correct:

  • Compare the resolved diff rather than the base-ref OID, and treat a drift whose diff is unchanged as non-stale.
  • Keep the banner as a warning but stop classifying post-completion drift as a job failure (it already carries complete output).
  • Distinguish HEAD moved from base ref ... moved in the reported class — HEAD drift invalidates findings, base-ref drift usually does not.
  • Offer an opt-out (--allow-stale or similar). There is currently no flag or env var that suppresses or downgrades the classification.

Workaround in use

Pinning an immutable base defeats the base-ref half: --base <merge-base sha> re-resolves to itself. Combined with -C <worktree> (whose HEAD is the worktree's own branch tip, unaffected by commits landing on main in the primary checkout), neither side of the comparison moves. Verified across 3 consecutive reviews: all returned status: completed with no banner.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions