fix(approval-queue,lifecycle): reopen expired staged actions, restore accept-path protections, and stop honest iteration escalating (#9481, #9482, #9483)

[JSONbored]

Summary

Three states a PR could enter and never leave, plus a set of protections that were silently inert on the approval-queue replay path.

Closes #9481
Closes #9482
Partially addresses #9483 (verdict-flip half; see Scope)

#9481 — an expired staged action could never be re-staged

agent_pending_actions_target_unique is a full unique index on (repo_full_name, pull_number, action_class) — no status predicate — and createPendingAgentActionIfAbsent used onConflictDoNothing against it. So once #9032's sweep expired a row, every later re-plan conflicted with it permanently: created: false, so stageForApproval returned before notifying, and decidePendingAgentAction reported already_decided for a non-pending row. Nothing anywhere deletes or reopens an expired row.

Net effect: any auto_with_approval merge/close whose maintainer was away for the expiry window became permanently unexecutable via the queue for that PR + action class. That is strictly worse than the pre-#9032 behaviour (rows waited forever but stayed acceptable), and it is the opposite of what the sweep's own doc promises — "a later pass that re-plans the same action stages a fresh row with a fresh notification".

An expired row is now reopened in place, guarded on status = 'expired' so a concurrent accept/reject can never be clobbered, and reported as created: true so the caller notifies exactly as for a new row. Only expired reopens: pending is legitimately sticky and accepted/rejected are terminal by design.

Also fixed: the step-4 audit fired unconditionally, so when staging no-op'd against an existing row every later sweep still recorded "awaiting maintainer approval" — an audit trail asserting a notification that was never sent. It is now conditional on staging having actually happened.

#9482 — the accept path built a thinner context, voiding three protections

decidePendingAgentAction's executor context omitted authorLogin, expectedBaseRef and moderationSettings. All three fields are optional, so nothing forced parity with the live path's context and nothing failed loudly:

• orb(race): the contributor-cap mutex is released before the merge it guards, so the #7284 TOCTOU is still open — a legitimate PR is closed for a cap already relieved #9159's cap mutex was inert. Step 8c claims claimContributorCapLock(env, repo, ctx.authorLogin ?? ""), so an absent author produced the key contributor-cap-lock:<repo>: — the empty-author key, sharing no namespace with the live path's ...:<author>. The mutex that shipped specifically so "the two can never both act on a stale view" provided zero mutual exclusion on this path, leaving the feat(miner): add tenant create/list/destroy control-plane admin CLI #7284 TOCTOU open, while also making every accept-path merge in a repo contend on one shared key.
• The moderation ladder undercounted. maybeEscalateModeration early-returns on a falsy authorLogin, so a staged-then-accepted blacklist / contributor-cap / review-nag close recorded no moderation violation. A contributor whose enforcement closes always route through approval accumulated zero standing violations toward the warning/ban threshold.
• orb(webhook): retargeting a PR base re-uses the diff computed against the OLD base — stale CI, stale review, wrong merge #9055's base-retarget guard could never fire. fetchPullRequestFreshness only evaluates the base when expectedBaseRef is set, so a contributor retargeting the PR base between staging and accept — head unchanged, so the head-SHA pin still matched — merged into a base the reviewed diff and CI never targeted.

#9483 (verdict-flip half) — honest iteration escalated, permanently

The flip guard counted a flip on any direction change, keyed per (repo, pull) with no content dimension. The exploit it defends against is re-rolling a non-deterministic reviewer over the same diff until a lucky clean roll lands — but honest iteration produces an identical signal: defect(A) → fix → clean(B) → new issue(C) → fixed(D) is three flips, i.e. two honest fix cycles reach the threshold of 3.

Worse, flipCount never decreased, so once escalated the PR escalated on every future fresh verdict — an absorbing state whose own user-facing advice ("push a substantive fix so the next review reflects real content change") was structurally unable to clear it, because a substantive fix produces a fresh verdict that re-escalates.

Flips now count only when the review's content fingerprint matches the prior verdict's; genuinely changed content resets the count. Migration 0196 adds the nullable column. Re-rolling unchanged content still escalates exactly as before.

Scope

#9483 also covers the merge-train stranding half (a waiting PR is never re-driven when its blocker resolves by any means other than a merge). That needs a new wake path and its own tests, and it interacts with the ordering work in #9499 — it stays open rather than being half-done here.

Validation

• npx tsc --noEmit, npm run db:migrations:check (contiguous 0001..0196), npm run db:schema-drift:check — all clean
• 201 passed across verdict-flip-guard, approval-queue-staleness, agent-approval-queue, routes-agent-approval, mcp-automation-state

Regressions, each verified to fail against the unfixed code:

• expectedBaseRef reaches the freshness check; with it reverted the assertion fails.
• The cap lock is author-keyed: the test pre-claims the author key and asserts the accept is denied. With authorLogin reverted the accept proceeded (it claimed the empty-author key), so the test genuinely pins the fix.
• An expired row reopens with created: true, a fresh reason, cleared decision metadata, and still exactly one row for the target.

Invariants:

• A pending row stays sticky — a re-plan must not reset an outstanding decision.
• accepted and rejected rows stay terminal (parameterised over both) — only expiry reopens.
• A reopened row can expire and reopen again — recovery is not one-shot.
• Honest iteration on changed content never accumulates flips; a substantive fix clears an already-escalated state; and — importantly — the exploit still escalates immediately after a reset, so the reset is not a laundering path.
• A missing fingerprint on either side never counts a flip, so pre-migration rows fail open.

Merge-order note

This adds migrations/0196_verdict_flip_fingerprint.sql. #9503 also adds a 0196 (retention_column_indexes_round_two) — both branch from main, so the contiguity check passes on each in isolation but whichever merges second needs renumbering to 0197. #9503 is already fully green, so it will likely land first; flagging so the collision is expected rather than surprising.

[loopover-orb]

Important

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏳ LoopOver is waiting…

LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting}

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

❌ Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.27%. Comparing base (d1c770c) to head (2d87621).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/queue/processors.ts	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9508      +/-   ##
==========================================
+ Coverage   89.55%   95.27%   +5.71%     
==========================================
  Files         843      741     -102     
  Lines      110135    60483   -49652     
  Branches    26207    21314    -4893     
==========================================
- Hits        98635    57623   -41012     
+ Misses      10238     1618    -8620     
+ Partials     1262     1242      -20     

Flag	Coverage Δ
backend	95.27% <93.33%> (+<0.01%)	⬆️
control-plane	?
engine	?
rees	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/db/repositories.ts	96.80% <100.00%> (+<0.01%)	⬆️
src/review/verdict-flip-guard.ts	100.00% <100.00%> (ø)
src/review/verdict-flip-store.ts	100.00% <100.00%> (ø)
src/services/agent-action-executor.ts	96.77% <100.00%> (+<0.01%)	⬆️
src/services/agent-approval-queue.ts	98.10% <ø> (ø)
src/queue/processors.ts	94.83% <0.00%> (ø)

... and 272 files with indirect coverage changes