Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
8 changes: 8 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
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
18 changes: 7 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,13 @@
// 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(),
),
// Typed-only, structurally (#2179): the managed coordinator's
Comment thread
Jurij89 marked this conversation as resolved.
Outdated
// options carry NO string-barrier member, so first-party
// composition cannot fall back onto the deprecated purpose-string
// contract by any edit short of changing that internal interface —
// the loudest possible place for such a change. External composers
// with string barriers use the public
// `createSystemRecordLaneControllerV1` compatibility adapter.
typedBarrier: (kind, transition) =>
runTypedBarrier(SYSTEM_RECORD_BARRIER_KEYS_V1[kind], transition),
setAdmissionActive: (active) => { this.systemRecordAdmissionActive = active; },
Expand Down Expand Up @@ -1428,7 +1424,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 1427 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 1427 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 @@ -4,10 +4,9 @@ import {
type SystemRecordAtomicApplyHttpClientV1,
} from '../system-record-atomic-apply-executor-v1-internal.js';
import {
createSystemRecordLaneControllerV1,
createSystemRecordLaneControllerTypedV1,
type SystemRecordApplyOutcomeV1,
type SystemRecordChildHandoffV1,
type SystemRecordLaneBarrierV1,
type SystemRecordLaneControllerV1,
type SystemRecordLaneExecutionBindingV1,
type SystemRecordLaneTypedBarrierV1,
Expand All @@ -27,7 +26,15 @@ export interface ManagedSystemRecordCoordinatorOptionsV1 {
proof: unknown,
childGeneration: string,
) => Promise<SystemRecordApplyOutcomeV1>;
readonly barrier: SystemRecordLaneBarrierV1;
/**
* The ONLY barrier the managed path accepts. There is deliberately no
* string-barrier member on these options: the purpose-string contract is
* retired for first-party composition (#2179), and its absence here is
* structural — a future edit cannot fall back onto it without changing
* this interface, which is the loudest possible place for that change.
* External composers with string barriers use the public
* `createSystemRecordLaneControllerV1` compatibility adapter instead.
*/
readonly typedBarrier: SystemRecordLaneTypedBarrierV1;
readonly setAdmissionActive: (active: boolean) => void;
}
Expand All @@ -44,7 +51,7 @@ export function createManagedSystemRecordCoordinatorV1(
updateEndpoint: options.updateEndpoint,
resolveClient: options.resolveClient,
});
return createSystemRecordLaneControllerV1({
return createSystemRecordLaneControllerTypedV1({
Comment thread
Jurij89 marked this conversation as resolved.
Outdated
lease: options.lease,
handoff: options.handoff,
executor: {
Expand All @@ -53,7 +60,6 @@ export function createManagedSystemRecordCoordinatorV1(
applyVerifiedSettlementBound: (proof, binding, registerRecovery) =>
atomicExecutor.execute(proof, binding, registerRecovery),
},
barrier: options.barrier,
typedBarrier: options.typedBarrier,
setAdmissionActive: options.setAdmissionActive,
});
Expand Down
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
74 changes: 64 additions & 10 deletions packages/storage/src/system-record-materializer-v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,13 +248,28 @@ export interface SystemRecordLaneControllerDepsV1 {
* 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 builds on the typed-only
* `createSystemRecordLaneControllerTypedV1`, whose deps have 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. */
/**
* 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 typedBarrier?: SystemRecordLaneTypedBarrierV1;
/**
* Adapter-owned admission latch, driven by the lifecycle's physical state.
Expand Down Expand Up @@ -428,23 +443,42 @@ export async function releaseSystemRecordLaneControllerV1(
CONTROLLER_SESSIONS.delete(controller);
}

export function createSystemRecordLaneControllerV1(
deps: SystemRecordLaneControllerDepsV1,
/**
* Typed-only controller deps: the managed path's shape. 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. The compile-time can't-forget
* guard that `SystemRecordLaneControllerDepsV1.barrier` provides for external
* composers is provided here by `typedBarrier` being required.
*/
export interface SystemRecordLaneControllerTypedDepsV1 {
Comment thread
Jurij89 marked this conversation as resolved.
Outdated
readonly lease: ManagedOxigraphOwnershipLeaseV1;
readonly handoff: SystemRecordChildHandoffV1;
readonly executor: SystemRecordTransactionExecutorV1;
readonly typedBarrier: SystemRecordLaneTypedBarrierV1;
readonly setAdmissionActive?: (active: boolean) => void;
}

/**
* The typed-only core builder. Managed composition calls this directly;
* {@link createSystemRecordLaneControllerV1} is the compatibility adapter
* that normalizes the public legacy-capable deps down to this shape.
*/
export function createSystemRecordLaneControllerTypedV1(
deps: SystemRecordLaneControllerTypedDepsV1,
): SystemRecordLaneControllerV1 {
if (registeredController) throw new SystemRecordControllerRegistrationError();

const publicBarrier: SystemRecordLaneTypedBarrierV1 = deps.typedBarrier ??
((kind, transition) => deps.barrier(`system-record.${kind}`, transition));
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 deps.typedBarrier('enable', transition),
),
runVoidBarrier: (kind, transition) => publicBarrier(kind, transition),
runVoidBarrier: (kind, transition) => deps.typedBarrier(kind, transition),
});
const controller: SystemRecordLaneControllerV1 = Object.freeze({
open: (activation: SystemRecordLaneActivationV1) => session.open(activation),
Expand All @@ -455,6 +489,26 @@ export function createSystemRecordLaneControllerV1(
return controller;
}

/**
* Public compatibility entry point. Normalizes the legacy-capable deps to the
* typed core: a supplied `typedBarrier` is used as-is; otherwise the string
* `barrier` is wrapped, preserving the purpose-string contract for external
* composers until its removal at a breaking version boundary.
*/
export function createSystemRecordLaneControllerV1(
deps: SystemRecordLaneControllerDepsV1,
): SystemRecordLaneControllerV1 {
const typedBarrier: SystemRecordLaneTypedBarrierV1 = deps.typedBarrier ??
((kind, transition) => deps.barrier(`system-record.${kind}`, transition));
return createSystemRecordLaneControllerTypedV1({
lease: deps.lease,
handoff: deps.handoff,
executor: deps.executor,
typedBarrier,
setAdmissionActive: deps.setAdmissionActive,
});
}

/* ------------------------------------------------------------------ *
* The one aggregate session
* ------------------------------------------------------------------ */
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,9 +23,16 @@ vi.mock('../src/system-record-materializer-v1.js', async (importOriginal) => {
>();
return {
...actual,
createSystemRecordLaneControllerV1: (deps: never) => {
// The managed path builds on the TYPED-ONLY entry point (#2179): the
// coordinator imports createSystemRecordLaneControllerTypedV1 directly,
// so the injection must sit there to reach production composition. Note
// the scope honestly: the public compatibility adapter reaches the typed
// builder through a module-internal call, which a module mock cannot
// intercept — this injection covers the coordinator's imported route,
// which is the route production takes.
createSystemRecordLaneControllerTypedV1: (deps: never) => {
if (injected.error) throw injected.error;
return actual.createSystemRecordLaneControllerV1(deps);
return actual.createSystemRecordLaneControllerTypedV1(deps);
},
};
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2915,4 +2915,36 @@ describe('system-record lane session lifecycle V1', () => {
expect(session.state).toBe('shutdown');
});
});

describe('compatibility adapter with a failing string barrier (#2179)', () => {
// First-party composition is typed-only and structurally cannot reach the
// string path (the managed coordinator's deps have no string member).
// External composers still enter through this public adapter with string
// barriers, so the compat path's failure shape is pinned here: a barrier
// that throws at the boundary must fail CLOSED, not leave a half-enabled
// lane over a child nobody stopped.
it('a string barrier that throws at the boundary fails enable loudly and closed', async () => {
const controller = createSystemRecordLaneControllerV1({
lease: ownership.lease,
handoff,
executor,
barrier: () => {
throw new Error('external barrier infrastructure refused the transition');
},
});

await expect(controller.open(ACTIVATION)).rejects.toThrow(/refused the transition/);

// Closed, not just loud, in one assertion: the ONLY handoff interaction
// is the fail-closed order. The barrier threw before the transition ran,
// so no physical step happened — the child was never stopped, destroyed,
// or replaced under an enable that could not settle.
expect(handoff.calls).toEqual([
'failManagedMutationsClosed:enable transition did not physically settle',
]);
// The lane is terminally unavailable, matching every other failed-enable
// path in this file.
await expect(controller.open(ACTIVATION)).rejects.toThrow(/terminal/);
});
});
});
12 changes: 12 additions & 0 deletions packages/storage/tsconfig.typetests.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"noEmit": true,
"rootDir": "."
},
"include": [
"src",
"test/store-control-barrier-contract-v1.test.ts"
Comment thread
Jurij89 marked this conversation as resolved.
Outdated
],
"references": [{ "path": "../core" }, { "path": "../rdf-utils" }]
}
Loading