fix(agent): seed selected SWM recovery on cold bootstrap - #2241
Conversation
| this.log.info( | ||
| ctx, | ||
| `VM exact fetch for "${localCgId}" from ${peerId.slice(-8)}: requested=1 fetched=${result.fetchedDataTriples + result.fetchedMetaTriples} inserted=${result.insertedTriples} failed=${result.failedPeers + result.failedPhases} deferred=${result.deferredBackpressure} disposition=${disposition}`, | ||
| if (!isRecoveryCurrent()) return noRecovery(); |
There was a problem hiding this comment.
🔴 Bug: Stale footprint enrichment leaves the VM active-fetch cooldown installed
What's wrong
This branch is inside the new microbatch sizing path after active-fetch admission has already installed a cooldown. Returning noRecovery() leaves that cooldown in place even though the lifecycle is stale. A replacement subscription or rebound for the same Context Graph can then be delayed by the full reconcile sweep interval, which is the state-transition hazard the owner token was added to avoid.
Example
Start VM recovery for cg-1, pass the active-fetch gate, then have isTargetCurrent() become false while enrichVmRecoveryFootprints() is awaiting policy or sizing reads. The function returns at line 5850 with the cooldown still installed, so an immediate replacement recovery for the same CG is suppressed until VM_RECONCILE_SWEEP_INTERVAL_MS expires. Expected behavior: stale lifecycle exits clear their owned cooldown so the replacement lifecycle can run immediately.
Suggested direction
Route this stale branch through staleRecovery() or otherwise centralize post-admission stale cleanup so every stale exit clears only its owned cooldown token.
For Agents
In packages/agent/src/dkg-agent-swm-host.ts, audit all !isRecoveryCurrent() returns after activeFetchCooldownOwner is assigned. Change the post-enrichVmRecoveryFootprints branch to use the same cleanup path as the other stale exits, and prove it with a test where the lifecycle flips stale during footprint enrichment and the next recovery attempt is not blocked by the old cooldown.
| * discriminated by whether the physical attempt had already been admitted; | ||
| * only the pre-admission variant guarantees zero attempt side effects. | ||
| */ | ||
| async executeVmRecoveryBatch(this: DKGAgent, input: { |
There was a problem hiding this comment.
🟡 Issue: VM recovery batching is still embedded in the SWM host god class
What's wrong
The PR creates useful pure helpers, but the hard part remains in the already oversized SWM host class. This worsens the local architecture because one method/class now mixes lifecycle state, admission, curator discovery, provider policy, sizing, selector packing, exact transport, per-UAL reconciliation, and cooldown settlement. That makes the implementation harder to scan and harder to safely extend.
Example
A future change to VM recovery batching has to reason across providerPolicy, enrichVmRecoveryFootprints, planVmRecoveryMicrobatch, executeVmRecoveryBatch, rotation records, and cooldown cleanup inside one host method instead of one focused recovery runner.
Suggested direction
Extract the VM recovery batch coordinator out of SwmHostModeMethods. The host should supply capabilities; a dedicated runner should own provider selection, footprint enrichment, packing, execution, and result merge.
For Agents
Look at packages/agent/src/dkg-agent-swm-host.ts around recoverVmReconcileBatch and the new VM recovery helper modules. Preserve current recovery behavior, but move batch orchestration into a focused vm-recovery runner/service with a narrow host adapter for topology, transport, chain reads, and rotation settlement. Keep tests proving the same batching, stale lifecycle, and cooldown behavior through the new boundary.
| return this.inflight; | ||
| } | ||
|
|
||
| classify(input: { |
There was a problem hiding this comment.
🟡 Issue: Global backpressure now infers feature semantics from source strings
What's wrong
This moves selected RFC-64 recovery knowledge into the generic global backpressure module. The result is an implicit, cross-cutting contract: scheduler behavior depends on a context graph appearing in a WeakMap and on particular source names being classified as recovery. That is brittle and makes the scheduler less reusable.
Example
To know whether an admission can use the reserved slot, a reader must trace the lifecycle config resolver, the selectedRecoveryScopeIds WeakMap, the source string, and the selectedSwmPriority boolean. Adding another selected recovery source would require changing the global scheduler classifier instead of only the caller that owns that feature.
Suggested direction
Export or accept an explicit capacity/reservation claim from callers. Keep RFC-64 selected-scope resolution in the lifecycle/RFC layer, and let backpressure remain a feature-neutral admission queue.
For Agents
Inspect packages/agent/src/sync/backpressure.ts and the callers passing selected SWM / VM recovery work through withGlobalSyncBackpressure. Preserve the current reservation behavior, but make capacity intent explicit at the admission boundary, then simplify the scheduler to count declared claims rather than infer feature semantics from config and source strings.
| * but resumptions created by the catalog bootstrap use this dedicated entry | ||
| * point so disabling broad sync does not disable selected recovery. | ||
| */ | ||
| export async function runSelectedSharedMemoryRetry( |
There was a problem hiding this comment.
🟡 Issue: Selected SWM retry duplicates the on-connect protocol/accounting workflow
What's wrong
The new dedicated retry entry point is structurally close to a subset of the existing on-connect orchestrator. That buys isolation, but it also creates two places with the same lifecycle/accounting rules, which is a maintainability risk in an already branch-heavy sync path.
Example
If the no-sync protocol handling or selected shared-memory accounting rule changes, it now has to be updated in both runSelectedSharedMemoryRetry and runSyncOnConnect; otherwise the selected retry path and normal on-connect path drift.
Suggested direction
Factor the common selected-lane and accounting pieces instead of maintaining a second mini-orchestrator. The selected retry boundary can stay narrow without copying the workflow internals.
For Agents
Look at packages/agent/src/sync/on-connect/sync-on-connect.ts. Preserve the selected-only retry behavior, but extract shared helpers for protocol gating, selected shared-memory lane execution, and accounting finalization, or model selected retry as the same ordered-plane orchestrator with durable/discovery/ordinary-SWM planes disabled.
| ); | ||
| }); | ||
|
|
||
| it('does not reseed a plane-proven SWM provider on periodic bootstrap refresh', async () => { |
There was a problem hiding this comment.
🟡 Issue: Extract the repeated RFC-64 bootstrap receiver setup
What's wrong
The PR expands an already 3k-line integration spec with several near-copy-pasted setup blocks. That makes the important admission-state differences harder to see and increases the cost of future config changes because each scenario must be edited in lockstep.
Example
The test bodies beginning at does not reseed..., re-admits an incomplete..., and re-admits the same provider... all recreate nearly the same receiver with syncOnConnectEnabled: false, syncReconcilerEnabled: false, syncContextGraphs, agentProfileHeartbeatMs: 0, rfc64CatalogDeploymentProfile, bootstrap config, connectToPeerId spy, queue spy, start(), and whenRfc64PublicCatalogBootstrapIdleV1() before reaching the scenario-specific assertions.
Suggested direction
Pull the repeated receiver/policy/bootstrap setup into a small helper so these tests read as distinct admission scenarios instead of another block of copied integration scaffolding.
For Agents
Extract a local fixture such as createBootstrapReceiver({ name, policies, providerPeerId, syncContextGraphs, retryIntervalMs }) in rfc64-dkg-agent-native-wiring.integration.test.ts or a nearby helper. Keep the existing assertions and behavior, but make the three new scenario bodies express only the state transition under review.
2 participants
Outcome
An RFC-64-selected public Context Graph now starts native SWM recovery on a cold Edge even when broad
syncOnConnectis disabled. A dedicated admission owner tracks provider plus the canonical selected-CG scope and a monotonic transfer generation. Incomplete scopes remain retryable, exact terminal scopes stop periodic reseeding, runtime scope growth is re-admitted, and an older in-flight completion cannot erase newer selected work.This is stacked on #2215 and changes only the selected RFC-64 bootstrap lane. Ordinary sync-on-connect behavior remains unchanged.
Before
sequenceDiagram participant Edge as Cold Edge participant Bootstrap as RFC-64 bootstrap participant Scheduler as Sync scheduler participant Provider as Selected SWM provider Bootstrap->>Provider: Connect pinned provider Bootstrap->>Scheduler: Queue selected retry Scheduler->>Scheduler: Retry marker is absent Scheduler-->>Bootstrap: Reject admission Note over Edge: Broad sync is disabled, so selected SWM stays emptyAfter
sequenceDiagram participant Edge as Cold Edge participant Bootstrap as RFC-64 bootstrap participant Admission as Selected SWM admission participant Scheduler as Sync scheduler participant Provider as Selected SWM provider Bootstrap->>Provider: Connect pinned provider Bootstrap->>Admission: Request provider and selected CG scope Admission->>Scheduler: Admit first or incomplete scope Scheduler->>Provider: Pull selected SWM scope Provider-->>Edge: Verified metadata and snapshot pages Scheduler->>Admission: Mark exact generation terminal Bootstrap->>Admission: Request same terminal scope later Admission-->>Bootstrap: Suppress duplicate pull Note over Admission: Expanded scope is admitted as new workVerification
48d2797ea7083f576b9e5fb45e7001dd8315e24egit diff --check: cleanThe focused regressions cover first admission, incomplete re-admission, exact-terminal suppression, clean no-work, runtime
trackSyncContextGraphscope growth for the same provider, and stale in-flight completion ownership.Live DKG Testnet
Validated exact head
48d2797eon public CG 302 through the actual DKG Testnet transport with generic sync-on-connect, shared-memory-on-connect, and system-graph sync disabled:The complete release gate still requires the separate strict remote-laptop participant run; this PR's bounded single-receiver Testnet gate is green.