Skip to content

fix(rfc64): settle SWM only after VM commit #2040

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
branarakic wants to merge 2 commits into
base: codex/rfc64-m1-edge-periodic-scope-plan
Choose a base branch
from
Open

fix(rfc64): settle SWM only after VM commit #2040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fcc8c21
fix(rfc64): settle SWM only after VM commit
Aug 3, 2026
6d75cc0
fix(rfc64): centralize SWM settlement ordering
Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 16 additions & 25 deletions packages/agent/src/dkg-agent-lifecycle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5723,36 +5723,27 @@ export class LifecycleSyncMethods extends DKGAgentBase {
const graphManager = new GraphManager(this.store);
await graphManager.ensureContextGraph(contextGraphId);
},
// Everything needed to materialize verified public SWM snapshots,
// as ONE dependency (a loose optional trio allowed a silent
// half-configured mode). Graph-scoped (contentScopeVersion 2) KAs
// carry no dkg:rootEntity, so the aggregate data phase returns 0
// data quads for them by design — their content arrives as
// immutable snapshots, and without this the catch-up lane cached
// every verified snapshot and never wrote one to the store.
// Thin wiring only: the materialization policy (content-digest
// guard, MAX head read + duplicate repair, atomic replace, head
// metadata swap) lives in `swm-snapshot-materializer.ts`. What
// the agent contributes here is its own resources — the store,
// the SAME lock map injected into SharedMemoryHandler (sharing
// the map + key helper is what closes the check-then-replace
// race with gossip), and list-cache invalidation.
// Graph-scoped (contentScopeVersion 2) KAs carry no
// dkg:rootEntity, so their content arrives as immutable
// snapshots rather than aggregate data quads. Materialization
// and post-commit retirement remain explicit dependencies so the
// sync coordinator owns their required ordering.
snapshotMaterializer: createSharedMemorySnapshotMaterializer({
store: this.store,
writeLocks: this.writeLocks,
invalidateListContextGraphsCache: () => this.invalidateListContextGraphsCache(),
settleGraphScopedSnapshot: async (contextGraphId, descriptor) => {
await this.getOrCreateFinalizationHandler()
.retireSyncedGraphScopedSwmIfFinalized({
contextGraphId,
ual: descriptor.kaUal,
assertionVersion: descriptor.assertionVersion,
...(descriptor.subGraphName
? { subGraphName: descriptor.subGraphName }
: {}),
}, ctx);
},
}),
settleGraphScopedSnapshot: async (contextGraphId, descriptor) => {
await this.getOrCreateFinalizationHandler()
.retireSyncedGraphScopedSwmIfFinalized({
contextGraphId,
ual: descriptor.kaUal,
assertionVersion: descriptor.assertionVersion,
...(descriptor.subGraphName
? { subGraphName: descriptor.subGraphName }
: {}),
}, ctx);
},
storeInsert: async (quads) => {
// Oversize guard (OT-RFC-56): drop+tombstone protocol-violating
// literals BEFORE insert so the SWM page cursor advances instead
Expand Down
196 changes: 108 additions & 88 deletions packages/agent/src/finalization-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1202,6 +1202,7 @@ export class FinalizationHandler {
blockNumber: verifiedBlockNumber,
txIndex: verifiedTxIndex,
};
let outcome: 'already-confirmed' | 'applied' | 'preserved-metadata' | 'stale' | undefined;
if (vmVerification.status === 'verified') {
const metadataState = await this.graphScopedMetadataState({
contextGraphId,
Expand All @@ -1217,46 +1218,50 @@ export class FinalizationHandler {
subGraphName,
});
if (metadataState === 'matching') {
await this.markMatchingGraphScopedSwmFinalized({
contextGraphId,
scope,
merkleRoot: msg.kcMerkleRoot,
subGraphName,
ctx,
});
this.markProcessed(dedupeKey);
this.log.info(ctx, `Finalization: graph-scoped KA ${scope.ual} is already confirmed`);
return 'already-confirmed';
outcome = 'already-confirmed';
}
}
if (!outcome) {
outcome = await this.applyVerifiedGraphScopedFinalization({
contextGraphId,
scope,
verifiedQuads: layerVerification.quads,
head,
privateMerkleRoot,
computedMerkleRoot: layerVerification.merkleRoot,
publisherAddress: msg.publisherAddress,
txHash: msg.txHash,
blockNumber: verifiedBlockNumber,
batchId,
authorAddress: verifiedAuthorAddress,
materializedVersion,
accessPolicy,
allowedPeers,
subGraphName,
source: 'finalization',
contentAlreadyMaterialized: vmVerification.status === 'verified',
ctx,
});
}
if (outcome === 'stale') {
this.markProcessed(dedupeKey);
this.log.info(ctx, `Finalization: newer graph-scoped assertion already materialized for ${scope.ual}`);
return 'already-confirmed';
}

const outcome = await this.applyVerifiedGraphScopedFinalization({
await this.markMatchingGraphScopedSwmFinalized({
contextGraphId,
scope,
verifiedQuads: layerVerification.quads,
head,
privateMerkleRoot,
computedMerkleRoot: layerVerification.merkleRoot,
publisherAddress: msg.publisherAddress,
txHash: msg.txHash,
blockNumber: verifiedBlockNumber,
batchId,
authorAddress: verifiedAuthorAddress,
materializedVersion,
accessPolicy,
allowedPeers,
merkleRoot: msg.kcMerkleRoot,
subGraphName,
source: 'finalization',
contentAlreadyMaterialized: vmVerification.status === 'verified',
ctx,
});
if (outcome === 'stale') {
this.markProcessed(dedupeKey);
this.log.info(ctx, `Finalization: newer graph-scoped assertion already materialized for ${scope.ual}`);
return 'already-confirmed';
}

this.markProcessed(dedupeKey);
if (outcome === 'already-confirmed') {
this.log.info(ctx, `Finalization: graph-scoped KA ${scope.ual} is already confirmed`);
return 'already-confirmed';
}
this.log.info(
ctx,
`Finalization: promoted graph-scoped KA ${scope.ual} (${publicTripleCount} public, ${privateTripleCount} private)`,
Expand Down Expand Up @@ -1660,19 +1665,10 @@ export class FinalizationHandler {
subGraphName,
});
if (vmVerification.status === 'verified') {
// A cold join can receive the durable VM snapshot and its still-present
// SWM recovery snapshot from different catch-up planes without ever
// seeing the live finalization envelope. Exact VM content plus the
// chain-resolved root is sufficient to retire only the matching current
// SWM head from the user-facing view. This does not synthesize confirmed
// transaction metadata: provenance repair remains pending below.
await this.markMatchingGraphScopedSwmFinalized({
contextGraphId,
scope,
merkleRoot,
subGraphName,
ctx,
});
let acceptedOutcome:
| 'stale-target'
| 'already-confirmed'
| 'verified-vm-metadata-pending';
const access = resolveGraphScopedAccessEnvelope(
head,
trustedAssertionEvidence?.accessPolicy,
Expand All @@ -1697,9 +1693,8 @@ export class FinalizationHandler {
materializedVersion,
});
this.log.info(ctx, `Chain-reconcile: ${ual} already has exact VM content and metadata`);
return preserveNewerWorkspaceLifecycle ? 'stale-target' : 'already-confirmed';
}
if (!trustedAssertionEvidence && access.accessPolicy !== 'ownerOnly') {
acceptedOutcome = preserveNewerWorkspaceLifecycle ? 'stale-target' : 'already-confirmed';
} else if (!trustedAssertionEvidence && access.accessPolicy !== 'ownerOnly') {
const failClosedMetadataState = await this.graphScopedMetadataState({
contextGraphId,
scope,
Expand All @@ -1721,51 +1716,77 @@ export class FinalizationHandler {
ctx,
`Chain-reconcile: ${ual} retains fail-closed access without assertion evidence`,
);
return 'already-confirmed';
acceptedOutcome = 'already-confirmed';
} else {
// No exact metadata is accepted. Without assertion-bound provenance
// the receiver may retire only the matching SWM recovery copy; it
// must leave confirmed metadata explicitly pending.
this.log.info(
ctx,
`Chain-reconcile: exact VM metadata for ${ual} cannot be repaired without `
+ 'transaction provenance; deferring',
);
acceptedOutcome = 'verified-vm-metadata-pending';
}
}
if (!trustedAssertionEvidence) {
} else if (!trustedAssertionEvidence) {
// A cold join can receive the durable VM snapshot and its still-present
// SWM recovery snapshot from different catch-up planes without ever
// seeing the live finalization envelope. Exact VM content plus the
// chain-resolved root retires only the matching current SWM head while
// transaction provenance remains explicitly pending.
this.log.info(
ctx,
`Chain-reconcile: exact VM metadata for ${ual} cannot be repaired without `
+ 'transaction provenance; deferring',
);
return 'verified-vm-metadata-pending';
acceptedOutcome = 'verified-vm-metadata-pending';
} else {
// A confirmed publish may have committed the exact VM graph before its
// graph-scoped metadata survived a crash. Reapply only the metadata
// tail. A failed write throws before the single retirement guard below,
// so the SWM recovery copy stays visible.
const outcome = await this.applyVerifiedGraphScopedFinalization({
contextGraphId,
scope,
verifiedQuads: vmVerification.quads,
head,
privateMerkleRoot,
computedMerkleRoot: vmVerification.merkleRoot,
publisherAddress: evidencePublisherAddress,
txHash: trustedAssertionEvidence.transactionHash,
blockNumber: evidenceBlockNumber,
batchId: kaId,
authorAddress: evidenceAuthorAddress,
materializedVersion,
accessPolicy: trustedAssertionEvidence.accessPolicy,
allowedPeers: trustedAssertionEvidence.allowedPeers,
subGraphName,
source: 'chain-reconcile',
contentAlreadyMaterialized: true,
ctx,
});
if (outcome === 'stale') return 'stale-target';
if (outcome === 'preserved-metadata') {
this.log.info(
ctx,
`Chain-reconcile: retained confirmed metadata for an older same-root assertion ${ual}`,
);
} else {
this.log.info(ctx, `Chain-reconcile: exact VM graph already matches ${ual}; repaired metadata`);
}
acceptedOutcome = preserveNewerWorkspaceLifecycle ? 'stale-target' : 'already-confirmed';
}
// A confirmed publish may have committed the exact VM graph before its
// graph-scoped metadata survived a crash. Reapply only the metadata tail:
// SWM writers use a different lock, so this recovery path must not delete
// a potentially newer staged assertion.
const outcome = await this.applyVerifiedGraphScopedFinalization({

// One policy guard for every accepted exact-VM branch. Any metadata or
// ordering write above must succeed before the SWM copy can be hidden.
await this.markMatchingGraphScopedSwmFinalized({
contextGraphId,
scope,
verifiedQuads: vmVerification.quads,
head,
privateMerkleRoot,
computedMerkleRoot: vmVerification.merkleRoot,
publisherAddress: evidencePublisherAddress,
txHash: trustedAssertionEvidence.transactionHash,
blockNumber: evidenceBlockNumber,
batchId: kaId,
authorAddress: evidenceAuthorAddress,
materializedVersion,
accessPolicy: trustedAssertionEvidence?.accessPolicy,
allowedPeers: trustedAssertionEvidence?.allowedPeers,
merkleRoot,
subGraphName,
source: 'chain-reconcile',
contentAlreadyMaterialized: true,
ctx,
});
if (outcome === 'stale') return 'stale-target';
if (outcome === 'preserved-metadata') {
this.log.info(
ctx,
`Chain-reconcile: retained confirmed metadata for an older same-root assertion ${ual}`,
);
return preserveNewerWorkspaceLifecycle ? 'stale-target' : 'already-confirmed';
}
this.log.info(ctx, `Chain-reconcile: exact VM graph already matches ${ual}; repaired metadata`);
return preserveNewerWorkspaceLifecycle ? 'stale-target' : 'already-confirmed';
return acceptedOutcome;
}

const swmVerification = await this.verifyExactGraphScopedLayer({
Expand Down Expand Up @@ -1836,6 +1857,13 @@ export class FinalizationHandler {
ctx,
});
if (outcome === 'stale') return 'stale-target';
await this.markMatchingGraphScopedSwmFinalized({
contextGraphId,
scope,
merkleRoot,
subGraphName,
ctx,
});
if (outcome === 'preserved-metadata') {
this.log.info(
ctx,
Expand Down Expand Up @@ -2035,15 +2063,7 @@ export class FinalizationHandler {
}
return 'applied' as const;
});
if (outcome !== 'applied') return outcome;

await this.markMatchingGraphScopedSwmFinalized({
contextGraphId,
scope,
merkleRoot: computedMerkleRoot,
subGraphName,
ctx,
});
if (outcome === 'stale' || outcome === 'preserved-metadata') return outcome;

this.eventBus?.emit(DKGEvent.MEMORY_GRAPH_CHANGED, {
contextGraphId,
Expand Down
38 changes: 18 additions & 20 deletions packages/agent/src/sync/requester/shared-memory-sync.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,19 +68,16 @@ interface SharedMemorySyncContext {
}>;
ensureContextGraph: (contextGraphId: string) => Promise<void>;
storeInsert: (quads: Quad[]) => Promise<void>;
/** Store adapter for verified public SWM snapshots. */
snapshotMaterializer?: SharedMemorySnapshotMaterializer;
/**
* Everything needed to MATERIALIZE verified public SWM snapshots into the
* triple store, as ONE cohesive dependency — the contract (and the
* production implementation) live in `swm-snapshot-materializer.ts`.
*
* Why it exists at all: contentScopeVersion-2 KAs carry no dkg:rootEntity,
* so the aggregate data phase legitimately returns 0 data quads for them —
* their content travels as immutable snapshots. The catch-up lane fetched
* and VERIFIED those snapshots and then never wrote them, so a node that
* missed the live gossip stayed empty forever ("0 data + N meta triples").
* Absent entirely => materialization is skipped (never half-applied).
* Post-commit policy for a materialized snapshot. It runs only after the
* verified metadata insert succeeds and outside the per-KA write lock.
*/
snapshotMaterializer?: SharedMemorySnapshotMaterializer;
settleGraphScopedSnapshot?: (

@otReviewAgent otReviewAgent Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: The split settlement dependency is not verified against the half-configured case

What's wrong
This change moves post-commit settlement out of SharedMemorySnapshotMaterializer, but the new callback is optional and the coordinator no-ops when it is missing. The added tests validate the desired ordering only in harnesses that provide the callback, so they would not fail if a production or future caller materialized verified public SWM snapshots without retiring the finalized recovery copy.

Example
A caller supplies snapshotMaterializer and publicSnapshotStore so a verified graph-scoped snapshot is materialized, but omits settleGraphScopedSnapshot. runSharedMemorySync still inserts metadata, records phase completion, and never calls retireSyncedGraphScopedSwmIfFinalized, so the missing post-commit retirement is not caught by the current tests.

Suggested direction
Either make settlement a required dependency whenever snapshot materialization is enabled, or add a focused test proving that omitting it fails or leaves the phase incomplete instead of silently checkpointing.

Confidence note
I did not run the test suite in the read-only sandbox, but the diff and surrounding tests show the coordinator only exercises settlement when a callback is provided.

For Agents
Look at runSharedMemorySync in packages/agent/src/sync/requester/shared-memory-sync.ts and its snapshot materialization tests. Preserve the new ordering where settlement runs after verified metadata insertion and outside the KA lock, but add a regression test or contract check for the half-configured case: materializer present with settled descriptors but no settleGraphScopedSnapshot should not silently complete as successful materialization.

contextGraphId: string,
descriptor: GraphScopedSwmRecoveryDescriptor,
) => Promise<void>;
publicSnapshotStore?: WorkspacePublicSnapshotStore;
getRegisteredSubGraphNames?: (contextGraphId: string) => Promise<readonly string[]>;
getExcludedSubGraphNames?: (contextGraphId: string) => Promise<readonly string[]>;
Expand Down Expand Up @@ -119,6 +116,7 @@ export async function runSharedMemorySync(context: SharedMemorySyncContext): Pro
ensureContextGraph,
storeInsert,
snapshotMaterializer,
settleGraphScopedSnapshot,
publicSnapshotStore,
getRegisteredSubGraphNames,
getExcludedSubGraphNames,
Expand Down Expand Up @@ -468,15 +466,15 @@ export async function runSharedMemorySync(context: SharedMemorySyncContext): Pro
summary.insertedTriples += processed.verifiedMeta.length;
summary.insertedMetaTriples += processed.verifiedMeta.length;
}
// Deliberately outside the per-KA snapshot lock above: the finalizer
// acquires the same lock before stamping the local retirement marker.
// Running settlement inside the materializer critical section would
// deadlock. The handoff remains part of the ONE required materializer
// contract, so production cannot wire materialization without it.
await snapshotMaterializer?.settleCommittedSnapshots(
pid,
[...settledDescriptors.values()],
);
// Deliberately after the verified metadata insert and outside the per-KA
// snapshot lock above: the finalizer acquires that same lock before
// stamping the local retirement marker. A failed insert therefore keeps
// the recovery snapshot visible and cannot deadlock settlement.
if (settleGraphScopedSnapshot) {

@otReviewAgent otReviewAgent Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Snapshot materialization can now be wired without its required settlement step

What's wrong
This PR reintroduces two independent optional knobs for one protocol. The code comment says settlement is post-commit policy for materialized snapshots, but the type system no longer enforces that relationship, so readers and future callers must remember a hidden invariant instead of being guided by the API.

Example
A future caller or test harness can pass snapshotMaterializer and publicSnapshotStore but omit settleGraphScopedSnapshot; the sync path will still write verified snapshots and metadata, then silently skip post-commit retirement. That is exactly the half-wired mode the old cohesive materializer contract was designed to prevent.

Suggested direction
Keep the ordering outside the lock, but model materialization plus post-commit settlement as one dependency again. A small GraphScopedSnapshotCommitter/commitSettledSnapshots abstraction could own the full protocol while still exposing an internal store adapter for the lock-bound writes.

For Agents
Look at SharedMemorySyncContext, runSharedMemorySync, and createSharedMemorySnapshotMaterializer. Preserve the current ordering requirement: materialize under the per-KA lock, insert verified metadata, then settle outside the lock. Prove the contract cannot be half-wired, ideally with a type-level/harness case where materialization-enabled sync must also provide settlement.

for (const descriptor of settledDescriptors.values()) {
await settleGraphScopedSnapshot(pid, descriptor);
}
}
recordPhaseOutcome(wsMetaResult);
recordPhaseOutcome(wsDataResult);
if ((wsMetaResult.timedOut || wsDataResult.timedOut) && shouldStopAfterBackoffWorthyFailure(pid, 'phase timeout')) {
Expand Down
Loading
Loading