fix(agent): data sync no longer unions a reconnecting member's shared memory#1191
fix(agent): data sync no longer unions a reconnecting member's shared memory#1191branarakic wants to merge 2 commits into
Conversation
… memory
parseAndFilterNQuads over-accepted _shared_memory quads under the CG prefix and
the data path blind-inserted them (no per-subject REPLACE), so a member holding
v1 that re-synced a graph now at v3 ended up with the union {v1,v3}. Graph-gate
the filter: data requests exclude _shared_memory; a _shared_memory request is a
no-op (per-KA SWM subgraph sync unchanged). SWM convergence stays owned by the
SWM sync path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // cannot truncate a root spanning a page boundary. The literal "/_shared_memory" | ||
| // matches every SWM URI form and only those (validateSubGraphName forbids a | ||
| // leading "_" on subgraph names, so no data/meta/context graph can collide). | ||
| const swmRequest = graphUri.includes('/_shared_memory'); |
There was a problem hiding this comment.
🟡 Issue: includes('/_shared_memory') is broader than the comment implies. The repo already has non-live SWM graph shapes like .../_shared_memory_snapshots/.../_shared_memory, and this substring check will classify those as SWM too. That makes the new filter depend on an incidental substring instead of the actual root/meta/per-KA SWM URI shapes. Consider a shape-aware predicate here (exact /_shared_memory, exact /_shared_memory_meta, or the existing descendant helper for .../_shared_memory/{addr}/{n}) so future reserved graphs under the same prefix do not get pulled into this branch accidentally.
| const swmRequest = graphUri.includes('/_shared_memory'); | ||
| return { | ||
| quads: quads.filter((q) => q.graph === graphUri || q.graph.startsWith(cgUriPrefix)), | ||
| quads: quads.filter( |
There was a problem hiding this comment.
🟡 Issue: this changes requester-side sync admission in a correctness-sensitive path, but the PR doesn't add regression coverage for the new boundary. A focused SyncVerifyWorker.parseAndFilter test with a mixed page (durable graphs plus _shared_memory graphs, including descendant .../_shared_memory/{addr}/{n} forms) would make it much safer to maintain and would catch accidental broadening/narrowing of this filter.
Export parseAndFilterNQuads and pin the union-corruption guard: a durable DATA/meta
sync request drops _shared_memory quads (so a streaming apply can't union a
reconnecting private-CG member into {old,new}), while a shared-memory request is a
no-op that keeps the base bucket + per-KA subgraphs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| // cannot truncate a root spanning a page boundary. The literal "/_shared_memory" | ||
| // matches every SWM URI form and only those (validateSubGraphName forbids a | ||
| // leading "_" on subgraph names, so no data/meta/context graph can collide). | ||
| const swmRequest = graphUri.includes('/_shared_memory'); |
There was a problem hiding this comment.
🔴 Bug: graphUri.includes('/_shared_memory') (and the matching check on line 254) also matches the context-graph ID prefix itself. validateContextGraphId currently allows IDs like 0xabc/_shared_memory-demo, so a durable sync for that CG would be misclassified as SWM and either keep SWM quads or drop legitimate sibling graphs incorrectly. Please detect SWM from the graph suffix/path after did:dkg:context-graph:${contextGraphId} instead of a raw substring match, and add a regression test for a CG id containing /_shared_memory.
|
Closing as superseded — the durable-sync-union bug this PR fixed is covered in main: the responder-side fix landed as |
1 participant
What
Fixes a shared-memory corruption on reconnect: the general data sync over-accepted
_shared_memoryquads and blind-inserted them, so a reconnecting member that already held a valuev1and re-synced a graph now atv3ended up holding the union{v1, v3}instead ofv3.Root cause
parseAndFilterNQuads(sync-verify-worker-impl.ts) accepted any quad under the CG URI prefix. For a data request that swept in the CG's_shared_memorygraphs and the data pathstoreInsert-ed them additively (no per-subject REPLACE), unioning stale and fresh state.Fix
Graph-gate the filter: a data request excludes
_shared_memorygraphs; a_shared_memoryrequest is a no-op (reduces to the original filter), so per-KA SWM subgraph sync is unchanged. SWM convergence is owned by the SWM sync path (per-subject REPLACE), not the data path.Scope / risk
One-file, ~19 lines. No-op for SWM requests (verified: a KA-published value survives reconnect); only data requests change, and only to stop sweeping SWM. Standalone — independent of the recovery/convergence work that consumes it.
🤖 Generated with Claude Code