fix(sync): converge public SWM catch-up across bounded repeat passes (#2050) - #2076
Open
Jurij89 wants to merge 32 commits into
Open
fix(sync): converge public SWM catch-up across bounded repeat passes (#2050)#2076Jurij89 wants to merge 32 commits into
Jurij89 wants to merge 32 commits into
Conversation
Chunks 1 and 4 of the #2050 plan, plus the test rows landed so far. Chunk 1 - per-peer SWM snapshot coverage: - SwmSnapshotCoverage carried as ONE coherent record, selected whole from a single peer. Ranked authority -> manifestComplete -> largest snapshotsTotal -> most resolved. Ranking by FRACTION returns 200/200 over 178/250 and so reports "0 outstanding" on a job 72 assets short - self-consistent and undetectable downstream, which is worse than the synthetic pair the record shape exists to prevent. - replayPhaseBytesReceived / snapshotPhaseBytesReceived split out of the merged bytesReceived scalar, so the accepted cost of repeating the walk is measurable in bytes rather than only in triples. Chunk 4 - the bounded repeat: - shouldRunAnotherCatchupPass: pure, injectable clock, documented stop-reason precedence (coverage-stalled outranks budget-exhausted, because a stalled run that also ran out of time must not read as "raise the budget"). - Capability gate reads coverage ONLY. A yielding peer carries failedPeers > 0 and is exactly the peer the next pass exists to revisit, so gating on "no failures" would strand the work the loop was built for. - Coverage is retained when a round fails and deleted only after a clean round reports none. A throw, a denial or a backpressure-deferred plane all return no coverage without being evidence of absence - and deferral is the r26 shape specifically, with sync-global at 2/2 for 99.7% of that job. - Distinct-peer accounting via Sets; peersNotAttempted could otherwise go negative once passes repeat. - Continuation passes are shared-memory only, so repeats never re-pull durable. Tests: - All ten terminal readiness strings pinned (previously zero coverage). - T9 reproduces G7 against a real OxigraphStore and FAILS on this commit by design: assertion graphs are written while head rows are absent. It goes green with Chunk 3's per-KA metadata insert. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
Chunk 2. Three changes that let a repeated peer walk make progress instead of restarting into the same wall. Skip-and-continue on a short prefix. Ref order is byte-identical every pass (Map insertion order), so returning on an unserveable ref pinned every future pass at that index and drove coverage to a fixed point. The ref is now recorded and the walk continues; the KA stays uncached and unapplied, and is retried from offset zero next pass. A non-blocking yield before each KA, checked before the fetch. A cache "hit" is O(KA size) and a miss is a round trip, so the clock is read before either. Checking before the fetch also means no SyncPageResult exists on that path, so timedOutPhases structurally cannot move: a local budget decision can never be reported as a peer timeout and put a healthy responder into backoff. It does set failedPhases - the round genuinely did not complete the plane, and without that a yielding round classifies as clean and reports the graph done with KAs still missing. Bounded missing state: an exact missingCount plus a missingSample capped at 10, since a public peer controls manifest size. completed is now derived (missingCount === 0) rather than hardcoded. All three give-up paths route through one noteMissing helper, so a round can no longer fall out of the loop claiming success while having abandoned work - which is exactly what skip-and-continue would have done against the old hardcoded true. swm-recovery.test.ts passes with no edits (14 tests), including the single-snapshot short-prefix case that pins completed: false. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
…2050) Chunk 5. The incomplete-progress terminal could not say WHAT was missing, even though the answer was computable in memory at the moment the round gave up. It now appends a clause naming the counts, the peer they came from, the pass count, and a bounded sample of the outstanding Knowledge Assets. Every figure comes from ONE SwmSnapshotCoverage record, so the sentence cannot describe a graph state no peer reported and the named KAs always belong to the manifest the counts came from. The base sentence is byte-identical; the clause is appended. It is omitted entirely - exactly '' - when there is no coverage record, and when the selected manifest was fully resolved: in that case the shortfall lies on another plane and "0 outstanding" beside an unreachable verdict would misdirect the reader. An incomplete manifest is reported as a lower bound rather than a total. Wording is scoped to "Shared memory" throughout, since continuation passes repeat the shared-memory walk only and must not imply the durable plane was retried. T16 pins that sentence as a PREFIX so the append does not break it - which means T16 is satisfied by the prefix followed by anything at all, including nothing. T16b added here is the row that can see the append: whole-string equality plus the cap, the truncation marker, the pass-count singular/plural, and both omission cases. Mutating the clause to vanish kills 7 of its rows while T16's prefix pin stays green, which is the point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
…g exit (#2050) `replaceHeadMetadata` is delete-only, and the compensating `storeInsert(processed.verifiedMeta)` sits below the `continue` on the incomplete branch. A round that ran out of clock mid-list therefore deleted the head rows of the KAs it had just materialized and never rewrote them: content present, heads absent, invisible to every head reader — and permanent, because the next round sees the content and skips. Each KA's verified metadata is now written inside its own write lock, immediately after the delete, at all three exits: the union-insert repair, the replace path, and the already-materialized return. That last one is widened from "no head at all" to "the head does not certify THIS descriptor's version", so a head left behind at an older version by a pass that stopped between the replace and the swap is repaired too. Both states are unreachable by any content-based check and never self-heal through partial rounds, which are the only rounds this scenario gets. `insertedMetaTriples` keeps its exact meaning: on a usable round it stays `processed.verifiedMeta.length`, byte-identical to before, because the bulk insert now counts the full length minus the rows the per-KA path already wrote. On a partial round it becomes non-zero, which inverts the field's diagnostic sense — it was the G7 symptom, it is now the repair signal. The ledger subtracts by size rather than re-filtering so the identity holds even if a meta response carries a duplicate quad. Also captures materialization counters before a throw. A snapshot-phase transport failure unwinds past every post-call merge, so a round that materialized N KAs and then threw reported zero inserted triples and read to the pass loop as non-advancing progress. The counters are now added per KA, inside the lock, once the KA is durably committed. Adds `continuationStopReason` to the shared-memory diagnostics, typed as the pass policy's closed union so a new stop reason cannot reach the terminal message unnoticed, and requires `manifestComplete` in the capability gate: a truncated-manifest round caches blobs and advances `snapshotsResolved` while materializing nothing, which would otherwise satisfy the coverage-advance gate forever. A peer that yielded mid-snapshot-list completed its meta phase first, so it still qualifies. Refs #2050. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
…2050) The shortfall clause reported how much was missing but not why the loop gave up. "72 outstanding because a further pass stopped making progress" and "72 outstanding because the time budget was exhausted" call for opposite operator actions, and the message could not tell them apart. Renders continuationStopReason through an exhaustive Record over the closed CatchupPassDecisionReason union, so adding a reason fails the build here rather than silently rendering nothing. Omitted when the loop never ran - shared memory not requested - and for 'continue', which is not a stop. coverage-stalled deliberately outranks budget-exhausted in the policy, so a run that stalled AND expired reports the stall. Its wording therefore never mentions time: "ran out of time" would send an operator to raise a budget that buys nothing, which is the precise misdirection that precedence exists to prevent. A test asserts the text matches no /budget|time|expired/, and mutating it to blame the clock kills exactly that row. Also states both facts an incomplete manifest carries: the denominator is a lower bound AND that peer was not retried, since the capability gate requires a complete manifest. A truncated round advances snapshotsResolved against a truncated denominator while materializing nothing, so "the count understates it" and "we stopped asking" are different things for the reader. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
A catch-up that will not converge is currently only diagnosable by reconstructing it from counters, and the two states an operator most needs to tell apart — a walk that stopped because it finished and one that stopped because it gave up — look identical in the numbers. Each pass now emits one line carrying the Context Graph, the capable-peer count, coverage before and after, elapsed time, and the typed stop reason. Coverage is always printed with the 8-char suffix of the peer that reported it: the record is selected whole from one round, so the counts and their peer belong together, and printing the counts alone would invite reading them as a fleet total. A truncated manifest says so, because its denominator is only a lower bound. Coverage before and after appear on the same line deliberately. A pass with large elapsed time and unmoved coverage is the signature of a job that is not converging, and that is invisible in either number alone. The Worker has no logger, so the line is formatted where the coverage records live and emitted through a new `logCatchupPass` host call. That call can never affect the job: a catch-up that failed because a log line could not be delivered would be strictly worse than one with a missing log line, so the RPC rejection is swallowed. That also keeps a host which does not implement the method from turning observability into a dependency. Refs #2050. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
Second half of "capture counters before a throw". The first half made the DATA counters honest by adding them per KA inside the write lock; this makes the DECISION honest. The continuation loop's progress signal is swmCoverage.snapshotsResolved, and that record is assembled from syncPublicSnapshotsForMeta's return value. A snapshot-phase transport failure throws, so the return never happened: a round that materialized 120 Knowledge Assets and failed on the 121st recorded no coverage at all, the high-water mark did not move, and the loop reported coverage-stalled and abandoned a peer that was converging. That is the r26 shape and the behaviour #2050 exists to remove - and with the counters already fixed it was worse than before, because the diagnostics looked right while the decision was wrong. The walk now attaches its own counts to the error and rethrows; the outer catch folds them into a coverage record. Everything from the failing index on is counted missing, so resolved + missing === total holds on the failure path exactly as on the returned one. manifestComplete is hoisted out of the try so a throwing round can still mark a truncated denominator as a lower bound. The payload is non-enumerable so it cannot widen a structured clone or a log dump, and it is validated on read rather than trusted: it crosses an unknown boundary, and a fabricated denominator would corrupt the record that both the capability gate and the terminal message read. No new abandonment path - the throw route calls the same abandonFrom helper - so completed: missingCount === 0 remains the single completion expression. No third bytesReceived site, so replayPhaseBytesReceived + snapshotPhaseBytesReceived === bytesReceived is untouched. T14 pins it. Mutating the recovery away reproduces the pre-fix symptom exactly: expected undefined to deeply equal the record. Also removes a duplicate import pair appended with T13 - the same two symbols are already imported at the top of the file. It survives esbuild today, but tsc does not typecheck test files, so nothing would have caught it under a stricter toolchain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
…ps (#2050) `continuationStopReason` was assigned on every pass decision, so the field transiently held `'continue'` — a value that is not a stop reason at all. It could not escape today, because the loop's only exit is the break on a non-continuing decision, but that made the guarantee a property of the current control flow rather than of the type: one added exit path, such as a cancellation break after a pass, would have published it. Assigning only on the stopping decision makes `'continue'` structurally unrepresentable in the field. The terminal message already absorbed it — `SWM_STOP_REASON_TEXT.continue` is the empty string, which is falsy and omits the clause — so this would have degraded to a missing sentence rather than malformed text. That defensive entry is now provably dead rather than load-bearing, which is the better state for it to be in. Refs #2050. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
…erified (#2050) Two seam defects that had to be fixed together, because fixing the first alone unmasks the second. FINDING 1 - the terminal message went silent on shared memory in exactly the failure class #2050 is about. swmShortfallClause gated on missingCount, which measures FETCH completeness only; it is produced entirely by noteMissing inside the snapshot walk. Materialization failures are a separate counter and never touch it, while usability reads both. So a round where every ref fetched cleanly and some KAs could not be WRITTEN - a store error inside the KA write lock, the G7 failure class, likeliest under the same store pressure that produces incomplete rounds - produced missingCount === 0 and an empty clause. The operator got the base sentence and nothing at all about shared memory in the one case where shared memory was what failed. FINDING 2 - snapshotsResolved counts refs present and digest-valid in the blob CACHE, not Knowledge Assets written, and the clause rendered it as "snapshots verified". Finding 1 was masking it: with the clause suppressed, the misleading sentence never printed. Fixing the gate alone would have started reporting "250/250 snapshots verified" on a graph where every write failed - wrong in the flattering direction and undetectable by the reader. Now "fetched", with writes reported separately. The two shortfalls are separate clauses, not one conflated number, because they send an operator to different places: not retrieved is a network and peer-set problem, retrieved but not written is a store problem. materializationFailures now travels ON the coverage record. Both the success path and the throw path build that record through ONE shared recordSnapshotCoverage helper, so it can never be reassembled in a catch block from scalars that did not travel together - the failure mode the whole-record contract exists to prevent. The counter is mirrored outside the try so a later throw cannot lose it, and is defaulted rather than trusted on read, since the record crosses a worker RPC boundary. Mutating the gate back to retrieval-only kills exactly the two rows that describe the defect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
The most consequential defect in this PR, and it sat inside the fix. snapshotsResolved was walk.readySnapshots - refs retrieved and digest-valid in the blob cache. The walk increments that unconditionally after the materialization hook, and the hook swallows its own failures into materializationFailures without rethrowing. So a round where all 250 refs are cached and EVERY materialization fails produced 250/250 with missingCount 0. Three consumers read that number, not one: - the capability gate computed 250 < 250 -> false -> peer not capable - the coverage high-water mark advanced to maximal - the terminal message consulted missingCount and stayed silent So the continuation stopped, silently, having written zero Knowledge Assets - a round that fetched everything and materialized nothing looked like perfect progress to the loop meant to catch exactly that. Resolved now counts refs whose every descriptor is locally present. missingCount is derived as total - resolved, so it covers both never-fetched and fetched-but-unwritten without tracking either twice, and the invariant holds by construction rather than by arithmetic maintained in two places. Message, gate and high-water mark correct together, and "250/250 verified on a graph with zero written" becomes unrepresentable rather than merely avoided. The walk's own readySnapshots is deliberately unchanged: it feeds completed/missingCount, which are genuinely about FETCH completeness and are consumed by the private lane. swm-recovery.test.ts passes with zero edits, as it has all session. A truncated-manifest round now reports resolved 0, which is correct - nothing was written - and manifestComplete already gates that case in the capability predicate. Mutating resolved back to readySnapshots kills both coverage rows. Also removes T14's invariant assertion rather than rewriting it. It computed 2 + 1 === 3 from an object the preceding deep-equal had already pinned, and the invariant now holds by construction, so any version of it is a check that cannot fail. A vacuous assertion is worse than none - it reads as coverage of a property nothing checks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
T9 pins that a partial round leaves head rows for exactly the KAs it materialized, and none for the rest, asserted over the full final quad set against a real OxigraphStore. It failed on the pre-fix tree with `expected [] to deeply equal [ …(2) ]` — both assertion graphs written, zero head rows — which is the r26 residual stated as a test rather than inferred from insert counters. T9b covers the THIRD head-deleting exit: the already-materialized return where the head is absent (`version === null`, so `needsRepair` is false) or pinned at an older version. Neither T9 row reaches it — both go through the materialize path — and a mutation removing the exit's widening survived 26/26 before this. Both run on the PARTIAL branch deliberately. On a complete round the bulk `storeInsert(processed.verifiedMeta)` writes every head regardless, so the assertions would pass with the repair removed; with the round incomplete, the exit's own insert is the only thing that can write them. `share()` gains `ual`/`payloadCount`, both defaulting to the previous values, so v1/v2 and the 13 existing tests are behaviourally unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
Moves `share()`/`manifest()` into `swm-descriptor-fixtures.ts` so the throw-path row in `sync-requester-progress.test.ts` can build DESCRIPTOR-shaped metadata instead of hand-rolling a fourth SWM fixture. Why this matters beyond deduplication: materialization is gated on `snapshotDescriptorsByRef.size > 0`, populated by `parseGraphScopedSwmRecoveryDescriptors`. Metadata carrying only `publicQuadsDigest`/`publicQuadsCount` is `collectPublicSnapshotMetadata` shape — enough to enumerate refs, not enough to yield a descriptor. A fixture in that shape leaves the map empty, so `snapshotsResolved` stays 0 for a reason unrelated to the code under test. And a subtly wrong fixture does not error: `validateOperationRows` throws on a kaUal/assertionVersion mismatch, the caller catches it and calls `snapshotDescriptorsByRef.clear()`, silently disabling materialization for the whole Context Graph. It stops testing rather than failing. These builders are known-good by provenance: the partial-round row built on them failed pre-fix with `expected [] to deeply equal [ …(2) ]` against a real OxigraphStore, which is only reachable if the descriptors are real. Positional signatures are kept as thin adapters so every existing call site is untouched. All 17 tests pass, including the 13 pre-existing ones — re-proven by run, since a defaults-preserving refactor is exactly the kind that looks obviously safe and isn't. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
T14 is titled "carries the resolved count out through the throw instead
of reporting zero" and asserted snapshotsResolved: 0. Its fixture wired
no snapshotMaterializer, so zero was true BY CONSTRUCTION - the row
passed while structurally unable to observe its own property.
Two opposite properties live on snapshotsResolved, and a
no-materializer fixture collapses them to the same number:
- do not OVER-report: a round that fetches N and materializes 0 must
not claim N/N. Covered by the coverage rows in this file, precisely
BECAUSE they wire no materializer, so fetched != materialized.
- do not UNDER-report: a round that materializes some and then THROWS
must report what it wrote. Covered by nothing - and it is the whole
reason "carry snapshot progress out through a throw" exists.
Distinct from the correct-mutant-wrong-property family: the assertion
was sound; the FIXTURE erased the distinction, so no strengthening of
the assertion could have helped. Renaming the row would have made the
title honest and left the load-bearing property at zero coverage.
Now builds DESCRIPTOR-shaped metadata via the shared fixtures, wires
the real materializer against a real OxigraphStore, and caches two of
three snapshots so the third's fetch throws after two are materialized.
Expectation becomes 2 resolved / 1 missing.
A real store and the real materializer rather than a stub: descriptor
metadata that is subtly wrong does not error. validateOperationRows
throws, runSharedMemorySync catches it and calls
snapshotDescriptorsByRef.clear(), and materialization is SILENTLY
disabled for the whole Context Graph - reproducing resolved: 0 for a
new reason, indistinguishable from a pass.
Verified by mutation, not by the passing run: forcing the throw path's
coverage record to 0 resolved kills the row with
- "snapshotsResolved": 2,
+ "snapshotsResolved": 0,
- "missingCount": 1,
+ "missingCount": 3,
which is the pre-fix defect exactly. The previous row asserted 0 and
would have PASSED under that mutant. 56/56 green with it reverted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
Every link in the continuation chain was pinned individually — the coverage record survives a throwing round, the capability gate reads coverage rather than failure counters, the high-water mark advances, and the pure stop rule decides correctly. Nothing executed the seam joining them: that a round which materialized some Knowledge Assets and then failed actually causes a second pass to run. That gap is where both of the last two defects on this change survived. Each piece was individually correct; a chain of correct links can still fail to be connected. The load-bearing assertion is the pass COUNT, not the terminal coverage record. A row that checks only the final numbers passes whether the loop ran once or twice, because the second pass simply overwrites the first pass's record with the converged values. It also asserts the durable plane was pulled exactly once, pinning continuation passes as shared-memory only. A negative row covers the other direction: a first pass that resolved nothing does not earn a repeat, since the high-water mark starts at 0. Without it the positive row would pass under an implementation that always ran a second pass. Mutation: forcing the loop to stop after the first decision regardless of its outcome kills the seam row on the pass count (`expected [ peer ] to deeply equal [ peer, peer ]`) and leaves the other 35 rows green — including the negative row, which already expects one pass. Kill set predicted before running, source verified clean before and the mutant verified present after, then reverted to 36/36. Refs #2050. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
Nine of the ten terminal error strings in this file are pinned with toBe. One used c.error?.startsWith(...) - and a prefix pin is satisfied by the prefix followed by ANYTHING, including nothing. That is the exact pattern this PR already catalogued after mutating a clause to emit nothing and watching seven rows die while a prefix pin stayed green. Verified the weaker form bought nothing: replacing it with full equality passes unchanged, 50/50. The suffix it was tolerating does not exist in this scenario, so the row was strictly weaker for no reason. toBe now fails if the shortfall clause is appended here, dropped, or reworded - none of which startsWith could see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
Both surviving gate clauses were unpinned, and the obvious fixture cannot see either. A single barren peer leaves the high-water mark unmoved, so the loop stops at `coverage-stalled` before the gate is consulted and the row passes whether the guard exists or not — the same fixture-collapse pattern that let a throwing-round row assert `snapshotsResolved: 0` under a title promising a non-zero one. Pairing a productive peer with the peer under test reaches the gate, and the assertion becomes the MEMBERSHIP of the second pass rather than whether one happened. `manifestComplete` is now pinned: mutating it to `true` re-contacts a truncated-manifest peer and the row dies with `expected [ 'peer-truncated-3333', …(3) ] to have a length of 1 but got 4`. `snapshotsTotal > 0` is NOT pinned, and the row does not pretend otherwise. Mutating it to `>= 0` leaves all 38 rows green, because the clause is redundant: a barren peer reports `0/0`, and `snapshotsResolved < snapshotsTotal` is already false there. For the clause to change any outcome, `resolved < total` would have to hold with `total <= 0` — that is, a negative resolved count, which no path produces. It is a defensive restatement of a condition its neighbour already enforces, kept for readability rather than effect, and recorded as such instead of being reported as covered. Refs #2050. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
…mutants The retain-on-failure rule had no coverage, and finding an observable for it meant ruling two out. After a throwing pass the stop reason cannot tell retention from forgetting: retained leaves coverage equal to the high-water mark, forgotten makes the max over an empty map 0, and both are <= the mark, so both report `coverage-stalled`. The terminal message cannot either — it renders the reduced `diagnostics.sharedMemory.swmCoverage`, which `accumulate` builds independently of `lastCoverageByPeer`, so forgetting a peer there is invisible to it. A second, still-productive peer keeps the mark advancing so a further pass runs at all, and membership of that pass becomes the observable. That peer's rate is load-bearing: at +1 per pass the peer under test, retained at 2, holds the max flat and the loop stops before the pass the row depends on. Both mutants die, on disjoint rows, which is what shows the pair tests the predicate rather than merely that something changed: unconditional delete (pre-fix) -> kills RETAINS, leaves FORGETS green unconditional retain -> kills FORGETS, leaves RETAINS green The FORGETS row needed a second fix to earn that. With a default clean round it passed under unconditional retain, because a clean DATA-BEARING round proves the plane and the loop stops at `plane-proven` before the capability gate is consulted — a negative control that could not see the guard it existed to protect. It now returns a clean round carrying no data. Also corrects the gate's own comment, which claimed `snapshotsTotal > 0` excludes barren peers. It does not: `snapshotsResolved < snapshotsTotal` already fails them at 0 < 0. The clause has no reachable effect and no honest fixture can kill it, so the comment now says so — in the code, where the next mutation window will read it, rather than in a commit body. Refs #2050. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
This was referenced Aug 4, 2026
Contributor
Author
Follow-up issues filedDeliberately out of scope here; each is referenced in the PR's What is NOT fixed section.
Not yet run: the paired Base Sepolia baseline/candidate runs. Methodology is prepared — rows normalized per job and per materialized KA (never per admission, since extra passes create admissions by construction), queue-wait labelled admitted-only because |
The per-pass line was the one #2050 surface with no coverage. It is the only PER-PASS observability an operator gets - the terminal record reports the FINAL state, so without it a job that converged in four passes is indistinguishable from one that converged in one. It needs a row rather than a reader because it fails silently by design: the line travels as a fire-and-forget RPC whose rejection is deliberately swallowed so observability can never fail a catch-up. A line that stops being emitted therefore produces no error anywhere. Asserts the TRANSITION, not the endpoint. A line reporting only the final 3 could not distinguish a pass that advanced coverage from one that ran and achieved nothing, which is the single fact the line exists to convey. Verified by mutation: stubbing logPassLine to emit nothing kills this row with "expected [] to have a length of 1", in 21ms - an assertion death, not a timeout. Exactly ONE test died, which is the evidence it covers something no other row does. 41/41 green with it reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
Comment-only. Found by re-reading T16b to confirm its assertions match its name - they do (full-equality toBe on the clause, plus the pass-count wording). Its HEADER did not: it justified T16b's existence by saying T16 pins that message as a prefix, which stopped being true when the preceding commit strengthened T16 to toBe. Self-inflicted staleness of exactly the kind this PR kept finding: the fact was corrected in one place and the prose explaining WHY a sibling exists still encoded the retired version. The justification is restated on the durable ground rather than the one that moved: T16's fixture never appends a shortfall, so no assertion of any strength in it can observe this clause. Strength and reachability are different properties, and it is reachability that makes these rows necessary - which stays true however T16 is later asserted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
7 tasks
Review on #2076 raised three defects in the fix itself. All three were validated against the code by independent readers and adversarially challenged before being accepted; the severity arguments are recorded on the threads. 1. The progress gate and the capability gate read different peer sets. `capablePeersForNextPass` retries only peers with `resolved < total`, but `highestResolvedCoverage` maxed over EVERY retained record. A peer at 400/400 pinned the reading, and a capable peer converging against a 120-ref manifest could never clear that bar, so it got exactly one continuation pass however well it was doing — and the stop was reported as `coverage-stalled`, whose text says more passes would not help. Replaced with `totalPeerProgress`, a sum over a monotone per-peer high-water ledger. Filtering the max to the capable set was rejected: it inverts the bug, because the capable set shrinks as peers finish. 2. The plane-proven stop was consulted before the capability gate and read an aggregate over every peer, so a member serving its own SWM rows and no snapshot refs — the ordinary shape in a multi-member public CG — ended the walk while another peer still had a complete manifest with refs outstanding. The loop contributed zero passes in exactly the topology it was written for. Now gated on the capable set being empty. 3. `missingCount` was redefined as a materialization shortfall while the type doc and terminal formatter still called it retrieval, so a ref that fetched and digest-verified but failed to write was named "not retrieved" — sending an operator to the network when the fault was the store. Note `materializationFailures > 0` with `missingCount === 0` is now unrepresentable, and a fixture pinned exactly that pair. Relabelled to "materialized"/"not materialized", corrected the type docs, made the write count a cause indicator rather than a second disjoint axis, deduped the ref sample, and rebuilt the T16c fixtures on records the producer can actually emit. Each new row was mutation-tested: reverting the fix kills it on an assertion, not a timeout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 4, 2026
Review on #2076 found three behaviours this PR introduced or changed that no test drove. Each row below was mutation-tested: reverting the behaviour it pins kills it on an assertion. 1. Skip-and-continue. The short-prefix branch of `syncPublicSnapshotsForMeta` became `noteMissing + continue`, so one unserveable ref no longer pins the manifest — but every existing short-prefix row declares exactly ONE ref, where "skip this ref" and "abandon the manifest" are indistinguishable. Two refs now: a cleanly-closed short prefix followed by a valid snapshot, asserting the second is still requested, verified and cached while the prefix is not. Under the `continue` -> `break` mutation the pre-existing sibling row still passes and only this one fails, which is the blind spot stated as evidence. 2. Deadline yield. A voluntary local round-budget yield must not be charged to the peer as a timeout. Driven deterministically with fake timers, the first ref resolving from cache and the ready-hook advancing the clock: pins `yieldedAtDeadline`, pre-deadline progress surviving, the abandoned tail counted rather than dropped, and `timedOutPhases: 0` so nothing here can reach `backoffWorthyFailure`. 3. Inline lifecycle seam. Deleting the SWM coverage forwarding in `runCatchupOverPeers` left every suite green. Two peers, not one: with a single peer `+=` is indistinguishable from `=` and last-writer-wins is indistinguishable from selected-whole. The larger short manifest must win whole, so a field-wise reduction synthesizing 200/250 fails the row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two leftovers from replacing the fleet-wide max, both in code the same change touched. A test comment still named `highestResolvedCoverage` — a function that no longer exists — and justified a fixture's wide per-pass margin as load-bearing against a hazard the fix removed. The margin was a dodge around a real defect; now that the gate sums each peer's own high-water it is belt-and-braces, and the comment says so rather than pointing at a deleted symbol. The per-pass log line printed the new cross-peer sum under the old "coverage" wording, directly beside `describeCoverage`, which renders ONE whole record from ONE peer — and whose own doc warns that counts printed without their peer invite being read as a fleet total. That is the same defect class as the `missingCount` mislabel this review round already fixed: a number whose rendering overstates what it measures. Now labelled "progress ... summed across peers". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…an emit Review found `SwmSnapshotCoverage` fixtures annotated with the type but omitting the required `materializationFailures`. Correct, and it is the same class as the T16c fixture fixed earlier this round: a fixture that does not represent producer output cannot catch the producer drifting away from it. Four base literals were short — `partialOfLarge`, `completeSmaller` and `truncatedButLarger` in the agent suite, `r26` in the CLI suite. The three spread-derived records inherit the field once the bases carry it. All four already satisfied `resolved + missing === total`, so adding the counter makes them emittable rather than merely well-typed. Worth recording WHY the annotation did not catch this, because it defeats the suggested remedy too: no tsconfig in this repo typechecks `.test.ts` at all. Both package configs are `include: ["src"]`, and `packages/agent/tsconfig.type-tests.json` covers only `test/**/*.typecheck.ts`. Measured rather than inferred — appending `const x: number = 'definitely not a number';` to this very file is caught by neither config and the suite still runs 56/56 green. So a typed fixture builder would enforce nothing either. The durable fix is to typecheck test sources in CI, which is a repo-wide change with its own baseline and belongs in its own PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…operator Review flagged that the shortfall clause caps how MANY identifiers it names but not what any one of them contains. Confirmed at the source: a ref is a `dkg:publicSnapshotRef` literal chosen by a remote peer, and the parser applies only `.trim()` before it reaches `missingSample` and then the terminal error, which surfaces through the API and the node UI. Two distinct exposures, so two bounds. Renderer — `sanitizeSnapshotRef` folds every C0/C1 control character to U+FFFD and bounds the rendered length with a visible marker. Folding rather than deleting is deliberate: deleting would let a crafted "a\nb" collapse into the real ref "ab" and impersonate it. This is the last point before an operator sees the string, so it holds whichever producer path filled the sample. Mapping after the slice keeps the "(+N more)" arithmetic on the sample COUNT, which sanitising cannot change. Producer — `boundSampledRef` caps each ref as it enters either sample. Capping the sample size bounds how many refs are kept, not how large each is; ten megabyte literals would still cross the worker RPC and sit in the diagnostics record, which the renderer cannot prevent from the far side. Oversized peer literals are a phenomenon this codebase has already met on the sync path. Two rows added, both mutation-tested: bypassing the sanitizer fails them on assertions in 4ms and 1ms — one on a forged newline surviving into the message, one on the overlong literal being rendered whole. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ust lost Found by an adversarial pass over this review round's own fixes, which were unreviewed code. 47bb6a9 narrowed the plane-proven stop so an unrelated peer's clean round could not end the walk while a capable peer still had refs outstanding, and left the structurally identical ungated stop one clause below it. At the pass-1 boundary `coverageHighWaterMark` is 0 by initialization, so `lastPassCoverage <= coverageHighWaterMark` collapses to "the whole walk materialized zero" — a state a CAPABLE peer routinely reports: a store fault that failed every write, or a round whose deadline was spent by the metadata and aggregate phases so the snapshot walk yielded at index 0. Such a peer emits `{resolved: 0, total: 250, manifestComplete: true}`, which is the peer stating it holds 250 refs we lack. It earned ZERO repeats while the terminal message told the operator more passes would not help — and on a warm cache those repeats would have cost no network bytes at all. Suppressed only at that boundary, only while the capable set is non-empty. This does not re-admit the barren-retry the 0-init guards against: a barren peer emits no coverage record at all, and a truncated-manifest peer is excluded by `capablePeersForNextPass`. A non-empty capable set on pass 1 is a peer's own statement that it holds what we lack, not an absence of failure. Later passes stall normally. Mutation-tested at both levels — disabling the suppression fails the unit row and the worker-loop row on assertions in 4ms each. Note for whoever runs these next: the CLI worker loads the policy through the agent's BUILT dist, so the worker row was a FALSE GREEN until `node scripts/build.mjs` ran. Rebuild before trusting any CLI-side result after an agent-side edit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ing forever Second defect found by the adversarial pass over this round's own fixes. `snapshotsTotal` counts refs in the peer's MANIFEST; `snapshotsResolved` counts refs we MATERIALIZED. The two are read by different producers, and a ref can exist for the first and be invisible to the second: `collectPublicSnapshotMetadata` needs only a digest and a count, while `parseGraphScopedSwmRecoveryDescriptors` visits only operation subjects a head row names. Re-sharing a Knowledge Asset leaves the superseded share-operation row behind while the head names the current operation — so that ref stays in the manifest with no descriptor. Such a ref could never enter `materializedRefs`, so `snapshotsResolved < snapshotsTotal` held FOREVER. `capablePeersForNextPass` kept calling the peer capable, and every future catch-up job spent its whole pass budget re-walking a graph that was already complete — at O(KA size) per cached ref, since a cache hit is a full blob read, a digest, a COUNT and a CONSTRUCT. Left alone this would have been the more expensive of the two defects, because it recurs per job rather than once. The early return conflated two states. Missing WIRING means nothing CAN be written and the ref stays unresolved; no DESCRIPTOR under a complete manifest means there is nothing to write, which is resolved by vacuity. Now split, and gated on `manifestComplete` — a truncated meta phase parses no descriptors at all, so "no descriptor" there means "not known yet". The regression row shares one KA twice and keeps only the superseded operation rows, which is the production shape rather than a synthetic one; both payloads are pre-cached so neither ref touches the transport. Reverting the fix fails it on `snapshotsResolved 2 -> 1`, and only that row fails out of 57 — so nothing previously covered this. Known gap, stated rather than implied: when NO ref in a manifest has a descriptor, `onSnapshotReady` is never wired and this path is not reached, so that shape still reports zero resolved. It needs its own row and its own fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… just the parser Review noted that `DKG_SWM_CATCHUP_PASS_BUDGET_MS=0` is documented as the operator kill switch but only the pure parser was tested. Verified: the only test naming those env vars was the agent-side parser suite. The gap is specific and the failure it hides is expensive. Both constants resolve ONCE AT MODULE LOAD in catchup-pass-policy.ts and are then imported by the worker, so a typo'd env name, a broken export, or the worker reading a literal default would pass every existing test — and this is the lever an operator reaches for during an incident, when nobody is in a position to discover it does nothing. Two files rather than one, because `max-passes-reached` is evaluated before `budget-exhausted`: a single file holding both switches would mask the budget row. Each proves the switch behaviourally — one `syncSharedMemory` call and a stop — not merely that a constant parsed. The anti-vacuity control is explicit: the fixture is copied verbatim from the existing row that asserts the SAME peer is contacted TWICE, so the environment variable is the only difference between a fixture that continues and one that stops. Mutation: renaming the env var the policy reads, then rebuilding, fails the behavioural assertion in both files. Reproduced independently after the delegated run. Env is set via `vi.hoisted` with an `afterAll` restore, following the existing `catchup-runner-worker-killswitch.test.ts` precedent, and the restore was verified with a temporary leak probe — an unrestored env write here would silently run later files under the kill switch and green them for the wrong reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The "known gap" recorded in e64f10a is not exotic. It is what the primary shared-memory write path produces, and it is a third round of the same denominator/numerator mismatch. The manifest and the descriptors come from different readers. `collectPublicSnapshotMetadata` accepts any subject carrying `dkg:publicQuadsDigest` + `dkg:publicQuadsCount`; `parseGraphScopedSwmRecoveryDescriptors` anchors only on `#dkg-swm-head` subjects. An entity-level share writes its public slice under a `urn:dkg:public-stage:...` subject with no head row, so a Context Graph written entirely by that path yields manifest refs and NO descriptors at all. The hook was gated on `snapshotDescriptorsByRef.size > 0`, which reads as an optimization and meant `materializeReadySnapshot` never ran — so the vacuity branch added in e64f10a was unreachable in precisely the shape it was written for. Such a graph reported `0/N` snapshots for ever. `capablePeersForNextPass` nominated a peer that owed us nothing on every pass of every catch-up job. 7403331 made it worse rather than better: before it, pass 1 stopped immediately at `coverage-stalled`. After it, that false capable signal is taken as positive evidence and buys a full extra pass — complete metadata and aggregate replay over the wire, plus an O(KA size) blob read and digest per cached ref — to resolve zero again. The two fixes were sound individually and wrong together, which is only visible with both applied. Reproduced against the built dist before and after: `0/1 capable=true continue` becomes `1/1 capable=false plane-proven`. The hook still early-returns when the materializer or the store is absent, so the distinction that matters is preserved and now load-bearing: missing WIRING means nothing can be written and the ref stays unresolved; no DESCRIPTOR under a complete manifest means there is nothing to write and it is resolved. Under the mutation restoring the old guard, exactly one row fails out of 58 — the two rows pinning the other side of that distinction stay green, which is the positive proof it was not weakened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
a04b01b wired the snapshot-ready hook unconditionally so the vacuity rule could be reached. It also made the parse-failure path reachable by that rule, and that combination reported a graph fully materialized while writing nothing. The parse runs INSIDE a block whose entry condition is `wsMetaResult.completed`, so on the failure path `manifestComplete` is guaranteed true exactly where the descriptor map is empty for the wrong reason. Manifest complete AND descriptors never parsed is a third state, distinct from a truncated meta phase and from a genuine entity-share manifest with no head rows — and `manifestComplete` cannot express it. Consequences, all silent: coverage read N/N with zero assertion graphs written; `materializationFailures` stayed 0 because nothing was attempted, so the phase read usable and the bulk `storeInsert(processed.verifiedMeta)` landed head rows certifying assertion graphs that hold nothing — after which `isGraphAssetMaterialized` skips those KAs for good, the same permanent invisibility the G7 repair exists to prevent. The peer was then dropped as satisfied, the loop stopped `plane-proven`, and the shortfall clause rendered nothing at all. Both triggers are production-reachable rather than synthetic: union-insert head residue — the state this branch's own materializer names and repairs, so any peer running the pre-fix code serves it — and a head written at a newer `contentScopeVersion`. Because the parser builds its whole array before returning, ONE malformed head discards the descriptors of every valid Knowledge Asset alongside it. `descriptorsAuthoritative` now gates both the vacuity branch and `snapshotPhaseUsable`. Measured before and after on both shapes: `2/2` and `3/3` with `failedPhases: 0` become `0/2` and `0/3` with `failedPhases: 1`. Two mutations, each landing on its own assertion: dropping the flag from the catch returns full coverage; dropping it from `snapshotPhaseUsable` returns `failedPhases: 0`. Under both, exactly one row fails out of 59 — the three rows pinning the other states stay green, so the four-way distinction is complete rather than incidentally covered. Known diagnostics residual, not fixed here: on this path `missingCount` is non-zero while `missingSample` is empty, because the sample is populated only from the materialization-failure branch and nothing was attempted. The operator gets a count with no named refs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…napshots Partial correction to e50e9e0, which over-reached. With an empty manifest there is no snapshot the metadata could wrongly certify, so `descriptorsAuthoritative` protects nothing there and only discards the round's verified metadata. `recordSnapshotCoverage` also returns early on an empty manifest, so no coverage record exists and the shortfall clause renders nothing — leaving `unreachable` with no statement of what is missing. A Context Graph whose public slices are all graph-backed (`dkg:publicSnapshotGraph`, written when the publisher has no snapshot store) declares zero refs and hit exactly that. Measured: 0 of 8 verified meta rows written with `failedPhases: 1` becomes 8 of 8 with `failedPhases: 0`. REMAINING AND NOT FIXED HERE — see the PR discussion. `descriptorsAuthoritative` is still Context-Graph-wide while the decisions it gates are per-ref. An entity-level share writes a `urn:dkg:public-stage:` slice that has NO head row, so it can never have a descriptor even on a perfectly clean parse; one unrelated malformed head therefore still withholds the whole graph's verified metadata, including the `WorkspaceOperation` and `dkg:rootEntity` rows that bind data which DID land. Measured: 3 entity shares alone give `3/3 resolved, 21 meta triples, failedPhases 0`; the same 3 plus one poisoned head give `0/4, 0 meta triples, failedPhases 1`, permanently, because the parse is deterministic over the peer's served metadata. Fixing that properly means deciding authority per REF, which needs the source subject that `collectPublicSnapshotMetadata` computes and discards — a change to the exported `PublicSnapshotMetadata` shape. Deliberately not attempted in the same pass as the correction above. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
12 tasks
Two defects introduced by making the SWM walk repeatable. Neither is visible in the terminal record - both live in the scalars the daemon route branches on - and neither was covered. 1. deferredBackpressure is a JOB-LEVEL total, and accumulate() runs for continuation rounds too. routes/context-graph.ts short-circuits on `deferredBackpressure > 0 && !denied` BEFORE classification, and that else-branch is the only path to classifyContextGraphCatchupReadiness, the readiness write, markContextGraphSubscriptionState and PROJECT_SYNCED. Its premise - "an incomplete round has no readiness to inspect" - is true when the round was cut short and FALSE when pass 1 completed and only a best-effort extra pass was refused capacity. So a job that fully succeeded could be demoted to `deferred` and have its readiness discarded because an OPTIONAL pass hit local pressure - on exactly the workload #2050 targets, since a large public graph under store pressure is precisely when admission defers. The pass budget's own rationale anticipates this state ("sized so at least two extra passes fit even when a peer's plane is deferred"), so it is designed for, not an edge case. The diagnostic still counts every deferral; only the job-level scalar is gated, so observability is unchanged. 2. deniedPeers was a scalar `+= 1` per round, sitting four lines below the comment explaining why peersTried/peersResponded/peersSucceeded became Sets: once the walk repeats, a peer that denies on every pass is counted once per pass. The agent driver already models this as a set (accessDeniedPeers), so the two drivers disagreed on a field they both return - the drift the review predicted, already realized. Verified by mutation, on disjoint rows, both assertion deaths: - neutralize the gate to `if (true)` -> continuation row dies "expected 1 to be +0"; the MANDATORY-pass control stays green. - revert deniedPeers to the counter -> denial row dies "expected 2 to be 1" in 9ms. Each mutant kills exactly one row, and the mandatory-pass control is what stops the first row passing under a gate on the wrong thing. Test note: a locally deferred plane is RETRIED and only surfaces once CATCHUP_BACKPRESSURE_MAX_WAIT_MS (180s) expires, so the rows stub DKG_CATCHUP_BACKPRESSURE_MAX_WAIT_MS=0. Left at the default they die on the vitest timeout, which would read as a failing assertion while proving nothing. Added to the vitest include allow-list; a file not listed is silently uncollected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
This was referenced Aug 5, 2026
Contributor
Author
Review round: both open threads addressed, plus two defects they surfacedBoth remaining 🟡 comments were valid. Both are deferred to tracked issues — but auditing them found two real defects, now fixed in Fixed here (not raised by either comment)1. A deferred continuation pass could demote an already-successful job. The route's premise — "an incomplete round has no readiness to inspect" — is true when the round was cut short and false when pass 1 completed and only a best-effort extra pass was refused capacity. So the fix could stop a subscribe settling on exactly the workload it exists to fix: a large public graph under store pressure is precisely when admission defers. The pass budget's own rationale anticipates the state ("sized so at least two extra passes fit even when a peer's plane is deferred"). Only the job-level scalar is gated; the diagnostic still counts every deferral, so observability is unchanged. 2. EvidenceBoth proved by mutation, on disjoint rows, both assertion deaths (not timeouts):
The mandatory-pass control is what stops the first row passing under a gate on the wrong thing. Test note: a locally deferred plane is retried and only surfaces once Deferred
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
syncPublicSnapshotsForMetaabandoned every remaining snapshot, and the identity of the missing set — fully computable in memory at that instant — was discarded one stack frame later. Each peer got one round, the walk ran once, and the job terminatedunreachablewith a partial graph and no resume path. Another pass now runs while the job budget allows, coverage is still advancing, and at least one peer demonstrably holds descriptors we lack.MAX_POST_APPROVAL_CURATOR_SYNC_ROUNDS). SWM catch-up can leave a partial unrecovered tail after phase timeouts under backpressure #2050 is that loop missing from the general foreground catch-up. Admission is acquired and released per peer per pass by the existing code, so scheduler fairness is preserved by construction — no coordinator, no manifest freeze, no new admission primitive, no change toordered-sync.tsor the single-flight keys.replaceHeadMetadatais delete-only, and the compensating bulkstoreInsert(processed.verifiedMeta)sits below thecontinueon the failure branch. A partial round therefore deleted head rows for the KAs it had just materialized: content present, heads absent, invisible to every head reader, and permanent — the next round sees the content and skips. Now compensated immediately after every call that deletes a head, inside the existing write lock.snapshotsResolvedcounted refs fetched, not KAs materialized — and that field is read by three consumers: the terminal message, the capability gate, and the coverage high-water mark. A round that cached all N refs and failed every materialization reportedN/N, so the gate computedN < N→ false and dropped the peer while the mark saturated. The walk stopped having written zero KAs, silently disabling the retry loop in exactly the failure class the loop exists for. It presented as a reporting defect and was classified as one.missingCountis now derived from the resolved count, soresolved + missing === totalholds by construction.unreachablesaying nothing about shared memory at all. Stop reasons render through an exhaustiveRecordover a closed union, so a new reason fails the build rather than rendering nothing.Deferred deliberately — please read before approving
replayPhaseBytesReceived/snapshotPhaseBytesReceivedsplit the previously-mergedbytesReceivedscalar. → Frozen manifest — remove metadata and aggregate-data replay per pass #2078syncSchedulerQueueWaitMsis recorded only afterstart()succeeds, and validity gated on comparable rejection counts. In a change that increases admissions, an unlabelled p95 would be unfalsifiable in the flattering direction.runCatchupOverPeersparity is not implemented. All three live callers are already bounded retry loops, so naive parity would nest a loop inside loops that already retry — a defect, not a missing half. That path still propagates every SWM catch-up can leave a partial unrecovered tail after phase timeouts under backpressure #2050 diagnostic; it does not repeat the walk.snapshotsTotal > 0in the capability gate is inert — a defensive restatement its neighbour already enforces at0 < 0. Documented in code as unpinnable-by-construction rather than patched around with an impossible fixture.Related
31be166b8shared-memory-sync-ownership.test.ts· SWM has no chain-anchored inventory — complete is irreducibly peer-relative #2084 no chain-anchored SWM inventory · The public snapshot blob store has no GC #2085 blob-store GCDiagrams
Flow 1 — the snapshot walk when the deadline expires mid-list
Before — one pass per peer; the tail is abandoned and the missing set is discarded:
sequenceDiagram participant Job as Catch-up job participant Walk as syncPublicSnapshotsForMeta participant Peer participant Store as Triple store Job->>Walk: one pass (120s deadline covers meta + data + ALL snapshots) Walk->>Peer: fetch manifest (250 snapshots) loop until the deadline expires Walk->>Peer: fetch snapshot[i] Walk->>Store: materialize KA[i] end Note over Walk: deadline expires at i=178 Walk-->>Job: return (72 snapshots abandoned, identity DISCARDED) Job-->>Job: terminal "unreachable", partial graph, no resume pathAfter — the walk repeats while coverage advances and a peer still holds what we lack:
sequenceDiagram participant Job as Catch-up job participant Walk as syncPublicSnapshotsForMeta participant Peer participant Store as Triple store Job->>Walk: pass 1 Walk->>Peer: fetch manifest (250 snapshots) Walk->>Store: materialize 178 KAs Walk-->>Job: coverage {resolved 178, total 250, missing 72} Job->>Job: shouldRunAnotherCatchupPass -> continue Job->>Walk: pass 2 (shared-memory plane only) Walk->>Peer: re-fetch manifest (accepted replay cost) Walk->>Store: materialize remaining 72 KAs Walk-->>Job: coverage {resolved 250, total 250, missing 0} Job->>Job: no capable peers left -> stop Job-->>Job: terminal "done", complete graphFlow 2 — head metadata on a partial round (the defect not in the issue)
Before — the head is deleted and its rewrite is unreachable:
sequenceDiagram participant Walk as syncPublicSnapshotsForMeta participant Mat as snapshotMaterializer participant Store as Triple store Walk->>Mat: replaceHeadMetadata(KA) Mat->>Store: deleteByPattern(head + operations) Note over Mat,Store: delete-only, no insert Walk->>Store: write assertion graph (content) Note over Walk: snapshot phase incomplete -> continue Walk--xStore: storeInsert(verifiedMeta) NEVER REACHED (below the continue) Note over Store: content present, heads ABSENT<br/>invisible to every head reader, and permanentAfter — each KA's verified metadata is rewritten immediately, inside the same lock:
sequenceDiagram participant Walk as syncPublicSnapshotsForMeta participant Mat as snapshotMaterializer participant Store as Triple store Walk->>Mat: replaceHeadMetadata(KA) Mat->>Store: deleteByPattern(head + operations) Walk->>Store: insertVerifiedDescriptorMeta(KA), same write lock Note over Store: head + operations restored per KA Walk->>Store: write assertion graph (content) Note over Walk: snapshot phase incomplete, continue Note over Store: content AND heads present<br/>the round is durableFiles changed
packages/agent/src/sync/requester/shared-memory-sync.tssnapshotsResolvedcounts materialized refs withmissingCountderived from it; progress carried out through a throw on a non-enumerable, validated-on-read payload; per-KA verified-meta insert at both head-deleting call sites, with aperKaInsertedMetaKeysledger soinsertedMetaTriplesstays byte-identical on a usable round;bytesReceivedsplit into replay vs snapshot phasespackages/agent/src/sync/catchup-pass-policy.tsshouldRunAnotherCatchupPassover a closedCatchupPassDecisionReasonunion, plus budget/max-pass resolvers. Lives inpackages/agentbecausepackages/agenthas no dependency onpackages/clipackages/agent/src/dkg-agent-types.tsSwmSnapshotCoverage, continuation diagnostics, the byte-split fieldspackages/agent/src/dkg-agent-lifecycle.tsrunSharedMemorySynccallerpackages/agent/src/index.tspackages/agent/src/sync/catchup-policy.tspackages/cli/src/catchup-runner-worker-impl.tscapablePeersForNextPass;highestResolvedCoverage; per-pass structured log line; retention of a peer's coverage across a failed round (delete only on a clean round that reported none)packages/cli/src/context-graph-readiness.tsswmShortfallClause; stop reasons rendered through an exhaustiveRecordpackages/cli/src/catchup-runner.tspackages/agent/test/sync-requester-progress.test.tspackages/agent/test/swm-snapshot-materializer.test.tsOxigraphStore— partial-round heads, and the already-materialized repair for an absent or older-version headpackages/agent/test/swm-descriptor-fixtures.tspackages/agent/test/catchup-pass-policy.test.tspackages/agent/test/swm-public-snapshot-materialization.test.tspackages/cli/test/catchup-runner-worker-impl.test.tspackages/cli/test/context-graph-catchup-readiness.test.tspackages/agent/vitest.unit.config.tsincludeallow-list (it is an allow-list; a file not added is silently uncollected)Test plan
packages/agentandpackages/cliunit lanes, as "baseline + exactly N named new tests" rather than "identical to baseline"Attribution guarantee: 20/20 files any chunk touches are green at base (
31be166b8), enumerated by name — 9 CLI, 11 agent. Three agent files the full run never reached were run separately at the same commit in the same pristine worktree (3 passed | 35 passed) rather than assumed. Red in any of those 20 now is a real regression.Mutation set, serial, kill sets predicted in advance. 5 killed, 1 documented survival:
<=→<manifestComplete→truesnapshotsTotal > 0→>= 0materializedRefsForCg→0on the throw path2 → 0); the previous T14 asserted0and would have passedlogPassLineemits nothingexpected [] to have a length of 1)All kills are assertion deaths, not timeouts — 10–21 ms against vitest's 5000 ms minimum, checked because a test that times out under load reads as a killed mutant.
Removing the widened already-materialized repair survived 26/26 rows — not thin coverage, none. Reachability, not assertion strength: both existing rows go through the materialize path while that exit short-circuits earlier. Closed by T9/T9b.
Paired Base Sepolia runs — NOT DONE. Same receiver snapshot and fixture, baseline commit then candidate; exact 250 KA / 30,900 triple parity and terminal
done; scheduler metrics normalized per job and per materialized KA; replay reported separately from useful bytes.Known-red at base, not caused by this PR
Neither lane is green at base on Windows, and this PR does not claim otherwise. There is no whole-lane agent total and none is invented here — the deepest run reached 72 of ~151 files with 52 failures confined to 4 RFC-64 files (
native-gate1.integration29,dkg-agent-native-wiring.integration16,current-head-discovery-v14,transport-v13). No chunk touches any of them.That red is deterministic, not flaky and not contention: gate1 fails 29/29, measured three times — 904 s contended, 941 s alone on a verified-clear box, 965 s. The chain is
EBUSY: resource busy or locked, unlink …inventory-v1.lease.sqlite3→Hook timed out in 10000ms→ every test timing out at its 30 s limit awaitingsetupLiveReceiver(). A teardown file-handle failure, not an assertion, so the four files should be subtracted by name rather than re-measured.This does not mean the branch lost its CI signal. Those files are in the local
test:unitallow-list, not inrfc64-inventory-windows.yml, and #2053'sSQLite lifecycle (Windows)passed on this same base.