Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/workflows/system-record-managed-ownership.yml
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,14 @@ jobs:
- name: Typecheck the ownership gate harness
run: pnpm typecheck:live:system-record-managed-ownership

- name: Typecheck barrier type contracts (issue 2179)
# The type-level pins in store-control-barrier-contract-v1.test.ts
Comment thread
Jurij89 marked this conversation as resolved.
Outdated
# (expectTypeOf + @ts-expect-error) only PROVE anything under a
# compiler. The package tsconfig includes only src/ and vitest does
# not typecheck, so without this lane every type assertion in that
# file is decorative — a check that cannot fail.
run: pnpm --filter @origintrail-official/dkg-storage run typecheck:type-contracts

- name: Storage unit conformance
run: |
pnpm --filter @origintrail-official/dkg-storage exec vitest run \
Expand Down Expand Up @@ -168,6 +176,7 @@ jobs:
test/changelog-store.test.ts \
test/graph-write-gen.test.ts \
test/system-record-decorator-apply-outcomes-v1.test.ts \
test/system-record-managed-coordinator-v1.test.ts \
test/store-control-barrier-contract-v1.test.ts \
test/store-priority-scheduler.test.ts \
test/store-scheduler-system-record-admission.test.ts \
Expand Down
1 change: 1 addition & 0 deletions packages/storage/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"build": "tsc",
"test": "vitest run",
"test:coverage": "vitest run --coverage",
"typecheck:type-contracts": "tsc --noEmit -p tsconfig.typetests.json",
"clean": "rm -rf dist tsconfig.tsbuildinfo"
},
"dependencies": {
Expand Down
13 changes: 2 additions & 11 deletions packages/storage/src/adapters/sparql-http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -783,17 +783,8 @@
// when the controller was BUILT would seal a generation that has since
// been replaced.
//
// Keep the published string callback intact for external/legacy
// coordinator composition. Production lifecycle calls use the
// domain-owned typed methods below, which translate to scheduler keys
// only at this adapter boundary.
barrier: (purpose, transition) =>
externalStorePriorityScheduler.runControlBarrier(
this,
purpose,
transition,
barrierGeneration(),
),
// The only barrier managed composition has (#2179) — rationale on
// SystemRecordLaneControllerTypedDepsV1.
typedBarrier: (kind, transition) =>
runTypedBarrier(SYSTEM_RECORD_BARRIER_KEYS_V1[kind], transition),
setAdmissionActive: (active) => { this.systemRecordAdmissionActive = active; },
Expand Down Expand Up @@ -1428,7 +1419,7 @@
async countQuads(graphUri?: string, options?: QueryOptions): Promise<number> {
const sparql = graphUri
? `SELECT (COUNT(*) AS ?c) WHERE { GRAPH <${escapeUri(graphUri)}> { ?s ?p ?o } }`
: `SELECT (COUNT(*) AS ?c) WHERE { { ?s ?p ?o } UNION { GRAPH ?g { ?s ?p ?o } } }`;

Check notice on line 1422 in packages/storage/src/adapters/sparql-http.ts

View workflow job for this annotation

GitHub Actions / SPARQL scalability lint

sparql-scale-lint R2 graph-var-scan

All-variable triple inside GRAPH ?var enumerates every graph × every triple (the #1597 listGraphs-storm shape). Bind the graph (VALUES/exact IRI), bind a term, or use a FILTER EXISTS existence probe. [pre-existing (grandfathered; fix when touched)] To acknowledge: "sparql-scan-allow: R2 -- <why this is bounded>"

Check notice on line 1422 in packages/storage/src/adapters/sparql-http.ts

View workflow job for this annotation

GitHub Actions / SPARQL scalability lint

sparql-scale-lint R1 unscoped-all-var-scan

All-variable triple pattern with no graph scope scans the ENTIRE store. Scope it to an exact named graph, bind at least one term, or add LIMIT (without ORDER BY). [pre-existing (grandfathered; fix when touched)] To acknowledge: "sparql-scan-allow: R1 -- <why this is bounded>"
const r = await this.query(sparql, {
...options,
source: options?.source ?? 'sparql-http.countQuads',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
createSystemRecordLaneControllerV1,
type SystemRecordApplyOutcomeV1,
type SystemRecordChildHandoffV1,
type SystemRecordLaneBarrierV1,
type SystemRecordLaneControllerTypedDepsV1,
type SystemRecordLaneControllerV1,
type SystemRecordLaneExecutionBindingV1,
type SystemRecordLaneTypedBarrierV1,
Expand All @@ -27,7 +27,10 @@ export interface ManagedSystemRecordCoordinatorOptionsV1 {
proof: unknown,
childGeneration: string,
) => Promise<SystemRecordApplyOutcomeV1>;
readonly barrier: SystemRecordLaneBarrierV1;
/**
* The ONLY barrier the managed path accepts — no string member exists here
* by design; rationale on {@link SystemRecordLaneControllerTypedDepsV1}.
*/
readonly typedBarrier: SystemRecordLaneTypedBarrierV1;
readonly setAdmissionActive: (active: boolean) => void;
}
Expand All @@ -44,7 +47,11 @@ export function createManagedSystemRecordCoordinatorV1(
updateEndpoint: options.updateEndpoint,
resolveClient: options.resolveClient,
});
return createSystemRecordLaneControllerV1({
// The deps literal is typed as the typed-only shape, so the managed path
// resolves the single factory's typed overload: no string member exists
// here to fall back to, and adding one is a type error pinned in the
// typecheck lane.
const typedDeps: SystemRecordLaneControllerTypedDepsV1 = {
lease: options.lease,
handoff: options.handoff,
executor: {
Expand All @@ -53,8 +60,8 @@ export function createManagedSystemRecordCoordinatorV1(
applyVerifiedSettlementBound: (proof, binding, registerRecovery) =>
atomicExecutor.execute(proof, binding, registerRecovery),
},
barrier: options.barrier,
typedBarrier: options.typedBarrier,
setAdmissionActive: options.setAdmissionActive,
});
};
return createSystemRecordLaneControllerV1(typedDeps);
}
19 changes: 18 additions & 1 deletion packages/storage/src/store-priority-scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,16 @@ export type StoreQueuedAdmissionV1 = StoreAdmissionV1 & {
readonly mode: Exclude<StoreAdmissionMode, 'control-barrier'>;
};

/**
* @deprecated The `run()` control-barrier admission is the same unsound
* purpose-string contract as {@link StorePriorityScheduler.runControlBarrier}:
* the run() generic `T` is chosen by each caller while coalescing is keyed by
* `(storeId, purpose-string)`, so a coalesced caller receives the first
* transition's value under its own `T`. Use
* {@link StorePriorityScheduler.runTypedControlBarrier} with a key from
* `createStoreControlBarrierKeyV1`. Removed, together with `runControlBarrier`,
* at the next allowed breaking version boundary.
*/
export type StoreControlBarrierAdmissionV1 = StoreAdmissionV1 & {
readonly mode: 'control-barrier';
};
Expand Down Expand Up @@ -1080,7 +1090,14 @@ export class StorePriorityScheduler extends ObservableScheduler {
* callers that already use the historical free-form string contract.
*
* @deprecated Use {@link runTypedControlBarrier} with a key created by
* `createStoreControlBarrierKeyV1`.
* `createStoreControlBarrierKeyV1` — one key per transition, created once at
* module scope, binds every coalescing caller to that key's result type.
* First-party code no longer calls this method (managed composition poisons
Comment thread
Jurij89 marked this conversation as resolved.
Outdated
* its string-barrier fallback so the path cannot silently re-animate); it is
* removed, together with the `'control-barrier'` `run()` admission mode, at
* the next allowed breaking version boundary. Until then behavior is
* unchanged: both entry points share the coordinator, so coalescing,
* timeout, sealing, quiescence and metrics are identical.
* @param timeoutMs Overrides the default bound for this transition.
*/
runControlBarrier<T>(
Expand Down
123 changes: 104 additions & 19 deletions packages/storage/src/system-record-materializer-v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,36 +239,58 @@ export type SystemRecordLaneTypedBarrierV1 = <K extends SystemRecordLaneBarrierK
transition: () => Promise<SystemRecordLaneBarrierResultsV1[K]>,
) => Promise<SystemRecordLaneBarrierResultsV1[K]>;

export interface SystemRecordLaneControllerDepsV1 {
/**
* Dependencies shared by every controller composition route. The legacy and
* typed entry points differ ONLY in how the barrier is supplied; everything
* else lives here exactly once, so a future shared dependency (a tracing
* hook, a lifecycle signal) is added in one place and reaches both routes or
* neither.
*/
export interface SystemRecordLaneControllerSharedDepsV1 {
/** The supervisor-issued live ownership lease. Captured, never accepted per-call. */
readonly lease: ManagedOxigraphOwnershipLeaseV1;
readonly handoff: SystemRecordChildHandoffV1;
readonly executor: SystemRecordTransactionExecutorV1;
/**
* Adapter-owned admission latch, driven by the lifecycle's physical state.
* `true` is published synchronously before enable can enqueue its barrier;
* `false` is published only after disable physically commits or the lane is
* terminally unavailable. Merely constructing the controller never calls it.
*/
readonly setAdmissionActive?: (active: boolean) => void;
}

export interface SystemRecordLaneControllerDepsV1 extends SystemRecordLaneControllerSharedDepsV1 {
/**
* Required, not optional. An optional barrier is one that gets forgotten:
* this capability shipped once with a barrier implemented, exported and
* tested but with zero production callers, so the enable path stopped the
* child while ordinary requests were still in flight.
* child while ordinary requests were still in flight. Required-ness is the
* compile-time guard against that recurring, which is why retiring the
* string contract does NOT make this member optional before the break.
*
* @deprecated Managed composition uses `typedBarrier`. Retained so existing
* external controller integrations keep their purpose-string contract.
* @deprecated The purpose-string contract cannot carry a sound result type:
* coalescing is keyed by a runtime string while each caller picks a static
* `T`, so a later same-purpose caller receives the first promise under its
* own `T`. Supply {@link typedBarrier}; migrate string barriers to
* `runTypedControlBarrier` with keys from `createStoreControlBarrierKeyV1`.
* First-party composition no longer passes through this interface at all —
* the managed coordinator calls the factory's typed overload with
* {@link SystemRecordLaneControllerTypedDepsV1}, which has no string member
* to fall back to. This member is removed at the next allowed breaking
* version boundary, not before: the removal is source-incompatible for
* external composers.
*/
readonly barrier: SystemRecordLaneBarrierV1;
/** Optional typed path; the string callback above remains the compatibility contract. */
readonly typedBarrier?: SystemRecordLaneTypedBarrierV1;
/**
* Adapter-owned admission latch, driven by the lifecycle's physical state.
* `true` is published synchronously before enable can enqueue its barrier;
* `false` is published only after disable physically commits or the lane is
* terminally unavailable. Merely constructing the controller never calls it.
* When supplied it is ALWAYS used and the string callback above is never
* invoked; the fallback exists only for external composers that predate
* typed keys.
*/
readonly setAdmissionActive?: (active: boolean) => void;
readonly typedBarrier?: SystemRecordLaneTypedBarrierV1;
}

type SystemRecordLaneSessionDepsV1 = Pick<
SystemRecordLaneControllerDepsV1,
'lease' | 'handoff' | 'executor' | 'setAdmissionActive'
> & {
type SystemRecordLaneSessionDepsV1 = SystemRecordLaneControllerSharedDepsV1 & {
readonly runEnableBarrier: (
transition: () => Promise<SystemRecordLaneBarrierResultsV1['enable']>,
) => Promise<SystemRecordMaterializationEpochRotationSnapshotV1>;
Expand Down Expand Up @@ -428,23 +450,86 @@ export async function releaseSystemRecordLaneControllerV1(
CONTROLLER_SESSIONS.delete(controller);
}

/**
* Typed-only controller deps: the managed path's shape, and the CANONICAL
* home of the typed-barrier rationale (call sites carry one-line pointers
* here, not copies).
*
* There is NO string `barrier` member — not deprecated, not optional,
* structurally absent — so first-party composition cannot regress onto the
* purpose-string contract by any edit short of changing this interface,
* which is the loudest possible place for that change. The string contract
* is unsound for coalescing (a runtime purpose string cannot bind the result
* type shared by coalesced callers) and survives only on the public
* compatibility entry point until its breaking-version removal. The
* compile-time can't-forget guard that `SystemRecordLaneControllerDepsV1.
* barrier` provides for external composers is provided here by `typedBarrier`
* being required. A type-contract pin fails the typecheck lane if a string
* member is ever re-added to a typed-only shape.
*/
export interface SystemRecordLaneControllerTypedDepsV1
extends SystemRecordLaneControllerSharedDepsV1 {
readonly typedBarrier: SystemRecordLaneTypedBarrierV1;
}

/**
* The ONE controller constructor. Accepts either the typed-only deps (the
* managed path — no string member exists on that shape) or the legacy-capable
* public deps, and normalizes the difference internally: a supplied
* `typedBarrier` is used as-is; otherwise the string `barrier` is wrapped,
* preserving the purpose-string contract for external composers until its
* breaking-version removal. One entry point, one singleton lifecycle — the
* shape split lives in the TYPES, not in parallel factories.
*
* ORDER MATTERS on the duplicate-registration guard: it runs before ANY
* caller-supplied dependency property is read. Deps come from composers and
* may carry accessors; a second registration must be refused with
* {@link SystemRecordControllerRegistrationError} before composer code gets a
* chance to run (or throw something else) from a property read.
*/
/**
* The barrier-mode split, stated once and PROVEN by narrowing rather than
* asserted by a cast: `'barrier' in deps` discriminates the union, so the
* compiler itself proves the legacy branch has a string `barrier` to wrap
* and the typed-only branch has a required `typedBarrier`. A future deps
* change that breaks either premise fails to typecheck here instead of
* surviving inside an `as`.
*/
function normalizeControllerBarrierV1(
deps: SystemRecordLaneControllerDepsV1 | SystemRecordLaneControllerTypedDepsV1,
): SystemRecordLaneTypedBarrierV1 {
if (!('barrier' in deps)) return deps.typedBarrier;
// Legacy-capable shape: a supplied typed member always wins; the string
// barrier is wrapped only when it is all the composer has.
return deps.typedBarrier ??
((kind, transition) => deps.barrier(`system-record.${kind}`, transition));
}

export function createSystemRecordLaneControllerV1(
deps: SystemRecordLaneControllerTypedDepsV1,
): SystemRecordLaneControllerV1;
export function createSystemRecordLaneControllerV1(
deps: SystemRecordLaneControllerDepsV1,
): SystemRecordLaneControllerV1;
export function createSystemRecordLaneControllerV1(
deps: SystemRecordLaneControllerDepsV1 | SystemRecordLaneControllerTypedDepsV1,
): SystemRecordLaneControllerV1 {
// Guard FIRST, before the normalizer or any other deps read — deps come
// from composers and may carry accessors; see the registration-invariant
// tests.
if (registeredController) throw new SystemRecordControllerRegistrationError();

const publicBarrier: SystemRecordLaneTypedBarrierV1 = deps.typedBarrier ??
((kind, transition) => deps.barrier(`system-record.${kind}`, transition));
const typedBarrier = normalizeControllerBarrierV1(deps);
const session = new SystemRecordLaneSession({
lease: deps.lease,
handoff: deps.handoff,
executor: deps.executor,
setAdmissionActive: deps.setAdmissionActive,
runEnableBarrier: async (transition) =>
snapshotSystemRecordMaterializationEpochRotationV1(
await publicBarrier('enable', transition),
await typedBarrier('enable', transition),
),
runVoidBarrier: (kind, transition) => publicBarrier(kind, transition),
runVoidBarrier: (kind, transition) => typedBarrier(kind, transition),
});
const controller: SystemRecordLaneControllerV1 = Object.freeze({
open: (activation: SystemRecordLaneActivationV1) => session.open(activation),
Expand Down
79 changes: 78 additions & 1 deletion packages/storage/test/store-control-barrier-contract-v1.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import {
StorePriorityScheduler,
type StoreQueuedAdmissionV1,
} from '../src/store-priority-scheduler.js';
import { createStoreControlBarrierKeyV1 } from '../src/store-control-barrier-key-v1.js';
import {
createStoreControlBarrierKeyV1,
type StoreControlBarrierKeyV1,
} from '../src/store-control-barrier-key-v1.js';
import type { ManagedSystemRecordCoordinatorOptionsV1 } from '../src/adapters/system-record-managed-coordinator-v1-internal.js';

/**
* The scheduler/coordinator boundary, asserted through the PUBLIC scheduler.
Expand Down Expand Up @@ -266,3 +270,76 @@ describe('control barrier contract survives the coordinator extraction', () => {
expect(ran).toBe(true);
});
});

describe('typed barrier keys close the generic result channel (#2179)', () => {
// The retired contract's defect was that `T` was chosen per CALL while
// coalescing was keyed per runtime STRING, so the type system had no say in
// which caller's `T` a coalesced promise satisfied. These are type-level
// pins that the replacement actually closes that channel: `T` is chosen per
// KEY, once, and no call site can renegotiate it.
it('binds the result type to the key, not the call site', async () => {
const scheduler = new StorePriorityScheduler({ maxConcurrent: 1 });
const epochKey = createStoreControlBarrierKeyV1<{ epoch: string }>('typed.epoch');
const effectKey = createStoreControlBarrierKeyV1<void>('typed.effect');

const typed = scheduler.runTypedControlBarrier({}, epochKey, async () => ({ epoch: '1' }));
expectTypeOf(typed).toEqualTypeOf<Promise<{ epoch: string }>>();
expect(await typed).toEqual({ epoch: '1' });

// A void key is an effect barrier BY TYPE: its promise carries no data, so
// it cannot be used as a typed side channel between coalescing callers.
const effect = scheduler.runTypedControlBarrier({}, effectKey, async () => {});
expectTypeOf(effect).toEqualTypeOf<Promise<void>>();
await effect;

});

// Compile-time-only negatives, in a function that is DELIBERATELY never
// invoked: the typecheck lane compiles the body (each @ts-expect-error is
// verified live — fixing the suppressed error away yields TS2578), while
// the vitest run never executes an intentionally-invalid call, so no
// real barrier is enqueued and no promise floats.
const negativeTypeContracts = (
scheduler: StorePriorityScheduler,
epochKey: StoreControlBarrierKeyV1<{ epoch: string }>,
) => {
// A transition cannot smuggle a different result type past its key.
// @ts-expect-error — the epoch key demands { epoch: string }, not number
void scheduler.runTypedControlBarrier({}, epochKey, async () => 7);
Comment thread
Jurij89 marked this conversation as resolved.
Outdated

// A key cannot be forged from a plain literal: the module-private brand is
// a required member no caller outside the factory can produce.
// @ts-expect-error — structural literal lacks the private brand
void scheduler.runTypedControlBarrier({}, { purpose: 'forged' }, async () => 7);

// The managed coordinator is typed-only STRUCTURALLY: its options carry no
// string-barrier member, so first-party composition cannot regress onto
// the purpose-string contract without editing that interface. The literal
// below is COMPLETE apart from `barrier`, deliberately: with every
// required member present, the excess `barrier` property is the ONLY
// error, so re-adding the member to the interface turns this suppression
// into an unused @ts-expect-error and fails the typecheck lane. (An
// incomplete literal could not discriminate — missing-member errors would
// keep the suppression alive either way.)
const typedOnly = (options: ManagedSystemRecordCoordinatorOptionsV1) => options;
void typedOnly({
lease: null as never,
handoff: null as never,
storeId: {},
queryEndpoint: '',
updateEndpoint: '',
resolveClient: () => null,
applyLegacy: null as never,
typedBarrier: null as never,
setAdmissionActive: () => {},
// @ts-expect-error — a string `barrier` member does not exist on the
// managed coordinator's options
barrier: null as never,
});
};
void negativeTypeContracts;

it('rejects an empty purpose at key creation', () => {
expect(() => createStoreControlBarrierKeyV1('')).toThrow(/must not be empty/);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ vi.mock('../src/system-record-materializer-v1.js', async (importOriginal) => {
>();
return {
...actual,
// There is ONE controller constructor (#2179 round 3): both the managed
// coordinator and any legacy composer go through it, so this single
// injection point covers every construction route.
createSystemRecordLaneControllerV1: (deps: never) => {
if (injected.error) throw injected.error;
return actual.createSystemRecordLaneControllerV1(deps);
Expand Down
Loading
Loading