Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
33 changes: 22 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,28 @@
// 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 string barrier is POISONED in managed composition, not
// supplied (#2179). Every production lifecycle call goes through
// `typedBarrier` below, so this callback is reachable only through
// the controller's legacy fallback — i.e. only if a future edit
// removes `typedBarrier` from these deps or reorders the fallback.
// Supplying the real string barrier here would make that edit
// SILENT: transitions would keep working while quietly moving onto
// the deprecated purpose-string contract with its unsound result
// cast. A loud throw turns the same edit into an immediate, named
// failure. The member itself stays required on the deps interface —
// required-ness is the compile-time guard that already caught a
// barrier shipping with zero callers once in this stack's history —
// and external composers keep supplying real string barriers until
// the deprecated contract is removed at a breaking version.
barrier: () => {
Comment thread
Jurij89 marked this conversation as resolved.
Outdated
throw new Error(
'system-record managed composition retired the purpose-string barrier (#2179): ' +
'lifecycle transitions must run through typedBarrier and the scheduler\'s ' +
'runTypedControlBarrier. Reaching this throw means the typed path was ' +
'removed or bypassed in sparql-http\'s controller deps.',
);
},
typedBarrier: (kind, transition) =>
runTypedBarrier(SYSTEM_RECORD_BARRIER_KEYS_V1[kind], transition),
setAdmissionActive: (active) => { this.systemRecordAdmissionActive = active; },
Expand Down Expand Up @@ -1428,7 +1439,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 1442 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 1442 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
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
23 changes: 19 additions & 4 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`.
* Managed composition already supplies a POISONED callback here (throws on
* use) so the retired path cannot silently re-animate. Removal of this
* member — and the required/optional flip that makes `typedBarrier` the
* mandatory one — happens together at the next allowed breaking version
* boundary, not before: both are source-incompatible for external
* composers.
*/
readonly barrier: SystemRecordLaneBarrierV1;
/** Optional typed path; the string callback above remains the compatibility contract. */
/**
* The production lifecycle path. 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
36 changes: 36 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,39 @@ 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;

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

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 @@ -2915,4 +2915,40 @@ describe('system-record lane session lifecycle V1', () => {
expect(session.state).toBe('shutdown');
});
});

describe('poisoned string barrier under the legacy fallback (#2179)', () => {
// Managed composition routes every lifecycle call through `typedBarrier`
// and supplies a string barrier that THROWS. The controller's fallback
// reaches that string barrier only when `typedBarrier` is absent — i.e.
// only if a future edit removes it from the production deps. This pins
// what such an edit produces: an immediate, NAMED failure, instead of the
// silent alternative where transitions keep working while quietly moving
// onto the deprecated purpose-string contract and its unsound result cast.
it('a lane composed with only a poisoned barrier fails enable loudly, named, and closed', async () => {
const controller = createSystemRecordLaneControllerV1({
lease: ownership.lease,
handoff,
executor,
// The same shape sparql-http supplies in managed composition.
barrier: () => {
throw new Error(
'system-record managed composition retired the purpose-string barrier (#2179)',
);
},
});

await expect(controller.open(ACTIVATION)).rejects.toThrow(/#2179/);

// Closed, not just loud, in one assertion: the ONLY handoff interaction
// is the fail-closed order. The poison threw at the barrier boundary, so
// no physical step ran — 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