feat(storage): centralize system-record runtime authority - #2140
Conversation
| : Object.freeze([]) as OwnedSubjectTableObjectV1; | ||
|
|
||
| if (authority.equalHead) { | ||
| if (authority.equalHead && !(snapshot.state === 'present' |
There was a problem hiding this comment.
🟡 Issue: Model rematerialization before the equal-head branch instead of negating it at the call site
What's wrong
The change threads rematerialization through the already-applied path as a special-case exception, so equalHead no longer means what its name suggests unless the reader also knows about the snapshot epoch helper. This spreads one state-machine decision across two modules and makes future changes to the equal-head path easier to get wrong or duplicate.
Example
For a present snapshot from an older applied tuple epoch, classifyAuthorityAdvance still returns { outcome: 'advance', equalHead: true }; the caller then has to reinterpret that as not actually reusable via !(snapshot.state === 'present' && requiresSystemRecordSnapshotRematerializationV1(snapshot)).
Suggested direction
Fold epoch freshness into classifyAuthorityAdvance, return a richer outcome such as already-applied versus advance, or make the snapshot model distinguish reusable present state from present-but-needs-rematerialization. That would let this call site stay direct and remove the double-negative special case.
For Agents
Refactor around classifyAuthorityAdvance and the present-snapshot epoch model. Preserve behavior for equal current-epoch heads and prior-epoch rematerialization, but make the classifier or snapshot type express whether the persisted tuple is reusable. Keep the existing rematerialization test proving a prior-epoch equal head advances to a fresh tuple.
There was a problem hiding this comment.
🟡 Issue: Fold rematerialization into the authority classification instead of special-casing equal heads
What's wrong
The new condition bolts an epoch-rematerialization exception onto an already busy control path. Readers now need to understand that equalHead does not mean “already applied”; it means “same head, unless the snapshot tuple is from an older durable epoch.” That weakens the abstraction boundary between snapshot freshness and authority advancement and makes future tuple states likely to add more compound exceptions here.
Example
For a present snapshot with the same head but appliedTupleEpoch !== materializationEpoch, classifyAuthorityAdvance reports equalHead: true; line 235 then says this equal head must not take the already-applied path because it needs rematerialization. That behavior may be right, but the model now says both “equal head” and “not reusable” in separate places.
Suggested direction
Have the classifier or snapshot model expose the actual decision the caller needs, such as equalCurrentHead, rematerializeEqualHead, or canReusePersistedMaterialization, instead of returning equalHead and then negating it with a separate epoch freshness check at the call site.
For Agents
Look at classifyAuthorityAdvance, requiresSystemRecordSnapshotRematerializationV1, and the already-applied branch in deriveSystemRecordActiveReplacementV1. Preserve the prior-epoch rematerialization behavior, but move the freshness decision into the authority/snapshot classification so the main derivation branches on a direct state like alreadyApplied vs rematerializeEqualHead. Keep the existing prior-epoch next-state test passing.
There was a problem hiding this comment.
Validated as a maintainability refinement, not a correctness blocker for this default-off runtime foundation. Deferred explicitly to #2155, which now requires the authority decision to expose a direct reusable/rematerialize outcome before activation.
There was a problem hiding this comment.
🟡 Issue: Make equal-head rematerialization an explicit state, not a hidden exception
What's wrong
This adds a special-case conditional into the busiest part of the derivation function. The reader has to know that equalHead sometimes means already-applied and sometimes means rematerialize, depending on epoch state owned by a different module. That is exactly the kind of branching growth that will make future transition cases harder to place correctly.
Example
Equal head at the current epoch enters the already-applied branch; equal head from a prior tuple epoch falls through into the normal advance/rematerialization path. That distinction is real, but it is hidden in a compound condition instead of being named by the authority/snapshot classification.
Suggested direction
Reframe the branch so the top-level flow dispatches on a named derivation state rather than equalHead && !(present && requiresRematerialization(...)).
For Agents
In system-record-next-state-v1-internal.ts, move the prior-epoch equal-head classification into a named model: for example have classifyAuthorityAdvance return advanceKind: 'new-head' | 'rematerialize-equal-head' | 'already-applied-candidate', or compute a clearly named shouldRematerializeEqualHead before branching. Preserve the current already-applied and rematerialization behavior, and keep the existing tests for equal-head prior epoch/current epoch passing.
| } from '@origintrail-official/dkg-core/system-record-v1'; | ||
|
|
||
| export interface SystemRecordRuntimeReservationGateV1 { | ||
| acquire(owner: object, bytes: number): void; |
There was a problem hiding this comment.
🟡 Issue: Make the reservation gate own the lease token instead of accepting release metadata
What's wrong
The new gate is supposed to encapsulate runtime accounting, but its interface pushes the exact release identity and byte count back onto the caller. That keeps wrong-owner and partial-release states in the contract, then compensates with runtime checks. A smaller, more maintainable abstraction would make those combinations impossible and remove bookkeeping from the registry caller.
Example
Current shape: the registry manufactures an owner object, calls reservationGate.acquire(reservation.identity, reservation.bytes), then must later repeat the exact pair in reservationGate.release(reservation.identity, reservation.bytes). The new tests explicitly exercise wrong-owner and partial-release attempts, which is a sign the abstraction is exposing invalid states instead of making them unrepresentable.
Suggested direction
Change the gate API from acquire(owner, bytes) / release(owner, bytes) to an opaque lease/token model, for example const token = gate.acquire(bytes); gate.release(token). The registry can still expose a separate opaque reservationIdentity in facts if needed, but the accountant should not require callers to reconstruct its internal release key.
For Agents
Look at system-record-reservation-gate-v1-internal.ts and the reservation calls in system-record-verified-replacement-v1-internal.ts. Preserve the single nonqueued process-wide/accounted-byte behavior, but have acquire return an opaque reservation/token that captures owner/bytes internally, and release by that token or by a token method. Existing reservation lifecycle tests should still prove exact-once release and cross-runtime exclusion.
There was a problem hiding this comment.
Validated as a worthwhile API hardening refactor, not a behavioral blocker for the default-off foundation. Deferred explicitly to #2155, which now requires an opaque gate-owned reservation token while preserving exact-once release and process-wide exclusion.
There was a problem hiding this comment.
🟡 Issue: Return an opaque reservation token instead of exposing owner/byte bookkeeping
What's wrong
The new abstraction is too thin: it extracts accounting state into a separate module but leaves the validity protocol spread across both sides. That makes the boundary more fragile than the original local state because the type system cannot express “release exactly the reservation that was acquired.”
Example
gate.acquire(firstOwner, bytes); gate.release(firstOwner, bytes - 1) is representable through the public internal interface and only fails dynamically. The caller should not need to remember the exact owner/byte pair to release a reservation it just acquired.
Suggested direction
Let the gate own its reservation record and expose a token-based API, or collapse the gate back into the registry if there is only one meaningful caller. Either option removes the primitive protocol between modules.
For Agents
In system-record-reservation-gate-v1-internal.ts and system-record-verified-replacement-v1-internal.ts, make acquire(bytes) return an opaque/branded reservation capability that the gate owns, and release by that token. Keep the one-live-reservation and max-accounted-bytes behavior, but remove the raw owner/bytes round trip from the registry/gate boundary.
| // the storage boundary. Every adapter for this lease receives the same | ||
| // registry, and all authentic leases share one process-wide accountant. | ||
| // The issuer remains outside the store and has no production caller in | ||
| // this default-unused stack; the later lifecycle verifier captures it. |
There was a problem hiding this comment.
🟡 Issue: Adapter wiring is not verified with a lease-issued proof
What's wrong
The PR's key storage-boundary behavior is that the adapter receives the same lease-owned runtime consumer that the verifier issuer will use. Current tests exercise the runtime directly and only check that the adapter rejects a forged object, which would also pass if the adapter were wired to the wrong private registry. That leaves the changed integration contract unverified.
Example
A regression that accidentally changed the adapter back to a fresh private registry would still reject Object.freeze({}) and the direct runtime tests would still pass, but a handle issued by resolveOwnedSystemRecordRuntimeV1(ownership.lease).issuer would fail through session.applyVerified(handle) instead of being consumed by the adapter's atomic executor.
Suggested direction
Cover the production composition, not only the resolver in isolation: the test should prove the store consumes handles from the same lease-bound runtime it resolves here.
Confidence note
I found direct tests for the lease runtime and for the atomic executor, plus an adapter test that only sends a forged object. I did not find an adapter test that sends a valid handle issued from the lease-bound runtime.
For Agents
Add an adapter-level regression test around SparqlHttpStore.getSystemRecordLaneControllerV1: create a managed lease, resolve its runtime, issue a valid active replacement handle with the same lifecycle binding, pass it to an open session's applyVerified, and assert the path reaches the atomic executor behavior rather than returning capability-lost for an unrecognized registry.
There was a problem hiding this comment.
Validated as a missing integration-evidence case rather than an identified runtime defect. Added the exact lease-issued-handle-through-adapter regression to #2154; the current direct runtime, adapter rejection, atomic executor, and live ownership gates remain green.
2 participants
Summary
This is the runtime-authority portion of Stack B4. It does not enable signed system-record sync. Same-version fork/root-collision quarantine and the live maximum-size/abort/lost-response activation evidence remain explicit gates before Stack C.
Related
feat/2052-system-record-atomic-apply).Diagrams
Runtime authority and reservation ownership
Before:
sequenceDiagram participant C as Public caller participant A as Managed adapter A participant B as Managed adapter B participant RA as Local registry A participant RB as Local registry B C->>A: Open activation descriptor A->>RA: Create isolated proof registry C->>B: Open activation descriptor B->>RB: Create isolated proof registry Note over RA: Own reservation state Note over RB: Own reservation stateAfter:
sequenceDiagram participant C as Public caller participant A as Managed adapter A participant B as Managed adapter B participant R as Lease runtime participant G as Process gate C->>A: Open activation descriptor A->>R: Resolve proof runtime by lease B->>R: Resolve proof runtime by lease R->>G: Reserve one 12 MiB atomic lease R->>G: Retain during recovery R->>G: Release after settlement Note over C,A: Activation contract is unchangedDurable epoch recovery
Before:
sequenceDiagram participant Runtime participant Decoder participant Deriver participant Store Runtime->>Decoder: Read prior tuple and current epoch Decoder-->>Runtime: Reject epoch binding Note over Deriver: No bounded rematerialization pathAfter:
sequenceDiagram participant Runtime participant Decoder participant Deriver participant Store Runtime->>Decoder: Read prior tuple and current epoch Decoder-->>Deriver: Authentic prior-epoch snapshot Deriver->>Store: One CAS replacement at epoch N Store-->>Runtime: Current-epoch receipt and state revisionFiles changed
packages/storage/src/system-record-reservation-gate-v1-internal.tspackages/storage/src/system-record-runtime-v1-internal.tspackages/storage/src/system-record-verified-replacement-v1-internal.tspackages/storage/src/adapters/sparql-http.tspackages/storage/src/system-record-materializer-v1.tspackages/storage/src/system-record-state-snapshot-v1-internal.tspackages/storage/src/system-record-next-state-v1-internal.tspackages/storage/test/system-record-*.test.tsTest plan
pnpm --filter @origintrail-official/dkg-storage buildpnpm exec vitest run test/system-record-state-snapshot-v1.test.ts test/system-record-next-state-v1.test.ts(22 passed)pnpm exec vitest run test/blazegraph.unit.test.ts test/oxigraph-worker-resilience.test.ts test/oxigraph-worker-respawn.test.ts --maxWorkers=1(68 passed)pnpm exec vitest run --maxWorkers=1frompackages/storage(808 passed,26 integration-only skipped)pnpm -r --filter @origintrail-official/dkg... --filter '!@origintrail-official/dkg-evm-module' run buildpnpm typecheck:live:system-record-managed-ownershippnpm test:live:system-record-managed-ownership(PASS: 27 checks,3 predecessor entries)git diff --check