Skip to content

fix(sync): promote discovery after partial bootstrap - #2049

Open
branarakic wants to merge 1 commit into
codex/rfc64-m1-live-gate-timeoutfrom
codex/rfc64-m1-partial-bootstrap-discovery
Open

fix(sync): promote discovery after partial bootstrap#2049
branarakic wants to merge 1 commit into
codex/rfc64-m1-live-gate-timeoutfrom
codex/rfc64-m1-partial-bootstrap-discovery

Conversation

@branarakic

Copy link
Copy Markdown
Contributor

User impact

A cold Core can now turn public Context Graph definitions that were already durably received during a partial system bootstrap into bounded automatic VM and SWM coverage. A timeout later in the same bootstrap batch no longer leaves those public graphs invisible until an unrelated future discovery event.

The pressure stop remains intact: the node records the committed discovery state, stops the pressured peer fanout, and lets the existing coverage-epoch reconciler schedule a fresh bounded public round. Edge selection rules, private-CG exclusion, batch size, and concurrency are unchanged.

Live failure reproduced

The anchored M1 testnet run placed all five fresh graph definitions and on-chain bindings in the cold Core store. A large agents transfer later returned a verified prefix with a backoff-worthy timeout. The peer round exited before discoverContextGraphsFromStore, leaving trackedContextGraphs: 0 and producing no public Core VM/SWM work.

Before

sequenceDiagram
  participant P as Sync peer
  participant C as Cold Core
  participant S as Coverage scheduler
  P->>C: Bootstrap ontology plus agents
  C->>C: Commit fresh public definitions
  P--xC: Later agents page times out
  C->>C: Stop before store discovery
  C-->>S: No public admission
  Note over C,S: trackedContextGraphs stays zero
Loading

After

sequenceDiagram
  participant P as Sync peer
  participant C as Cold Core
  participant S as Coverage scheduler
  participant R as Reconciler
  P->>C: Bootstrap ontology plus agents
  C->>C: Commit fresh public definitions
  P--xC: Later agents page times out
  C->>C: Discover committed definitions
  C->>S: Admit public graphs and advance coverage epoch
  C->>C: Stop pressured peer fanout
  R->>S: Plan fresh bounded public batch
  R->>P: Fetch exact public VM and SWM
Loading

Implementation

  • Add a commit-side callback to an ordered sync phase.
  • Run bootstrap store discovery after result accounting even when backpressure or a backoff-worthy transport result stops further fanout.
  • Preserve the existing successful ordering: durable refresh, then discovery, then newly discovered graph work.
  • Keep discovery failures classified as local post-sync failures, so they do not incorrectly grow peer transport backoff.

Validation

  • pnpm exec vitest run packages/agent/test/sync-on-connect-churn.test.ts packages/agent/test/sync-on-connect-retry.test.ts --reporter=dot: 77 passed
  • pnpm --filter @origintrail-official/dkg-agent build: TypeScript, type tests, package-root test passed
  • pnpm test:m1:rfc64-selective-coverage:unit: 82 passed
  • git diff --check: passed

Stack

Base: #2048 (codex/rfc64-m1-live-gate-timeout)

This is the next independently reviewable M1 PR and is not merged into testnet-canary or main.

@branarakic
branarakic force-pushed the codex/rfc64-m1-partial-bootstrap-discovery branch from d12e95f to 1d04c6c Compare August 3, 2026 10:19
return finishSyncAccounting();
}
if (accounting.backoffWorthyFailure) {
if (phase.afterResult) await runNonTransportStep(phase.afterResult);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Bug: Bootstrap discovery now runs after any backoff-worthy aggregate result

What's wrong
The code assumes a backoff-worthy bootstrap result means authoritative discovery records may already be durably committed, but the aggregate result does not prove the Agents/Ontology portion completed. Because discovery reads whatever is currently in the store, a timeout during the ontology graph itself can promote a partial registry snapshot into subscription/admission state.

Example
Bootstrap requests [Agents, Ontology, configured-cg]. If Ontology times out after committing only an early page containing rdf:type/name but before the page containing accessPolicy/registration binding, the aggregate result is backoff-worthy. The new hook still runs discovery, so a core node can record that context graph with incomplete metadata, for example as core-hosting-only instead of automatic-public/private. Expected behavior is to promote discovery only when the ontology discovery records being scanned are known complete enough, or to wait for the retry.

Suggested direction
Gate afterResult on phase-specific completion evidence, split the bootstrap request so Ontology completion is known before later graphs can stop the fanout, or make the durable result report which context graph completed so discovery only runs for a completed ontology prefix.

Confidence note
This depends on whether the durable requester can expose per-context-graph completion for the bootstrap batch; the surrounding requester code shows it can stop after a timeout in any requested graph while still returning already inserted triples.

For Agents
Look at packages/agent/src/sync/on-connect/sync-on-connect.ts and the durable sync result shape from packages/agent/src/sync/requester/durable-sync.ts. Preserve the intended case where Agents/Ontology completed before a later graph timed out, but avoid running discovery when the timeout/backoff occurred before or during Ontology completion. Add a focused test where the bootstrap aggregate has inserted triples plus timedOutPhases from an incomplete ontology phase and prove discovery is not promoted until safe.

* prefix may already contain authoritative discovery records even when a
* later graph in the same phase timed out.
*/
afterResult?: () => Promise<void>;

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: Avoid hiding bootstrap discovery behind a generic phase hook

What's wrong
This turns a very specific lifecycle requirement into a generic optional callback on the shared phase model. The result is more indirect and more fragile: the runner now has duplicated hook calls in multiple branches, while the type gives readers no clear ownership boundary for why this callback exists or which phases may use it.

Example
A future fanout stop branch added to runSyncPhase now has to remember to call phase.afterResult before finishSyncAccounting(). Nothing in the type expresses that this hook is really a bootstrap durable post-accounting step, and shared phases can also provide it even though the documented invariant is durable-prefix discovery.

Suggested direction
Make the bootstrap post-accounting behavior explicit instead of adding a broad afterResult escape hatch. A cleaner shape would compute the phase stop reason once, run a single phase epilogue that knows about durable bootstrap discovery, and then handle the stop outcome, eliminating the three duplicated optional-callback calls.

For Agents
Look in runSyncOnConnectWithScopePlan and keep the current ordering: bootstrap discovery must happen after durable transport accounting and before final peer-sync accounting when the bootstrap phase stops. Refactor the phase runner so the stop decision is computed once, run a named post-accounting epilogue once, then return/log/finish from one place. Add/keep a focused test that proves bootstrap discovery occurs before the peer outcome callback on backoff-worthy pressure.

`Synced ${accounting.insertedTriples} ${phase.label} ${dataLabel} triples from peer ${shortPeer}`,
);
if (accounting.deferredByBackpressure) {
if (phase.afterResult) await runNonTransportStep(phase.afterResult);

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: Deferred-backpressure discovery promotion is not verified

What's wrong
The new behavior is intended to run the bootstrap afterResult hook even when sync fanout stops early. Tests now cover the backoff-worthy timeout branch, but not the separate admission-deferral branch added here. That leaves the deferral half of the behavior unverified, even though it is the path that returns deferred-backpressure and is explicitly changed by this line.

Example
A regression that removes if (phase.afterResult) await runNonTransportStep(phase.afterResult); from the deferred-backpressure branch would still leave the updated tests green. A focused test could return { insertedTriples: 1, insertedDataTriples: 1, completedPhases: 1, checkpointAdvances: 1, deferredBackpressure: 1 } from bootstrap sync and assert discoverContextGraphsFromStore.calls is [[]] before the function returns deferred-backpressure.

Suggested direction
Add a regression assertion for the deferredByBackpressure early-return path, parallel to the new backoff-worthy pressure test.

For Agents
In packages/agent/test/sync-on-connect-retry.test.ts, update one of the deferred-backpressure cases or add a new bootstrap deferred case to use a recorder for discoverContextGraphsFromStore. Preserve the existing assertions that shared-memory fanout does not continue and peer accounting remains correct, and add the assertion proving committed discovery is promoted before returning on local admission deferral.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants