RFC-64 M1 6/7: expose bounded runtime coverage evidence - #2016
RFC-64 M1 6/7: expose bounded runtime coverage evidence#2016branarakic wants to merge 14 commits into
Conversation
69c1dec to
072249e
Compare
fe9d92c to
175f486
Compare
072249e to
b166ef8
Compare
175f486 to
0b053ba
Compare
otReviewAgent
left a comment
There was a problem hiding this comment.
Operational Notice: Review Agent could not complete this review.
Business logic reviewer failed: retry_exhausted
8be9d57 to
13759eb
Compare
e7cb778 to
785b434
Compare
13759eb to
5793aed
Compare
d9a70d4 to
e176c85
Compare
otReviewAgent
left a comment
There was a problem hiding this comment.
Operational Notice: Review Agent could not complete this review.
Verification reviewer failed: retry_exhausted
otReviewAgent
left a comment
There was a problem hiding this comment.
Operational Notice: Review Agent could not complete this review.
Business logic reviewer failed: retry_exhausted
5793aed to
6992dd3
Compare
otReviewAgent
left a comment
There was a problem hiding this comment.
Operational Notice: Review Agent could not complete this review.
Business logic reviewer failed: retry_exhausted
e176c85 to
f123856
Compare
otReviewAgent
left a comment
There was a problem hiding this comment.
Operational Notice: Review Agent could not complete this review.
Business logic reviewer failed: retry_exhausted
| } | ||
|
|
||
| async attemptSyncFromPeerWithReconcilerAccounting( | ||
| protected async attemptSyncFromPeerWithReconcilerAccounting( |
There was a problem hiding this comment.
🔴 Bug: Changing the reconciler sync method to protected breaks typed callers
What's wrong
The PR changes an existing method on the exported agent class from public to protected. Any TypeScript consumer or subclass-adjacent integration that used this method as part of the agent lifecycle surface will be blocked at compile time by this release.
Example
A typed integration that previously did await agent.attemptSyncFromPeerWithReconcilerAccounting(peer, probe) will now fail to compile because the method is only accessible inside the class hierarchy, even though the runtime method still exists.
Suggested direction
Keep the method public for compatibility and hide only the new automatic trigger token behind an internal overload/helper, or introduce a public wrapper with the old call shape before making the internal implementation protected.
Confidence note
This is a TypeScript API compatibility issue: JavaScript callers can still reach the method at runtime, but typed consumers of the exported DKGAgent surface can no longer call it.
For Agents
Look at LifecycleSyncMethods in packages/agent/src/dkg-agent-lifecycle.ts. Preserve the previous public callable surface, or add a public compatibility wrapper that keeps the old signature while routing internal evidence-triggered calls through the new path. A type-level compatibility test should prove existing public calls still compile.
| * enabled, the caller must present a recognized token that is not bound to an | ||
| * agent identity; missing, unknown, and agent-scoped tokens all fail closed. | ||
| */ | ||
| export function isNodeAdminCaller(ctx: NodeAdminAuthContext): boolean { |
There was a problem hiding this comment.
🟡 Issue: Node-admin auth was only partially centralized
What's wrong
The new helper is a good direction, but landing it without migrating the existing duplicate gate leaves the daemon with two implementations of the same security boundary. That is architectural drift: future maintainers will not know which one is authoritative.
Example
Changing the node-admin rule later would require finding both isNodeAdminCaller(ctx) users and the local closure in context-graph.ts; missing one route would leave different node-wide diagnostics with different authorization semantics.
Suggested direction
Since this PR creates a canonical helper, route the existing node-wide context-graph subscription gates through it as well. Otherwise this extraction is a partial abstraction that reduces duplication in two routes while leaving a second source of truth in place.
Confidence note
This is a maintainability concern rather than a behavior defect; the duplicated helper may currently be equivalent, but the new helper’s name/comment make it look canonical.
For Agents
Use the new helper consistently across daemon routes that enforce the same node-admin boundary. Start with packages/cli/src/daemon/routes/context-graph.ts and replace the local closure with the shared helper while preserving response bodies and existing agent-scoped denial behavior.
| result: Object.freeze({ ...outcome.result } satisfies SharedMemoryContextGraphResult), | ||
| }; | ||
| } | ||
| if (outcome.disposition === 'deferred') { |
There was a problem hiding this comment.
🟡 Issue: Deferred shared-memory terminals are not verified at the lifecycle boundary
What's wrong
The new public detailed result is supposed to expose exact per-CG terminal state. The tests validate the generic scheduler can emit a deferred outcome, but they do not prove the lifecycle conversion returns the expected shared-memory terminal entries for a real admission deferral.
Example
For a shared-memory request over ['first', 'second', 'third'] where second throws SyncBackpressureBusyError, the detailed result should include first as settled, second as deferred, and third as skipped with reason: 'prior-deferral'. Today that mapping could be dropped or mislabeled without failing the lifecycle test.
Suggested direction
Assert the detailed terminal contract on a real syncSharedMemoryFromPeerDetailed backpressure-deferral scenario, not only the generic ordered scheduler outcome.
Confidence note
Based on diff and nearby tests; the generic scheduler outcome is tested, but the shared-memory detailed-result mapping is not exercised for this branch.
For Agents
Extend the existing preserves completed shared-memory progress when a later admission is deferred test in packages/agent/test/sync-requester-priority.test.ts, or add a focused lifecycle test, to assert the exact contextGraphTerminals list and immutability for the deferred and prior-deferral tail.
M1 stack
This is PR 6 of 7 in the RFC-64 M1 stack. It is stacked on exact adaptive-capacity head
6992dd36ec23bf298e039ad5fd7a9b517b7a855dand supplies the runtime evidence contract consumed by PR 7. The validated review head isf123856165e168f7d15664d7e07d26badcf9ad81.Summary
This PR adds bounded, node-admin-only evidence for automatic RFC-64 synchronization work.
It records immutable
runningand terminalcompleteorfailedentries for:always-onselections;Operators can poll:
The endpoint requires the node-admin token. Agent-scoped tokens cannot inspect node-wide synchronization work. The exact authorization predicate is shared with the existing node-wide backpressure diagnostics route, while each route retains its own 403 response.
This is observability only. It does not change CG selection, scheduler priority, admission, retry, persistence, single-flight, or sync completion decisions.
User and operator impact
Before this PR, aggregate status and final store state could not prove which CGs a Core scheduler actually admitted, whether work was automatic or explicit, or whether an Edge refresh came from its post-restart
always-onreconciler.After this PR, operators and the release harness can bind exact observations to actual automatic runtime work. The journal is not itself a convergence verdict: PR 7 independently verifies exact VM/SWM heads, inventories, assets, and triples.
Edge behavior is unchanged:
on-demandCGs move only after explicit requests;always-onCGs reconcile automatically;Bounds and fail-closed behavior
performance.timeOriginplus a distinct wave ID;nextSequenceanddroppedBeforeSequenceexpose overwritten evidence.Evidence fails closed when IDs are truncated, metadata/durable/SWM is incomplete, SWM reports failure/timeout/denial/backpressure/dropped triples, or the overall operation does not cleanly terminate.
The aggregate compatibility result carries no terminal field, while the production detailed SWM boundary requires the canonical per-CG terminal list. A legacy JavaScript override returning only the aggregate result, malformed terminal entries, or throwing terminal getters leaves automatic evidence unverifiable and failed closed without changing successful sync accounting or retry behavior.
The negative runtime coverage explicitly proves:
complete: falserecords progress but cannot produce a successful evidence terminal;void, a non-iterable, or a hostile iterable cannot abort synchronization and leaves the metadata plane unverified;swm_recoverydetailed terminals retain their exact lane and disposition, and their frozen results cannot be mutated by callers.Every Core per-CG completion carries the scheduler round's actual job ID, preventing detached or synthetic completion claims.
Before
sequenceDiagram participant T as Internal sync trigger participant A as DKGAgent participant P as Existing planner participant S as Existing sync runtime participant D as Triple store participant O as Node operator T->>A: Connection or reconciler event A->>P: Plan synchronization work P-->>A: Selected and automatic CGs A->>S: Execute existing work S->>D: Persist verified results S-->>A: Aggregate result O->>A: Read aggregate status Note over O,A: No immutable per-round provenanceAfter
sequenceDiagram participant T as Internal sync trigger participant A as DKGAgent participant P as Existing planner participant J as Bounded evidence journal participant S as Existing sync runtime participant D as Triple store participant O as Node operator T->>A: Explicit module-private automatic command A->>P: Plan automatic round P-->>A: Evidence snapshot and frozen automatic tail A->>J: Start journal-owned running entry A->>S: Execute or join existing single-flight Note over A,S: Explicit selections remain live during later phases S->>D: Persist verified results once S-->>A: Aggregate plus mandatory per-CG SWM terminals A->>J: Finish through opaque journal handle O->>A: GET evidence after sequence N A->>J: Read bounded snapshot J-->>A: Entries and overwrite boundary A-->>O: Node-admin evidence Note over A,S: Every joiner receives the same canonical terminalsCompatibility and risk
syncSharedMemoryFromPeerDetailed(peer, ids, options)call shape is unchanged.SharedMemorySyncResultremains aggregate-only for structural compatibility. The production detailed method returnsSharedMemorySyncDetailedResultwith mandatory terminals, and one detailed-boundary translator maps canonical scheduler executions to those terminals.always-onstate./api/context-graph/subscriptionsgains additive effectivesyncMode.isNodeAdminCallerhelper preserves auth-disabled behavior and fails closed for missing, unknown, and agent-scoped tokens when authentication is enabled.Validation
6992dd36ec23bf298e039ad5fd7a9b517b7a855d.f123856165e168f7d15664d7e07d26badcf9ad81.19 + 7).4 + 3).git diff --check: pass.Review focus