From 426a532d50750300228d191d10dc7c11ae385f9a Mon Sep 17 00:00:00 2001 From: Sinity Date: Tue, 21 Jul 2026 15:37:18 +0200 Subject: [PATCH] fix(sources): restore fail-closed refusal for byte-governed heads Problem Another lane reported 2 failures on current master: tests/unit/sources/test_live_batch_support.py:: test_bundle_replay_respects_unconvertible_single_session_head [bundle_texts2-False-False] and [bundle_texts3-False-True] (assert result.failed == [older_bundle], failing shape assert [] == [...]). The working hypothesis was that polylogue-miwv's messages_fts_identity INSERT OR REPLACE fix (PR #3239) removed an IntegrityError abort that had been masking this failure. Diagnosis (bisected by checking out each of the three recently-merged commits directly, not by stash): that hypothesis is wrong. The exact same test failure reproduces identically on commit b3429fae6 (#3234), the commit immediately BEFORE messages_fts_identity (#3235) was even introduced -- the ledger and my INSERT OR REPLACE fix are unrelated. git bisect via commit checkout traced the real regression to PR #3211 ("perf(sources): tree-byte decoded layer + in-cohort head-retire drift fix"), landed 2026-07-20, ~10.5 hours before #3235. That PR's "drift fix" half removed a byte-governance refusal from ArchiveStore.apply_raw_membership_classification (storage/sqlite/archive_tiers/archive.py) on the premise that its branch is only reachable after a real membership-governance conversion, so a still-set raw_sessions.logical_source_key must be interrupted-pass drift, not live evidence. That premise is false for the accepted head specifically: _apply_membership_sessions (sources/live/batch.py) unconditionally injects the CURRENT raw_revision_heads accepted raw into the comparison cohort even when it was NEVER converted -- exactly PR #2718's original scenario (a byte-governed head being compared against membership-discovered content for the first time). With the guard gone, a later-discovered bundle raw whose content happens to strictly extend the byte-governed head's content (a pure string-prefix superset) silently replaced the head via membership governance: message_count moved 2->3, accepted_raw_id changed, even though the head still had a live, unresolved QUARANTINED append raw blocking ordinary byte-chain conversion. Confirmed via a standalone diagnostic script inspecting raw_revision_heads/raw_session_memberships state directly (not relying on the test's own asserts, which stop at the first failure) -- content genuinely lands, not merely a reporting-list discrepancy: the fail-closed contract was actually violated. #3211 shipped this change bundled into an unrelated perf PR without running tests/unit/sources/test_live_batch_support.py in its own verification section, so the regression was undetected (per-PR CI also skips the heavy test suite by design). Solution Restored a byte-governance refusal in apply_raw_membership_classification, narrower than #2718's original blanket `logical_source_key IS NOT NULL` check so #3211's own interrupted-pass-drift resumption (a stale un-nulled key with no dangling append) keeps working: refuse only when (a) replay is about to CHANGE which raw is accepted (accepted_raw_id != existing_raw_id -- a no-op re-affirmation of the same raw is never refused, matching #2718's original condition) AND (b) a live raw_sessions row elsewhere in this logical_source_key still chains a predecessor_source_revision off the existing head's own source_revision and isn't already part of the classified cohort -- i.e. genuine unresolved byte-append evidence this classification pass never saw, not merely "was this raw ever membership-converted". Verification - Bisected via direct commit checkout (not git stash): reproduced the exact failure on b3429fae6 (pre-#3235) and traced the introducing diff to 996a3d6d3 (#3211) via `git log -S` on the removed guard string. - devtools test tests/unit/sources/test_live_batch_support.py -k test_bundle_replay_respects_unconvertible_single_session_head -> 4 passed (all four bundle_texts/succeeds/census_head parametrizations, including the succeeds=True case that must still work) - devtools test tests/unit/sources/test_live_batch_support.py tests/unit/storage/test_revision_replay.py tests/unit/sources/test_revision_backfill.py -> 136 passed - devtools test tests/unit/storage/test_raw_retention.py -> 59 passed - mypy --strict polylogue/storage/sqlite/archive_tiers/archive.py tests/unit/sources/test_live_batch_support.py -> Success, no issues - devtools verify --quick -> exit 0 (also regenerated docs/topology-status.md, which was already out of sync on unmodified origin/master before this branch -- unrelated pre-existing drift, picked up incidentally by the routine gate) - Anti-vacuity: reverted just the new guard body (replaced with a no-op `pass`) and reran the 4-case test -> 2 of 4 failed with the exact original symptom; restored the fix and reran -> 4 passed AC matrix | AC | Status | | --- | --- | | Diagnose true mechanism (bisect between the 3 merged changes) | Satisfied -- root cause is #3211, not messages_fts_identity/miwv | | Determine whether content actually lands or reporting-only | Satisfied -- content genuinely landed (message_count 2->3, head moved) before this fix | | Fix production path to preserve explicit refusal | Satisfied | | head_after == head_before + message_count stays (2,) for succeeds=False | Satisfied (already asserted by the pre-existing test, now passing) | Ref polylogue-miwv Co-Authored-By: Claude --- docs/topology-status.md | 6 +- .../storage/sqlite/archive_tiers/archive.py | 66 +++++++++++++++---- tests/unit/sources/test_live_batch_support.py | 25 +++++++ 3 files changed, 83 insertions(+), 14 deletions(-) diff --git a/docs/topology-status.md b/docs/topology-status.md index 7edf3d3038..5bf213a9f2 100644 --- a/docs/topology-status.md +++ b/docs/topology-status.md @@ -28,12 +28,12 @@ Generated by `devtools render topology-status`. Reads `docs/plans/topology-targe ### Summary -- **Stable** (no move scoped): 901 +- **Stable** (no move scoped): 902 - **Kernel** (polylogue/ root): 8 - **Primitives** (storage-root): 19 - **TBD** (cell needs explicit assignment): 9 -- **Total declared**: 1072 -- **Realized polylogue/**/*.py**: 1072 files declared +- **Total declared**: 1073 +- **Realized polylogue/**/*.py**: 1073 files declared ### TBD cells (require explicit routing) diff --git a/polylogue/storage/sqlite/archive_tiers/archive.py b/polylogue/storage/sqlite/archive_tiers/archive.py index ba89624592..37e754da96 100644 --- a/polylogue/storage/sqlite/archive_tiers/archive.py +++ b/polylogue/storage/sqlite/archive_tiers/archive.py @@ -3356,17 +3356,61 @@ def apply_raw_membership_classification( f"ambiguous={classification.ambiguous_raw_ids!r}) " f"persisted_session_raw={None if persisted_session is None else str(persisted_session[0])!r}" ) - # No byte-governance refusal here: this branch is only - # reachable when the head raw is a member of THIS - # cohort, and membership rows for a source-keyed raw - # only exist after a governance conversion decided it - # belongs to membership classification -- a still-set - # logical_source_key on raw_sessions is interrupted- - # pass ordering drift (the conversion's key-nulling - # had not committed when the pass died), not evidence - # of live byte-chain governance. Foreign byte heads - # never reach this branch: they yield in the - # chain-governed-head branch above. + # polylogue-miwv: #3211 removed a byte-governance + # refusal here on the theory that this branch is only + # reachable after a real governance conversion (a + # still-set logical_source_key being interrupted-pass + # drift, not live evidence). That premise is false for + # the accepted head specifically: ``_apply_membership_ + # sessions`` (sources/live/batch.py) unconditionally + # injects the current ``raw_revision_heads`` accepted + # raw into the comparison cohort even when it has + # NEVER been through a membership conversion -- the + # #2718 scenario this whole code path exists for + # (a byte-governed head being compared against + # membership-discovered content for the first time). + # Content-prefix growth alone cannot prove the older + # bundle raw supersedes a head that still has live, + # unresolved byte-append evidence hanging off it (a + # quarantined/pending append raw whose + # ``predecessor_source_revision`` chains to the head's + # own ``source_revision``) that this classification + # pass never saw. Only matters when replay is about to + # CHANGE the accepted raw (mirrors #2718's original + # ``accepted_raw_id != existing_raw_id`` guard + # condition) -- a classification that keeps the same + # existing_raw_id as the accepted member (e.g. the + # head's own content already dominates every other + # cohort member) is a no-op re-affirmation, not a + # replacement, regardless of dangling evidence. + # Narrower than #2718's original blanket + # ``logical_source_key IS NOT NULL`` check so #3211's + # own interrupted-pass-drift resumption (no dangling + # append descendant, just a stale un-nulled key) is + # unaffected. + if accepted_raw_id != existing_raw_id: + classified_placeholders = ", ".join("?" for _ in classified_raw_ids) or "NULL" + dangling_append = conn.execute( + f""" + SELECT 1 + FROM raw_sessions AS child + WHERE child.logical_source_key = ? + AND child.raw_id != ? + AND child.raw_id NOT IN ({classified_placeholders}) + AND child.predecessor_source_revision IS NOT NULL + AND child.predecessor_source_revision = ( + SELECT source_revision FROM raw_sessions WHERE raw_id = ? + ) + LIMIT 1 + """, + (logical_source_key, existing_raw_id, *classified_raw_ids, existing_raw_id), + ).fetchone() + if dangling_append is not None: + raise RuntimeError( + "membership replay cannot replace a head with unresolved byte-append " + f"evidence: logical_source_key={logical_source_key!r} " + f"existing_head(raw_id={existing_raw_id!r})" + ) self._conn.execute( "DELETE FROM raw_revision_heads WHERE logical_source_key = ?", (logical_source_key,), diff --git a/tests/unit/sources/test_live_batch_support.py b/tests/unit/sources/test_live_batch_support.py index 9cf662eb21..e871e748f4 100644 --- a/tests/unit/sources/test_live_batch_support.py +++ b/tests/unit/sources/test_live_batch_support.py @@ -3811,6 +3811,31 @@ def test_bundle_replay_respects_unconvertible_single_session_head( succeeds: bool, census_head: bool, ) -> None: + """Pins #2718's fail-closed contract: a bundle raw discovered later must + never silently replace an accepted head that still has live, unresolved + byte-append evidence (the QUARANTINED append raw this test binds), even + when the bundle's own content happens to strictly extend the head's + content (``bundle_texts2``/``bundle_texts3``: content-prefix growth alone + is not proof of provenance). + + polylogue-miwv (2026-07-21): #3211 ("in-cohort head-retire drift fix") + removed ``apply_raw_membership_classification``'s byte-governance refusal + on the mistaken premise that its branch is only reachable after a real + membership-governance conversion -- but ``_apply_membership_sessions`` + unconditionally injects the CURRENT accepted head into the comparison + cohort even when it has never been converted (exactly this test's byte- + governed-head scenario), so the removed guard's absence let the older + bundle's superset content silently move the head (message_count 2->3, + ``accepted_raw_id`` changed) for ``bundle_texts2``/``bundle_texts3``. + This was not caused by, and is unrelated to, the messages_fts_identity + UNIQUE(block_id) ledger work landing the same day (polylogue-miwv's + other commits) -- confirmed by reproducing this exact failure on the + commit immediately preceding messages_fts_identity's introduction. + Restored as a narrower guard (refuses only when replay is about to change + the accepted raw AND a live raw_sessions row still chains a + ``predecessor_source_revision`` off the existing head) so #3211's own + interrupted-pass-drift resumption keeps working. + """ root = tmp_path / "sessions" root.mkdir() current = root / "current.jsonl"