Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
* structurally omits the string barrier (its deps shape has no such
* member). 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
34 changes: 34 additions & 0 deletions packages/storage/test/store-control-barrier-contract-v1.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,3 +266,37 @@ 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;

});

// The compile-only NEGATIVE contracts (smuggled result types, forged keys,
// the coordinator's structural no-string-member pin) live in
// store-control-barrier-contract-v1.typetest.ts, compiled by the
// typecheck:type-contracts lane and never executed — this file stays
// runtime-only.

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
@@ -0,0 +1,57 @@
/**
* Compile-only negative type contracts for typed control-barrier keys (#2179).
*
* NEVER EXECUTED: this file is compiled by `typecheck:type-contracts`
* (tsconfig.typetests.json) and is not matched by the vitest include glob, so
* intentionally-invalid expressions prove type-level properties without
* enqueueing real barriers or floating promises. Each `@ts-expect-error` here
* is a live assertion — if the error it suppresses stops existing, the lane
* fails with TS2578 (unused directive). All values are `declare`d: there is
* no runtime, only shapes.
*/
import type { ManagedSystemRecordCoordinatorOptionsV1 } from '../src/adapters/system-record-managed-coordinator-v1-internal.js';
import type { StoreControlBarrierKeyV1 } from '../src/store-control-barrier-key-v1.js';
import type { StorePriorityScheduler } from '../src/store-priority-scheduler.js';

declare const scheduler: StorePriorityScheduler;
declare const epochKey: StoreControlBarrierKeyV1<{ epoch: string }>;
declare const takeOptions: (options: ManagedSystemRecordCoordinatorOptionsV1) => void;

// 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);

// 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 the suppression below into an unused
// directive (TS2578) and fails the typecheck lane. (An incomplete literal
// could not discriminate — missing-member errors would keep the suppression
// alive either way.) NOTE: never let the directive token itself start a
// wrapped comment line here — tsc parses any comment line beginning with the
// expect-error token as a REAL directive, and a phantom directive on a prose
// line reads as unused and fails this lane. That exact wrap bug shipped in
// this file's first version.
void takeOptions({
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
Comment thread
Jurij89 marked this conversation as resolved.
// managed coordinator's options
barrier: null as never,
});

export {};
Loading
Loading