RFC-64 M1 3/7: bound Core public CG coverage - #2013
Conversation
570f932 to
a7505e0
Compare
494ff1f to
c19f6a6
Compare
61d4902 to
76c5032
Compare
a7505e0 to
7f58a4a
Compare
14c4891 to
7042604
Compare
7f58a4a to
e628fe3
Compare
90e9805 to
1a1ad26
Compare
695cb97 to
3db3407
Compare
3db3407 to
1fab744
Compare
1fab744 to
07da61e
Compare
07da61e to
726333d
Compare
726333d to
d0dcb18
Compare
1a1ad26 to
04b9175
Compare
04b9175 to
9871c75
Compare
d0dcb18 to
fde07fa
Compare
9896d6b to
b0d93ed
Compare
b0d93ed to
34d3cf0
Compare
34d3cf0 to
235c807
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
| protected contextGraphSubscriptionRehydrationStatus: ContextGraphSubscriptionRehydrationStatus | null = null; | ||
| protected readonly contextGraphSubscriptionRehydrationAccountedIds = new Set<string>(); | ||
| /** Persisted admission hidden by a live-only configured explicit overlay. */ | ||
| protected readonly contextGraphSubscriptionDurableAdmissionOverrides = |
There was a problem hiding this comment.
🟡 Issue: Centralize sync-admission ownership instead of carrying hidden parallel state
What's wrong
The PR turns sync admission into a state machine, but the implementation spreads that state across the subscription map, mutable config, the coverage scheduler, and a hidden override map. That makes the invariant hard to audit: every future subscription mutation has to know whether it must update live admission, durable admission, config scope, scheduler membership, and persistence. This is exactly the kind of cross-cutting state that should have one canonical owner.
Example
A configured explicit overlay over a persisted automatic-public row now depends on multiple coordinated side effects: rehydration stores the durable override, persistence reads that external map, subscribe/unsubscribe/delete remember to clear it, and reconciliation mutates both config scope and scheduler registration.
Suggested direction
Model admission as a single explicit state, for example { liveAdmission, durableAdmission } on the normalized subscription or behind a small ContextGraphSyncAdmissionManager. Then make subscription/discovery/rehydration call transition methods and let persistence consume that state directly, deleting the external override map and scattered cleanup requirements.
For Agents
Look at contextGraphSubscriptionDurableAdmissionOverrides, reconcileContextGraphSyncAdmission, setContextGraphSubscription, persistContextGraphSubscription*, subscribeToContextGraph, unsubscribeFromContextGraph, and the rehydration block. Preserve the live-vs-durable overlay behavior, but make a single admission-state owner or typed model drive live state, scheduler registration, config scope, and durable projection. Existing overlay and rehydration tests should still pass.
| } | ||
| }, 30_000); | ||
|
|
||
| it('bounds store-discovered public coverage without capping explicit scope', async () => { |
There was a problem hiding this comment.
🟡 Issue: Split this boundary test before letting it sprawl past 1k lines
What's wrong
This PR pushes a test file from comfortably under 1k lines to well over it. The added cases are not just small assertions; they introduce several different concerns and repeat heavyweight setup. Even though this is test code, it becomes part of the codebase architecture: future reviewers now have to scan one large mixed suite to understand public coverage, discovery, rehydration, migration, and overlay behavior.
Example
The added scenarios cover bounded public coverage, explicit public rediscovery, chain access-policy classification, restart rehydration, legacy migration, configured overlays, and failure continuation in one already broad boundary file.
Suggested direction
Decompose the new coverage into focused files and shared local fixtures instead of extending this catch-all boundary suite. The production change is already concept-heavy; keeping the tests modular will make future admission-policy changes much easier to review.
For Agents
Split the new scenarios into focused specs such as context-graph-public-coverage.test.ts and context-graph-subscription-rehydration.test.ts. Extract small helpers for mock subscription stores, public/private chain rows, and started agents. Preserve the existing assertions; the proof is that the moved tests still run unchanged in behavior.
M1 stack
This is PR 3 of 7 in the RFC-64 M1 stack.
Stack base:
codex/rfc64-m1-selected-convergence(PR #2012).Exact restack coordinates:
da58c0c70453b32af4f355b00ca2c7a8fdf78fa9235c8071a098782d0aa454ec2eededd95280376bUser and operator impact
Core nodes maintain a separate bounded automatic-coverage set for revealed, chain-verified public Context Graphs. Edge nodes remain selected-only.
syncCorePublicBatchSizeorDKG_SYNC_CORE_PUBLIC_BATCH_SIZEchanges that cap;0disables automatic coverage without disabling discovery or live hosting.includeSharedMemorywhile preserving deprecatedincludeWorkspacecompatibility./api/statusexposes configured batch size, tracked coverage, active planning lanes, and bounded last-plan counters.Hash-only unresolved registrations, private CGs, and public CGs explicitly unsubscribed on that Core remain outside automatic coverage.
coreHostedremains ACK-derived and is not repurposed.Before
sequenceDiagram participant Chain participant Store as Durable subscription store participant Core participant Scope as Explicit sync scope participant Peer Chain->>Core: Discover a public CG Core->>Store: Persist registration state Note over Core: Automatic scheduler state is memory-only Note over Core,Store: Core restarts Peer->>Core: Start peer-sync round Core->>Scope: Read explicit selections Scope-->>Core: Selected CGs only Note over Core: Public coverage can be omitted or enter an uncapped pathAfter
sequenceDiagram participant Chain participant Store as Durable subscription store participant Core participant Scheduler as Core coverage scheduler participant Peer Chain->>Core: Confirm registered public CG Core->>Scheduler: Reconcile automatic coverage Core->>Store: Persist chain-bound eligibility Note over Core,Store: Core restarts Core->>Store: Rehydrate durable CG records Core->>Scheduler: Restore verified automatic coverage Peer->>Core: Start peer-sync round Core->>Scheduler: Plan one bounded automatic tail Scheduler-->>Core: Explicit selections plus automatic batch Core->>Peer: Pull durable VM using frozen round scope Core->>Peer: Pull eligible SWM using the same automatic tail Note over Core,Scheduler: New explicit selections remain live and uncappedScheduling and safety
Non-goals
Validation
git range-diff: 8/8 commits preserved; 6 are exactly patch-equivalent and two have only the expected canonical subscribe request-key/test-context delta from PR RFC-64 M1 2/7: expose verified VM and SWM convergence #2012.git diff --check: pass.Review focus