Skip to content

Spec: deepened Store interface #208

Description

@George-RD

Problem Statement

The kernel Store (crates/openspine-kernel/src/store/) is the widest interface in the workspace — ~150 pub methods across 40 impl Store files, StoreError with 37 variants — and it is a shallow module: the two invariants that make the Ledger promise hold ("audited before effect, crash-safe, can testify later") are enforced by convention at the call sites, not encapsulated behind the interface. Concretely:

  • conn is pub(crate) (store/mod.rs:170pub(crate) conn: Arc<Mutex<Connection>>). Any module in the crate can lock the raw connection and run arbitrary SQL, reaching around every Store guarantee. There is one production write bypass today (skill/review.rs:155, record_verdict opens its own Immediate transaction) plus production read bypasses (pipeline/mod.rs:905 hands &store.conn.lock() into a store fn; api/scoped_admission_support.rs usage_count/scheduled_timer_count/audit_count lock conn in non-test code).
  • BEGIN IMMEDIATE is hand-restated at ~40 write sites (59 transaction_with_behavior occurrences in store/). Each site re-derives the write-serialization discipline that D-050 established; a site that opens Deferred (or no transaction) instead is a silent write-skew / TOCTOU risk that compiles cleanly.
  • Audit-before-effect is hand-paired at 83 append_audit_conn(&tx, …) call sites over the store/audit_append.rs primitive (the module exports the discipline instead of encapsulating it, per AD-105 ledger-before-consume). An effect write that forgets its audit append — the Auditor's "an effect with no prior audit row" failure — compiles cleanly.
  • The store reaches up into a channel adapter. store/identity.rs:7 imports crate::telegram::VerifiedOwnerContext (owner-verification proof); store/worker_dispatch.rs:255 calls crate::telegram::telegram_chat_id() to compute a Telegram address for a dead-letter row whose column is chat_id INTEGER NOT NULL (store/failure_surfacing_types.rs:183/221). The privileged kernel-state module depends on an untrusted-ingress adapter, inverting the trust-boundary dependency direction (AGENTS.md).

Consequences (by user / promise):

  • Auditor (Ledger): the "effect has a prior audit row" and "the record can testify" guarantees rest on 83 unenforced conventions; a single missed pairing is invisible until an audit query comes up short months later.
  • Unattended workhorse (Ledger): with no principal present, a wrong transaction mode or a skipped audit on an internal-trigger path (replay firing an effect twice) has no reviewer to catch it — the discipline must hold structurally.
  • Lyra (Ledger): "delegation state is wrong after a crash" is exactly what crash-safe, ledger-before-consume writes prevent; the guarantee cannot depend on every future caller re-typing BEGIN IMMEDIATE correctly.

This is review candidate 2 (.raw/openspine-architecture-review-2026-08-20.md), ruled land → design ticket by the decision test. It is load-bearing for the Effect Truth settlement writes (#173/#174/#198) and for a later identity/tenancy audit dimension.

Solution

Deepen Store so the two Ledger invariants move inside the implementation and a wrong ordering stops compiling. Four seams:

  1. Transaction seam. A single private combinator with_immediate_tx(|tx| …) locks conn, opens TransactionBehavior::Immediate, runs the closure, commits. All ~40 write sites route through it. The one Deferred read path (store/audit_support.rs::verify_and_replay_aggregate) becomes a separate with_deferred_read combinator. conn becomes private, so no site can restate the transaction discipline.
  2. Audit-before-effect seam. A with_audited_effect(audit_descriptor, |tx| effect) combinator appends the audit row inside the same Immediate transaction and is the only entry that can also write an effect row on the effect path — an effect-only write does not compile there. Audit-only and non-effect internal-maintenance writes use separately named, explicit entries. Two shapes are formalized: (a) rollbackable effects commit the effect row and the audit row atomically; (b) non-rollbackable external effects commit state + audit + fence first, then the caller performs the external effect.
  3. Encapsulation seam. conn becomes a private field. The production write bypass in skill/review.rs::record_verdict migrates to Store::insert_eval_verdict; the production read bypasses migrate to Store methods. Test-only raw access is provided through store/test_hooks.rs. (The ~9 raw-SQL store/*_tests.rs files are migrated by the fault-seam track Add test seams for the two UNPROVEN in-transaction erasure rechecks and the Telegram callback pending-write fence #177/Fold the fault-seam evidence into #177 #185, referenced here, not duplicated.)
  4. Layering seam. crate::telegram::VerifiedOwnerContext is replaced by a channel-neutral zero-size capability token OwnerVerifiedProof in a neutral module, minted only by channel adapters after their own verification. crate::telegram::telegram_chat_id disappears from the store: the dead-letter row persists the channel-neutral OwnerSurfaceRef (this change owns the store-side chat_id INTEGERowner_surface_json column migration, since it owns the store schema), and the Telegram adapter / retry worker resolves the address at delivery. The store → telegram dependency edge is removed.

Two extension points are left open, defined but not filled:

Key design choices:

  • Combinators are private; the seam is Store's public write API. Below the seam, transactions and audit pairing are mandatory and unrestatable. Above it, callers pass intent + an audit descriptor + channel-neutral references (OwnerSurfaceRef, OwnerVerifiedProof, PrincipalId) and never a raw Connection or a channel address.
  • Expand-contract migration. The interface is added alongside the existing paths (expand), call sites move across incrementally, and the pub(crate) conn escape hatch and the store → telegram imports are removed last (contract), so the tree stays green per commit (scripts/check.sh).

User Stories

Auditor: every effect provably has a prior audit row

As an adversarial reader of the record months later, I want the "audited before effect" invariant enforced by the type system, so that no effect write path can commit without its audit row regardless of who wrote the caller.

Acceptance: effect writes on the effect path go through with_audited_effect; an effect-only write on that path does not compile. A test proves no effect row can be committed without an audit row in the same transaction. Promise: Ledger.

Unattended workhorse: crash-safe writes without a reviewer

As an internal-trigger path with no principal present, I want BEGIN IMMEDIATE write-serialization and ledger-before-consume ordering enforced inside the Store, so that a wrong transaction mode on an overnight path cannot fire an effect twice or skip an audit.

Acceptance: all store write sites route through with_immediate_tx; conn is private so no path can open a Deferred write or restate the discipline. The Deferred read path is a separate, explicit combinator. Promise: Ledger.

Lyra: delegation state survives a crash and the store stops depending on Telegram

As the trusted owner's assistant, I want the kernel-state Store to never depend on a channel adapter, so that owner-verification and owner-addressing are carried by channel-neutral, principal-bound handles that survive a crash and a channel change.

Acceptance: no crate::telegram:: import remains in store/; owner verification is carried by OwnerVerifiedProof, owner addressing by OwnerSurfaceRef. The dead-letter row persists a neutral surface binding, not a raw chat id. Promise: Ledger (with Switchboard neutrality).

Implementation Decisions

D-001: One private with_immediate_tx transaction combinator

Decision: add Store::with_immediate_tx<T>(&self, f: impl FnOnce(&Transaction) -> Result<T, StoreError>) -> Result<T, StoreError> that locks conn, opens TransactionBehavior::Immediate, runs f, and commits on Ok / rolls back on Err. Route all ~40 store write sites through it. Add a separate with_deferred_read for the one read path (store/audit_support.rs::verify_and_replay_aggregate).

Why: removes 59 hand-restatements of BEGIN IMMEDIATE; centralizes the D-050 write-serialization / TOCTOU-closure discipline (cf. D-167, D-169) in one place. Authority: none added — same authority as today. Containment: the combinator is private; conn is private. Audit: unchanged for pure state writes. Failure mode: a closure error rolls back atomically; a write opened Deferred by mistake is now impossible on the write path.

D-002: with_audited_effect pairs effect + audit in one transaction (type-enforced)

Decision: add Store::with_audited_effect(&self, audit: AuditDescriptor, effect: impl FnOnce(&Transaction) -> Result<EffectRows, StoreError>) that runs effect and append_audit_conn inside one Immediate transaction and commits both atomically. It is the only entry on the effect path that can write an effect row; effect-only and audit-only and internal-maintenance writes use separately named entries. Two shapes: (a) rollbackable — effect + audit atomic; (b) non-rollbackable external — see D-003.

Why: encapsulates the 83 hand-paired append_audit_conn sites; makes the Auditor's "effect with no prior audit row" failure a compile error on the effect path (AD-105 ledger-before-consume). Authority: none added. Containment: audit descriptor is data; no caller supplies a raw Connection. Audit: the audit row is folded into the same tx, durable before the call returns. Failure mode: a failed audit append rolls back the effect; a caller that skips the audit cannot reach the effect-write API.

D-003: begin_effect / settle_effect(disposition) seam for non-rollbackable external effects (extension point)

Decision: define a two-phase seam on the audit-before-effect combinator — begin_effect commits state + audit + a pending-write fence in one Immediate tx; the caller performs the external effect; settle_effect(disposition) writes the settlement (finalize / cancel / retain+fence), each paired with its audit row. The store owns the fence and settlement writes, never the connector call. This change defines the seam shape only.

Why: the Effect Truth track (#173/#174, epic #198) needs a Store-owned place for disposition-driven settlement writes; without it, settlement interpretation leaks back to callers. Authority: none. Containment: the external effect stays outside the store. Audit: every settlement transition emits its audit row inside a Store tx. Failure mode: DeliveryUnknown retains + fences (no duplicate send for the Unattended workhorse); classification logic is #198's, not this change's.

D-004: conn becomes private; production bypasses migrate to Store methods

Decision: change conn from pub(crate) to private. Migrate the production write bypass skill/review.rs::record_verdict to Store::insert_eval_verdict. Migrate the production read bypasses (pipeline/mod.rs:905, api/scoped_admission_support.rs::{usage_count,scheduled_timer_count,audit_count}, pipeline/standing_rule_timer.rs, reflection_miner_runtime) to Store methods. Provide test-only raw access via store/test_hooks.rs (#[cfg(test)]).

Why: closes the escape hatch that lets any crate module reach around every Store guarantee. Authority: none. Containment: the raw Connection is unreachable outside store; only #[cfg(test)] hooks expose it. Audit: the write bypass no longer sidesteps with_audited_effect. Failure mode: a new non-store site that needs raw SQL now fails to compile, forcing a Store method or a documented test hook. The ~9 raw-SQL store/*_tests.rs files are migrated by #177/#185, not here.

D-005: Channel-neutral OwnerVerifiedProof, constructor restricted to channel adapters

Decision: define a zero-size capability token OwnerVerifiedProof in a neutral module, with its constructor visibility restricted to channel adapters (each mints it after its own owner verification), replacing crate::telegram::VerifiedOwnerContext. store/identity.rs::owner_assert_identity_binding takes &OwnerVerifiedProof.

Why: preserves the existing static "owner-verified" guarantee (a value of this type cannot be forged by generic code) while removing the store → telegram type dependency; mirrors the archived 2026-08-08-add-channel-neutral-responsibility-review pattern and the existing neutral OwnerSurfaceRef. Authority: the proof is a capability, not authority — identity carries no authority (D-006). Containment: only adapters construct it. Audit: unchanged. Failure mode: a runtime boolean/string (rejected) would move the check to runtime and weaken the guarantee.

D-006: Store owns the chat_idOwnerSurfaceRef dead-letter column migration; cut telegram_chat_id

Decision: the dead-letter row (store/failure_surfacing_types.rs, chat_id INTEGER NOT NULL) is migrated to persist the channel-neutral OwnerSurfaceRef (serialized owner_surface_json, matching the task_grants precedent). store/worker_dispatch.rs::surface_stranded_worker stops calling crate::telegram::telegram_chat_id; the Telegram adapter / retry worker resolves the address at delivery via OwnerSurfaceRef::surface_id. A versioned migration (AD-139, PRAGMA user_version) carries existing rows; a legacy row with no resolvable neutral surface fails closed rather than fabricating one (the hydrate_task_grant precedent).

Why: removes the last store → telegram reach and the Telegram-shaped column from kernel state. Authority: none. Containment: raw channel addresses live only in adapter code. Audit: dead-letter surfacing still audits owner.notify_failed. Failure mode: an unresolvable neutral surface fails closed. The owner-surface track #129/#184 is the consumer of this neutrality (it inventories the broader leak set); this change owns only the store-schema column, referenced not duplicated.

D-007: Reserve the identity audit dimension, aligned to typed owner identity (#197)

Decision: extend the audit posture beyond the single identity.bound kind: carry the actor via AuditEvent.actor: Option<PrincipalId> (as designed in sibling spec #197 D-003, delivered by #201) rather than in reason strings, and add audit kinds for bootstrap binding and owner-config-mismatch rejection. Resolution (get_identity, principal_exists) stays read-only; identifiers stay hash-only (value_hash). The identity-dimension ticket is wired blocked_by #201.

Why: lets a later tenancy/identity audit query filter by actor without reopening the Store interface. Authority: none — identity carries no authority (D-006). Containment: no raw identifiers persisted. Audit: owner facts move from opaque reason strings to a typed dimension. Failure mode: resolution must not gain a write path (read-only invariant).

Testing Decisions

This change governs audit ordering and crash-recovery, so it is authority-sensitive (audit + recovery). Per the repo's authority-sensitive conventions the downstream tickets MUST include, at minimum:

  1. No effect without audit (compile + runtime): a store unit test asserts that with_audited_effect commits the effect row and the audit row in the same transaction, and that a forced audit-append failure rolls back the effect row (no orphan effect). The type-level guarantee (effect-only write does not compile on the effect path) is exercised by the migrated call sites building green.
  2. Write-serialization preserved: the D-050 / D-167 concurrent-write regression tests still pass with writes routed through with_immediate_tx; a concurrency test asserts two racing writers serialize (no write skew).
  3. conn is unreachable: a grep/lint verification task asserts no non-store production module references store.conn / .conn.lock() (only #[cfg(test)] test_hooks), and no store/*.rs production file imports crate::telegram::.
  4. Layering cut: a test asserts surface_stranded_worker succeeds without any Telegram symbol, persisting an OwnerSurfaceRef, and fails closed on an unresolvable neutral surface; the dead-letter migration upgrades an existing chat_id row.
  5. Audit chain still verifies end-to-end: verify_audit_chain passes after the migration; an E2E crash-recovery test (ledger-before-consume) still holds.
  6. Verification commands: ./scripts/check.sh green per commit (builds openspine-shell binary first, per AGENTS.md); the fault-seam and raw-SQL test-file migration is delegated to Add test seams for the two UNPROVEN in-transaction erasure rechecks and the Telegram callback pending-write fence #177/Fold the fault-seam evidence into #177 #185.

Out of Scope

Further Notes

Invariants preserved: AD-105 (ledger-before-consume, aggregate seq under the lock); D-050 / D-167 / D-169 (BEGIN IMMEDIATE write-serialization, in-transaction recheck, no preflight-only TOCTOU); D-006 (identity carries no authority); AD-139 (versioned migration with PRAGMA user_version); the five-crate trust-boundary dependency direction (AGENTS.md).

Coordination: aligns to #197 (typed owner identity) for AuditEvent.actor/PrincipalId; hands the settlement seam to #198 (Effect Truth); hands the broader owner-surface leak inventory to #129/#184; hands the raw-SQL test migration to #177/#185.

Lane context: wayfinder map #182; design ticket #189; review candidate 2 (.raw/openspine-architecture-review-2026-08-20.md). Users: Auditor, Unattended workhorse, Lyra. Promise: Ledger.


🤖 Deepened Store interface spec (inline, self-contained) — from wayfinder design ticket #189, map #182.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions