refactor(storage): retire result-bearing coalesced control barriers from first-party composition - #2195
Merged
Jurij89 merged 11 commits intoAug 9, 2026
Conversation
… composition Completes the migration #2169 started. The typed-key path (runTypedControlBarrier + createStoreControlBarrierKeyV1) already carried every production lifecycle transition; what remained of the string contract was one live production call site and a silent re-animation hazard. sparql-http no longer supplies a real string barrier. The controller deps member stays required -- required-ness is the compile-time guard that already caught a barrier shipping with zero callers once in this stack -- but managed composition now satisfies it with a POISONED callback that throws a named error. The fallback in createSystemRecordLaneControllerV1 reaches the string barrier only when typedBarrier is absent, i.e. only under the future edit that removes it; the poison turns that edit from a silent migration onto the deprecated unsound-cast contract into an immediate failure. Mutation-checked: deleting typedBarrier from the production deps fails the epoch-adapter test with exactly the poison's message. A lifecycle test pins the fail-closed shape -- the only handoff interaction is the fail-closed order; no physical step runs. runControlBarrier and the run() 'control-barrier' admission are deprecated with the migration path and the removal boundary stated (both go together at the next allowed breaking version; both are source-incompatible for external callers before it). External composers keep working: the deps interface shape is unchanged, the string members remain, and the lifecycle suite -- which supplies string-only barriers throughout -- is the compat pin. Type contracts are now PROVEN, not decorative: nothing typechecked storage test files (package tsconfig includes only src/, vitest does not typecheck), so every expectTypeOf in the suite was inert. A narrow tsconfig.typetests lane compiles the barrier contract file in CI; the new @ts-expect-error pins (a transition cannot smuggle a different result type past its key; a key cannot be forged from a literal) were verified live by TS2578 when their errors are fixed away. The suite-wide typecheck gap is flagged for follow-up rather than absorbed here. Closes #2179. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… by poison Review round 1 of #2195, both findings taken. The poison pill is gone, replaced by the stronger design the review pointed at: createSystemRecordLaneControllerTypedV1 is the typed-only core (deps have NO string-barrier member at all), the public createSystemRecordLaneControllerV1 becomes the compatibility adapter that normalizes legacy deps down to it, and the managed coordinator's options drop the string member entirely. First-party composition now cannot regress onto the purpose-string contract by any edit short of changing an internal interface -- enforcement moved from a runtime trap to the type system, and there is no impossible callback left to explain. The invariant's tripwire moved with it: a complete-literal @ts-expect-error pin proves the coordinator options reject a barrier member, and the pin discriminates -- re-adding the member turns it into an unused suppression and fails the typecheck lane (verified live, TS2578). The literal is complete apart from `barrier` deliberately: an incomplete one could not discriminate, because missing-member errors would keep the suppression alive either way. The poison lifecycle test is reframed as what it also proved and what still matters: an external string barrier that throws at the boundary fails enable loudly and CLOSED -- the only handoff interaction is the fail-closed order. Negative type assertions move out of the runtime path into a never-invoked function: the typecheck lane still compiles every @ts-expect-error, but vitest no longer executes intentionally-invalid calls or leaves their promises floating. The capability-probe injection follows the composition change: the coordinator now imports the typed builder, so the mock sits there. Its comment states the scope honestly -- a module mock cannot intercept the public adapter's module-internal call; the injection covers the route production takes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sh coordinator evidence Review round 2 of #2195, all three findings taken. SystemRecordLaneControllerSharedDepsV1 now carries lease/handoff/executor/ setAdmissionActive exactly once; the public legacy deps and the typed-only deps are barrier-specific extensions of it, and the session deps build on the same base. A future shared dependency is added in one place and reaches both composition routes or neither -- the drift the duplication invited. The coordinator composition test was STALE AND RED on this branch: it mocked the legacy factory, so the coordinator's import of the typed builder resolved to undefined under the mock. My curated-lane runs never saw it because the file was in no lane at all -- the reviewer caught from static reading what my green lane hid. The test now mocks the typed factory (mocking ONLY that export, deliberately: a coordinator that reaches for the legacy factory again finds undefined and fails at the call), asserts the deps production passes carry typedBarrier and that 'barrier' is structurally absent from them, and the file joins the storage conformance lane so the changed boundary stays covered. The typed-barrier rationale now lives in ONE place -- on SystemRecordLaneControllerTypedDepsV1 -- with one-line pointers at the coordinator option and the sparql-http call site. Full storage suite run this time, not just the lane: 918 passing; every failure is in the oxigraph-worker files, timeout-shaped, varies in count between identical runs, and reproduces identically on the untouched integration head under the same machine load. Not attributable to this diff, which does not touch the worker adapter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ore any deps read Review round 3 of #2195, both findings taken. The second exported factory is gone. createSystemRecordLaneControllerV1 is again the only controller constructor, with overloads accepting either the typed-only deps or the legacy-capable public deps and normalizing internally -- the shape split lives in the types, not in parallel factories. The managed coordinator passes a literal typed as SystemRecordLaneControllerTypedDepsV1, so its structural no-string-member invariant and the typecheck-lane pin are unchanged, while nothing leaks a second deep-importable construction surface. The duplicate-registration guard runs before ANY caller-supplied dependency property is read. The round-2 shape read deps.typedBarrier (and the shared members) in the compatibility wrapper before the core's guard, so a second registration with a throwing accessor surfaced the composer's error instead of SystemRecordControllerRegistrationError. Collapsing to one factory also collapses the ordering question: guard first, then normalization. Pinned by a test whose deps object throws on EVERY property access -- the typed refusal can only surface if nothing was read first -- and mutation-checked: hoisting a single deps read above the guard fails the test with exactly the trap's message. The mocks follow: one factory means one injection point again, and the capability-probe injection now covers every construction route rather than one of two. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review round 4 of #2195, finding taken. normalizeControllerBarrierV1 discriminates the deps union with 'barrier' in deps, so the compiler itself proves the legacy branch has a string barrier to wrap and the typed-only branch has a required typedBarrier -- the previous `as SystemRecordLaneControllerDepsV1` asserted what is now checked. A future deps change that breaks either premise fails to typecheck at the normalizer instead of surviving inside the cast. No casts remain in the factory. The registration-guard pin is tightened to match: `in` narrowing triggers a Proxy `has` trap, not `get`, so the booby-trapped deps now trap BOTH -- any inspection of composer deps before the guard trips the test, including the normalizer's discriminating check. Mutation-checked: hoisting the normalizer call above the guard fails the test with exactly the has-trap's message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…etest file Review round 5 of #2195, both items taken. The negative type contracts live in store-control-barrier-contract-v1 .typetest.ts: declare-d shapes only, no runtime, compiled by the typecheck:type-contracts lane and not matched by the vitest glob. The runtime spec is executable-only again -- no never-invoked function, no coordinator type assertions in a scheduler test. The stale @deprecated parenthetical on runControlBarrier (it described the retired poison design) now states the structural invariant. The move surfaced a tsc parsing hazard worth recording: a wrapped comment line that BEGINS with the expect-error token is parsed as a REAL directive. My explanatory comment wrapped exactly that way, creating a phantom directive on a prose line that read as unused and failed the lane -- while the same sentence in the runtime file happened to wrap differently and was clean. The comment is reworded, the file warns about the trap, and all three pins were re-proven to discriminate in their new home (interface mutant flips the real directive to TS2578; forged-key raw error is TS2345). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review round 6 of #2195, both items taken. The #2179 rationale lives once, on SystemRecordLaneControllerTypedDepsV1; the normalizer is a one-line doc and the factory doc states only what it owns (one constructor, both shapes, guard-before-deps-read). This also fixes a real docs bug the consolidation surfaced: the factory's docblock sat before the normalizer's, so API docs attached to the wrong symbol. The CI lane comment now names both files the lane compiles -- the runtime spec's expectTypeOf assertions and the never-executed .typetest.ts negative contracts -- instead of the pre-split single file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review round 7 of #2195. The deprecation guidance told external composers to migrate to the typed shape while the barrel exported only the legacy deps type -- the migration target was reachable by deep import alone. Both the typed deps and the shared base are now first-class barrel exports, so the documented path is the public path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…yped deps shape Review round 8 of #2195, all three findings taken. The barrier vocabulary, the three deps shapes, and the normalizer move to system-record-lane-controller-contract-v1.ts -- the compatibility boundary reads on its own, and the 2k-line materializer keeps lane-session mechanics. The materializer re-exports everything, so no import surface changes; the contract module imports only TYPES back, keeping the value graph acyclic. SystemRecordLaneControllerSharedDepsV1 is un-published from the barrel: it is a factoring device, not a valid factory input, and a public name for an incomplete shape is compatibility burden without use. (My round-7 change exported it alongside the typed deps; that was an overshoot.) The typed-only factory input now carries its own structural pin: a complete literal with excess `barrier` under a suppression, so the typed deps interface growing a string member -- even optional -- flips the directive to unused and fails the lane. Mutation-checked: adding `barrier?` to the interface yields TS2578 at exactly that pin. The coordinator-options pin alone could not see this regression. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The materializer re-exports only the contract types with real consumers; the normalizer (constructor plumbing) and the shared deps base (not a valid factory input) stay reachable only through the contract module, which the barrel does not publish. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…structor Review round 10 of #2195. This finding asked for a typed core constructor with the legacy entry point adapting into it -- which round 3 (id 3743757711) had rejected in its exported form as a second deep-importable construction surface. The shape that satisfies BOTH recorded objections at once: the one exported constructor stays the only construction surface and becomes a thin adapter (guard, normalize, delegate), and the session construction moves into a MODULE-PRIVATE typed-only core that knows nothing about the legacy shape. No overload, union, or normalizer call remains below the adapter's body. The round-3 ordering contract is untouched: the duplicate-registration guard runs in the public adapter before any caller-supplied deps property is read, and the private core documents that it assumes the guard already ran. Pinned by the existing accessor-trap registration test, which stays green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
(storeId, purpose-string)while each caller picks a staticT, so a later same-purpose caller receives the first transition's promise under its ownT. The typed-key path (runTypedControlBarrier+createStoreControlBarrierKeyV1, landed in refactor(storage): extract the control-barrier subsystem from StorePriorityScheduler #2169) bindsTto the key once; this PR finishes the migration so no first-party code calls the string API at all.createSystemRecordLaneControllerV1remains the only controller constructor, with overloads accepting eitherSystemRecordLaneControllerTypedDepsV1(no string-barrier member at all) or the legacy-capable public deps, normalizing internally. The managed coordinator's options forbid the string member and it passes a typed-deps literal, so first-party composition cannot regress onto the purpose-string contract by any edit short of changing an internal interface. A complete-literal@ts-expect-errorpin keeps that structural absence honest — re-adding the member fails the typecheck lane with TS2578 (verified live). The duplicate-registration guard runs before any caller-supplied deps property is read, pinned by a throw-on-every-access Proxy test and mutation-checked.runControlBarrierand therun()'control-barrier'admission with the migration path and the removal boundary stated: both are removed together at the next allowed breaking version, not before — both removals are source-incompatible for external callers. Until then externals keep working unchanged; the lifecycle suite (string-only barriers throughout) is the compat pin. Coalescing, timeout, sealing, quiescence and metrics behavior are untouched — both entry points share the coordinator.src/; vitest does not typecheck), so everyexpectTypeOfin the suite was inert. A narrowtsconfig.typetests.jsonlane compiles the barrier contract file in CI. The suite-wide gap is flagged as follow-up, not absorbed here.Related
.github/workflows/system-record-managed-ownership.ymlis also touched by open PR chore(storage): add an exports map and stop publishing daemon ownership authority #2189 (different hunks; whichever merges second rebases trivially)Diagrams
Lifecycle transition under the barrier
Before:
sequenceDiagram participant Lane as system-record lane participant Deps as controller deps participant Sched as StorePriorityScheduler Lane->>Deps: typedBarrier('enable', transition) Deps->>Sched: runTypedControlBarrier(storeId, key, transition) Note over Deps,Sched: if typedBarrier were ever removed: Lane->>Deps: barrier('system-record.enable', transition) Deps->>Sched: runControlBarrier(storeId, purpose, transition) Note over Sched: silent fallback onto the unsound<br/>string contract — as Promise#60;T#62;After:
sequenceDiagram participant Lane as system-record lane participant Deps as controller deps participant Sched as StorePriorityScheduler Lane->>Deps: typedBarrier('enable', transition) Deps->>Sched: runTypedControlBarrier(storeId, key, transition) Note over Deps,Sched: the coordinator's deps have NO string member —<br/>falling back is a type error, pinned by<br/>an #64;ts-expect-error tripwire in the typecheck laneFiles changed
packages/storage/src/adapters/sparql-http.tstypedBarrier; no string barrier exists in managed compositionpackages/storage/src/adapters/system-record-managed-coordinator-v1-internal.tspackages/storage/src/system-record-materializer-v1.tsSystemRecordLaneControllerSharedDepsV1); guard-before-deps-read ordering; deprecation docs with removal boundarypackages/storage/src/store-priority-scheduler.tsrunControlBarrier+StoreControlBarrierAdmissionV1deprecations: migration path, removal boundary, behavior-parity statementpackages/storage/test/system-record-materializer-lifecycle-v1.test.tspackages/storage/test/system-record-capability-probe-errors-v1.test.tspackages/storage/test/store-control-barrier-contract-v1.test.ts@ts-expect-errorpins (no smuggled result type, no forged key)packages/storage/tsconfig.typetests.json,packages/storage/package.jsontypecheck:type-contractslane so the type assertions are compiled, not decorative.github/workflows/system-record-managed-ownership.ymlTest plan
typecheck:type-contractslane green; both@ts-expect-errorpins proven live (fixing the suppressed error away yields TS2578)barriermember to the coordinator options turns the@ts-expect-errorpin into an unused suppression — typecheck lane fails with TS2578 (verified live)runControlBarrier(call sites outside the scheduler's own deprecated method