fix(storage): resolve duplicate-alias batch-race as a permanent no-op - #3369
Conversation
## Problem Discovered live 2026-07-28, immediately after deploying PR #3368 (which fixed polylogue-dmvo's original census crash): the daemon's raw-materialization pass still crashed, now with `RuntimeError: raw authority postflight changed a retryable/carried-forward plan`, holding the writer lock for 405 seconds before failing with 9 other queued actors starved behind it. One session (896c6b64) had genuinely converged onto its canonical raw -- proving PR #3368's fix works -- but 3 other fan-out siblings remained stuck. Root cause: at a single pre-apply census snapshot, the canonical raw is still genuinely dangling for EVERY fan-out sibling (forked/subagent/resumed sessions sharing one stale raw as their accepted head, polylogue-ihc8), so a batch selection can select more than one sibling's fold together in the SAME `apply_raw_authority_frontier` call (all looked executable before any of them committed). The first plan applied commits and claims the canonical; when the second plan's own apply then re-inspects (`_apply_strategy`'s FOLD_DUPLICATE_ALIAS branch), it legitimately finds `status="ineligible"` -- not a transient failure, a permanent one, since the canonical is gone for good. The code only handled `eligible` (mutate) and `already_repaired` (no-op success); any other status -- including this expected `ineligible` case -- fell through to a `raise`. The caller's generic exception handler labels ANY raised exception `RETRYABLE`, and the postflight check then requires that exact plan_id to survive byte-identical forever -- but a duplicate-alias sibling's own evidence legitimately changes once its canonical is claimed, so its plan_id changes too, tripping the check on every subsequent pass that reaches this fan-out group. ## Solution Extend the existing `already_repaired` no-mutation-but-resolved pattern to also cover `ineligible`: commit the empty transaction and return normally (skipping the witness-staleness check, which doesn't apply once no fold is being attempted at all) so the caller records this plan as EXECUTED (permanently resolved) rather than RETRYABLE. New regression test reproduces the exact live batch shape: both fan-out siblings selected together in one `apply_raw_authority_frontier` call, proving the whole apply+postflight sequence completes without raising and exactly one sibling folds while the other resolves as a permanent no-op. Anti-vacuity: reverting the fix reproduces the exact live error message verbatim ("raw authority postflight changed a retryable/carried-forward plan"). ## Verification - `devtools test tests/unit/storage/test_duplicate_raw_identity_repair.py` -> 11 passed - `devtools test tests/unit/storage/ -k "raw_reconciler or raw_authority or duplicate"` -> 86 passed - `mypy polylogue/storage/raw_reconciler.py` -> clean - `devtools verify --quick` -> exit 0 Ref polylogue-ewfp, polylogue-ihc8 Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3b95903d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| duplicate_locked = _inspect_duplicate_raw_identity( | ||
| conn, root, item.raw_id, canonical_ids[0], logical_source_key | ||
| ) | ||
| if duplicate_locked.status == "ineligible": |
There was a problem hiding this comment.
When two or more fan-out plans share the canonical raw, this path records every later sibling as EXECUTED without changing its stale head. Postflight then classifies that unchanged sibling as UNRESOLVED_PROVENANCE (_classify_frontier, lines 520-524), which archive_readiness.py counts as blocking; the next daemon census persists it as an obligation but cannot select it because it is non-executable. Thus the exception is avoided, but raw-materialization readiness remains permanently false for this exact batch scenario. This no-op must also produce a nonblocking terminal frontier classification rather than merely returning success.
Useful? React with 👍 / 👎.
…ification (#3370) ## Summary Fix the final layer of the live incident chain (PRs #3368, #3369 fixed two real bugs and measurably improved the writer-hold from 543s to 210s): crash-recovery finalization was applying the same strict "no plan may change" invariant as a normal apply, which a stuck multi-day-old census can never satisfy once its classification legitimately moves on. ## Problem recover_interrupted_raw_authority_frontier runs on every daemon startup and finalizes every still-planned census. One census had sat unfinalized across multiple days. A stopped-daemon read-only diagnostic showed the affected sessions now classify as unresolved_provenance/refine_quarantined_raw (the underlying raw got quarantined by an unrelated safety mechanism) -- not duplicate_alias anymore, as they were when the census was originally planned. finalize_raw_authority_census's postflight check required every retryable/carried-forward plan to survive byte-identical in the fresh post-census, regardless of whether this was a normal apply or crash recovery. This left the census permanently unable to finalize: every recovery attempt raised on the same stale plan_ids, on every single daemon restart. ## Solution Skip the byte-identical-persistence check when interrupted=True. Correct for a normal apply; wrong for crash recovery, whose entire purpose is reconciling against current ground truth after an arbitrary gap. ## Verification - devtools test tests/unit/storage/test_raw_authority_ledger.py tests/unit/storage/test_duplicate_raw_identity_repair.py -> 41 passed - mypy polylogue/storage/raw_authority.py -> clean - devtools verify --quick -> exit 0 Ref polylogue-ewfp, polylogue-ihc8 Co-Authored-By: Claude <noreply@anthropic.com>
… follow-up polylogue-ewfp's crash is fixed and confirmed live: PR #3370 (crash-recovery finalize tolerance) resolves the true recurring-crash mechanism after #3368/#3369 fixed two adjacent layers of the same stack. Verified: the stuck census finally finalized (outcome=success), every previously-starved daemon actor now runs cleanly, no more crash across multiple subsequent passes. Filed polylogue-zaiz for the distinct, unattempted residual: 3 fan-out sessions now correctly classify as refine_quarantined_raw (a separate actuator/workflow) rather than crashing -- non-urgent, daemon operates normally around it. Co-Authored-By: Claude <noreply@anthropic.com>
…e key (#3371) ## Summary Fix the architectural gap behind polylogue-zaiz (the residual "3 fan-out sessions stuck" finding from the ihc8/ewfp incident chain): _inspect_quarantined_accepted_raw had the same unscoped-lookup bug class _inspect_duplicate_raw_identity had before ihc8's fix. ## Problem Forked/subagent/resumed sessions can physically replay the identical parent evidence, so the same raw_id can legitimately be the accepted head of several logical source keys/sessions at once. _inspect_quarantined_accepted_raw looked up the accepted-head row by accepted_raw_id alone, requiring exactly one match system-wide -- for a fan-out raw, this unconditionally raised "expected one accepted head, found N" for every sibling, permanently blocking any of them from ever being refined. Investigation also surfaced a much larger, separate, pre-existing fact: 15,798 of ~41,334 raw_sessions rows (38% of the archive) carry revision_kind='unknown' -- a known backlog predating this session and unrelated to this scoping bug. ## Solution - _inspect_quarantined_accepted_raw requires a logical_source_key parameter, scoping its lookups. - inspect_quarantined_accepted_raws takes (raw_id, logical_source_key) pairs instead of bare raw_ids. - _strategy_overrides builds per-session override keys for the quarantine path specifically. - _apply_strategy's REFINE_QUARANTINE branch threads the same key through and gains the identical graceful "ineligible" no-op handling already shipped for FOLD_DUPLICATE_ALIAS (ewfp, #3369). ## Verification - devtools test tests/unit/storage/test_quarantined_accepted_raw_repair.py tests/unit/storage/test_duplicate_raw_identity_repair.py tests/unit/storage/test_raw_authority_ledger.py -> 52 passed - devtools test tests/unit/storage/ -k "raw_reconciler or raw_authority or duplicate or quarantine or repair" -> 228 passed, 1 pre-existing unrelated failure - mypy clean - devtools verify --quick -> exit 0 Ref polylogue-zaiz, polylogue-ihc8 Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
Summary
Fix a live production bug (discovered immediately after deploying PR #3368) where the raw-materialization pass still crashed on the
polylogue-ewfppostflight invariant when multiple duplicate-alias fan-out siblings are selected together in the same apply batch.Problem
At a single pre-apply census snapshot, the canonical raw is still genuinely dangling for every fan-out sibling, so a batch selection can select more than one sibling's fold together in the same
apply_raw_authority_frontiercall. The first plan applied commits and claims the canonical; the second plan's own apply then legitimately findsstatus="ineligible"— a permanent, not transient, outcome. The code only handledeligible/already_repaired; any other status fell through toraise, which the caller's generic exception handler labeledRETRYABLE, and the postflight check then required that exact plan_id to survive byte-identical forever — but a sibling's evidence legitimately changes once its canonical is claimed, tripping the check on every subsequent pass.Observed live: a 405-second writer-lock hold ending in an uncaught
RuntimeError, with 9 other queued daemon actors starved behind it.Solution
Extend the existing
already_repairedno-mutation-but-resolved pattern to also coverineligible: commit the empty transaction and return normally so the caller records this plan as EXECUTED (permanently resolved) rather than RETRYABLE.Verification
devtools test tests/unit/storage/test_duplicate_raw_identity_repair.py→ 11 passed (new regression test reproduces the exact live batch shape — both siblings selected together in one call; anti-vacuity confirmed by reverting and observing the exact live error message)devtools test tests/unit/storage/ -k "raw_reconciler or raw_authority or duplicate"→ 86 passedmypy polylogue/storage/raw_reconciler.py→ cleandevtools verify --quick→ exit 0Ref polylogue-ewfp, polylogue-ihc8
Co-Authored-By: Claude noreply@anthropic.com