locks: 600s actuation TTL underruns the AI review it wraps since #9013, with no renewal → concurrent double actuation

[JSONbored]

Parent: #9466

Summary

PR_ACTUATION_LOCK_TTL_SECONDS is 600 s, but since #9013 that lock wraps the entire publish → AI-review → maintain unit, whose worst-case legitimate duration exceeds 600 s. There is no renewal mechanism for any transient lock. When the TTL expires mid-work, the holder is never told, a second worker acquires the lock, and both proceed to actuate the same PR.

The AI-review lock (1800 s) has the same defect with a different arithmetic.

Mechanism (verified at HEAD, 776c414)

The lock now wraps the LLM call. #9013 (1533e4d4f) deliberately moved the claim before maybePublishPrPublicSurface so publish-then-maintain is one question with one answer — src/queue/processors.ts:4218:

const actuationLock = await claimPrActuationLock(env, repoFullName, pr.number);

The AI review runs inside that section (processors.ts:11075-11142).

The work can outlast the TTL. Per-attempt CLI timeout is 120–600 s by effort (src/selfhost/ai.ts:212; a per-repo override clamps at 1,800,000 ms per attempt, :217), with 3 attempts per model (src/services/ai-review.ts:1480) and up to 2 reviewers. At default medium effort, one timeout-retry on each of two reviewers is 2 × 2 × 180 s = 720 s of LLM time alone — already past the 600 s TTL, before grounding (96 KB file fetches), RAG, enrichment, publish, and the entire maintenance half.

Nothing renews. claimTransientLock (src/queue/transient-locks.ts:43-78) sets a TTL once. Only the queue's own processing lease got a heartbeat (#9023). An expired holder never learns it lost the lock — its releaseIfValue simply no-ops against the new owner's token (correct, but silent).

The interleaving

1. t0 — worker A (webhook path) claims the lock, enters the LLM pipeline.
2. t0+600 — Redis expires the key while A is still working. A is not notified.
3. t0+601 — worker B (sweep agent-regate-pr — a different coalesce key by design; processors.ts:2683 documents that this exact pair overlaps) claims successfully.
4. A has already written ai_review_cache and is stalled in the maintenance half under GitHub rate-limit backoff — the exact scenario orb(anti-abuse): contributor-cap-lock TTL (30s) is far shorter than the work it guards — reopens the #7284 TOCTOU #9024 documented for the sibling contributor-cap lock.
5. B reads the real close/merge-eligible verdict from cache and reaches executeAgentMaintenanceActions concurrently with A.
6. The executor's freshness check (src/services/agent-action-executor.ts:493-503) is read-then-act: both read "open" within the same seconds.

Outcome: duplicate close plus duplicate close-explanation comment plus duplicate decision records. On a merge, the loser records a spurious failed outcome. In the milder ordering, B's aiReviewLockContendedResult placeholder republishes over A's real check-run and panel — the verdict-thrash #9013 was written to eliminate.

Sibling: the AI-review lock

AI_REVIEW_LOCK_TTL_SECONDS = 1800 (src/queue/ai-review-orchestration.ts:89). At max effort a single model's retry budget is exactly 3 × 600 s = 1800 s. Any second reviewer, fallback chain, or per-repo claudeTimeoutMs override (clamped at 30 min per attempt, #8458) exceeds it. A second pass then claims, fires a duplicate LLM call, and the two passes' ai_review_cache upserts race — last writer wins, so two contradictory verdicts can alternate across passes. The TTL's own comment calls it "a crash-safety backstop, not a throughput bound", but nothing bounds work duration below it.

Requirements

1. A lock must remain held for as long as its owner is genuinely working, without picking a TTL large enough to strand the key after a crash.
2. A holder that loses its lock (renewal fails, key evicted, stolen) must find out and abort rather than actuating.
3. steal semantics (orb(review): forceAiReview does not bypass the AI-review lock — the re-run button is silently inert behind an orphaned lock (root cause of #9000) #9008) must keep working.

Deliverables

• Add a token-checked renewal primitive to src/queue/transient-locks.ts — re-assert the TTL every TTL/3 only if the stored value still equals this holder's ownerToken (Redis Lua PEXPIRE guarded by a value compare; the DO path gets the equivalent compare-and-extend). Mirror heartbeatProcessingLease (src/selfhost/pg-queue.ts:1323).
• Wire renewal into both long-hold call sites: the actuation lock around publish-then-maintain (processors.ts:4218, :7274) and the AI-review lock (ai-review-orchestration.ts).
• On a failed renewal (token no longer ours), surface it to the holder and abort before any mutation — treat as contention, deferring via the mechanism from queue: actuation-lock contention burns the retry budget and kills the job — one-shot reopen enforcement lost in production (PR #9450) #9465 rather than proceeding.
• Stop the renewal timer in a finally on every exit path, so a completed job never leaves a timer extending a released key.
• Alternative to evaluate and record in the issue before implementing: restructure so the actuation lock covers only the publish + maintain half, with the AI section outside it. Cheaper, but reopens the split-decision window orb(review): public-surface publish runs outside the per-PR mutex — duplicate gate check-runs and placeholder overwriting a real verdict #9013 closed — state explicitly why whichever option is chosen was chosen.
• Add a boot-time assertion (or config-resolution warning) when resolved attempts × timeout × reviewers exceeds the AI-review lock TTL, so a future timeout-config change cannot silently recreate this.

Tests (must fail against current main)

• A holder whose work runs 3× the base TTL still owns the lock throughout; a competing claimTransientLock is refused for the whole duration.
• Renewal stops after release: no timer extends a key the holder no longer owns.
• A holder whose key is deleted mid-work (simulating eviction or steal) observes the failed renewal and aborts without actuating.
• steal still takes ownership from a live holder, and the stolen-from holder aborts.
• Parity across the SubmissionLock DO path and the Redis cache path.

Expected outcome

Lock hold duration is decoupled from a guessed TTL. A long AI review cannot cause two workers to actuate the same PR, and no lock is stranded after a crash beyond one renewal interval.

[linear-code]

JSO-6531