From e3b95903d5980ce9c13273ab04fdbf3425b51b92 Mon Sep 17 00:00:00 2001 From: Sinity Date: Tue, 28 Jul 2026 12:07:18 +0200 Subject: [PATCH] fix(storage): resolve duplicate-alias batch-race as a permanent no-op ## 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 --- polylogue/storage/raw_reconciler.py | 34 ++++++++++++ .../test_duplicate_raw_identity_repair.py | 54 +++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/polylogue/storage/raw_reconciler.py b/polylogue/storage/raw_reconciler.py index 3b045b813e..86c3b13e3f 100644 --- a/polylogue/storage/raw_reconciler.py +++ b/polylogue/storage/raw_reconciler.py @@ -1148,6 +1148,40 @@ def _apply_strategy( duplicate_locked = _inspect_duplicate_raw_identity( conn, root, item.raw_id, canonical_ids[0], logical_source_key ) + if duplicate_locked.status == "ineligible": + # polylogue-ewfp: a legitimate N:1 duplicate-alias fan-out + # terminal outcome, not a transient apply failure. + # Several sessions can share one stale raw as their + # accepted head; only ONE fold can ever land on the + # single available canonical twin. When several fan-out + # siblings are selected together from one pre-apply + # census snapshot (all looked executable before any of + # them committed), a sibling earlier in this same batch + # can already have claimed the canonical by the time + # this plan's own apply runs -- this plan is now + # permanently moot, not merely blocked pending a retry. + # The witness comparison below is skipped deliberately: + # it exists to catch data drift that would invalidate an + # ELIGIBLE fold between census and apply, which does not + # apply once the fold is no longer being attempted at + # all. Committing this empty transaction and returning + # normally (rather than raising) matches the existing + # "already_repaired" no-mutation-but-resolved path + # immediately below, so the caller records EXECUTED + # (permanently resolved) instead of RETRYABLE -- which + # previously required the postflight check to see this + # exact plan_id survive unchanged forever, crashing + # every subsequent raw-materialization pass that + # reached this fan-out group. + conn.commit() + return json_document( + { + "strategy": item.actuator.value, + "repaired_count": 0, + "already_repaired_count": 0, + "ineligible_reason": duplicate_locked.reason, + } + ) if _duplicate_strategy_witness(duplicate_locked) != item.strategy_witness: raise RuntimeError("duplicate strategy proof changed after plan authorization") if duplicate_locked.status == "eligible": diff --git a/tests/unit/storage/test_duplicate_raw_identity_repair.py b/tests/unit/storage/test_duplicate_raw_identity_repair.py index 57fb522a13..4d8f6a6134 100644 --- a/tests/unit/storage/test_duplicate_raw_identity_repair.py +++ b/tests/unit/storage/test_duplicate_raw_identity_repair.py @@ -658,3 +658,57 @@ def fake_inspect( assert sibling_item.actuator is RawAuthorityActuator.NONE assert not sibling_item.executable assert "already an accepted head" in sibling_item.reason + + +def test_duplicate_alias_batch_race_does_not_crash_postflight(tmp_path: Path) -> None: + """polylogue-ewfp regression: applying BOTH fan-out siblings together must not crash. + + At a single pre-apply census snapshot, the canonical raw is still + genuinely dangling for every fan-out sibling, so a batch selection can + (and, live, did) select more than one sibling's fold together in the + SAME ``apply_raw_authority_frontier`` call. The first plan applied + commits and claims the canonical; the second plan's own re-inspection + inside ``_apply_strategy`` then legitimately finds ``status="ineligible"`` + (not a transient failure). Before the fix, this raised, which the + caller's generic exception handler labeled ``RETRYABLE`` -- an outcome + the postflight check then required to remain byte-identical forever, + crashing every subsequent raw-materialization pass that reached this + fan-out group (observed live: a 405s writer-lock hold ending in + "raw authority postflight changed a retryable/carried-forward plan"). + The second plan must instead resolve as a permanent, non-retryable + no-op. + """ + stale_raw_id, canonical_raw_id, heads = _seed_duplicate_raw_fanout(tmp_path) + (session_a, key_a), (session_b, key_b) = heads + + preview = inspect_raw_authority_frontier(_config(tmp_path)) + duplicate_items = { + item.logical_source_key: item + for item in preview.items + if item.raw_id == stale_raw_id and item.state is RawAuthorityFrontierState.DUPLICATE_ALIAS + } + assert set(duplicate_items) == {key_a, key_b} + + # The regression: selecting BOTH siblings together must not raise. + report = apply_raw_authority_frontier( + _config(tmp_path), + preview_census_id=preview.census_id, + selected_plan_ids=(duplicate_items[key_a].plan_id, duplicate_items[key_b].plan_id), + ) + + assert report.selected_plan_count == 2 + assert report.executed_plan_count == 2 + assert report.retryable_plan_count == 0 + assert report.success + + with sqlite3.connect(tmp_path / "index.db") as conn: + heads_by_key = dict( + conn.execute( + "SELECT logical_source_key, accepted_raw_id FROM raw_revision_heads WHERE logical_source_key IN (?, ?)", + (key_a, key_b), + ).fetchall() + ) + # Exactly one sibling folded onto the canonical; the other's own head is + # untouched, still pointing at the (now-orphaned) stale raw -- correctly + # recognized as permanently ineligible rather than corrupted or retried. + assert sorted(heads_by_key.values()) == sorted((canonical_raw_id, stale_raw_id))