Files
alkstore/docs/architecture/decisions/021-tx-reads-and-value-shape-fixes.md
T

18 KiB

ADR-021: Third review round — tx-side reads, Job.claimed_at, schedule() queue validation, tx-handle drop, receiver close/error arms

Status

Accepted (2026-10-07, Phase 1 — third architecture review round, pre-decomposition; amends ADR-008 §2/§8, ADR-019 §1/§3/§6, and ADR-009 §1 in place, pre-implementation (ADR-017 class 1 — no release exists; the amend-in-place frame is still open). Also records two factual corrections to accepted decisions found by the review — ADR-019 §1's tx-methods claim and ADR-019 §3's Job field list — corrected inline there, per this repo's convention that wrong statements in decisions are fixed where they stand, not left for readers to discover.) (Follow-through 2026-10-07, same round, second session: the review's remaining ambiguities — all decidable from decided material — were closed by single-sentence/annotation edits against the already-decided ADRs and specs, recorded in the annotations' own text rather than a new ADR: A4 (enqueue_tx stamps the derived engine defaults — annotated onto ADR-020 §3), A5 (stale sweep_expired(queue) shapes aligned to ADR-019 §1's handle form in core-contract.md and queues.md), retry -> bool + the refused-retry residue and the fail(None) default string (ADR-019 §3), the one-shot/repeatable split restated (core-contract), ack_batch's per-id predicate (ADR-010 §1), opts field types/defaults (core-contract, honker-rs parity), the consumer-local non-empty rules (ADR-008 §4), the with_tx signature (ADR-007), the PayloadTooLarge measurement basis (ADR-008 §5), per-stream offset-sequence equivalence (ADR-015 §4), the recv_timeout timeout arm and EventReceiver's no-recv_timeout posture (ADR-008 §3/§8).)

Context

The third review round (the pre-decomposition sweep after the second, which produced ADR-019/020) checked the spec set — the seven architecture documents and all twenty ADRs — for cross-document consistency and implementation-blocking ambiguity. Five findings were decidable from decided material; none opens a new design space:

  1. ADR-019 §1's sentence about tx-side reads was factually incorrect. It states: "get_job, save_offset, get_offset, read_* remain available from inside a transaction through the TxHandle's *_tx methods." The pinned TxHandle trait (ADR-008 §2, extended by ADR-014/ADR-015) carries only write-and-save ops (enqueue_tx, publish_tx, publish_with_key_tx, notify_tx, save_offset_tx, outbox_enqueue_tx) plus commit/rollback — there is no get_job_tx, get_offset_tx, read_since_tx, or read_from_consumer_tx. The capability the sentence claims is real and load-bearing: without tx-shaped reads, read-your-own- writes inside a business transaction is impossible on both engines (SQLite's reader pool cannot see the writer connection's uncommitted rows; a pg pool checkout may land on another connection without seeing the tx client's uncommitted writes). Honker provides the capability only through its raw Transaction::query_row/conn() escape hatch — the seam ADR-014 §3 deliberately rejected (downcast or core→engine dependency). So the inheritance claims named the capability without a mechanism: the review caught the claim outrunning the pinned surface.

  2. Job.claimed_at — ADR-010 §1 and ADR-019 §3 disagree. ADR-010 §1 pins get_job's timestamps as "created_at, run_at, claimed_at, expires_at"; ADR-019 §3's pinned Job struct omits claimed_at. ADR-019 is the struct of record and its list is the narrower, later pinning — but the fork's re-derivation adds the claimed_at column (ADR-012 §5), claim_expires_at (the field ADR-019 did carry) is meaningless without it, and claimed_at is the dual-execution window's diagnostic field (which worker's claim deadline was running when a job died). The omission was an error in ADR-019's field list, not a narrowing ADR-010 needed annotated.

  3. schedule()'s queue argument is an unvalidated write path into reserved names. Every directly-supplied name of every kind rejects the reserved prefix (ADR-008 §4), and ADR-014 rests its outbox guarantee on that: "there is no way to enqueue into the backing queue except through the outbox surface." But schedule(name, spec, queue, payload, opts) takes a queue name whose validation is unpinned (ADR-009 §1 validates the schedule name), and each boundary fire enqueues engine-side into whatever name was stored — schedule("x", "@every 60s", "__alkstore_outbox:foo", …) is a third write path into the outbox's backing queue, contradicting the guarantee as stated.

  4. The drop-of-un-committed-handle case is unpinned. A Box<dyn TxHandle> that is neither commited nor rollbacked — a dropped handle, an error path that exits early, a panic across the box — has no stated fate. This is observable behavior with direct interaction with the no-ghosts property (ADR-007), and on the pg side it determines whether a pooled client leaks.

  5. The EventReceiver close/error arms are unpinned. Its shape is pinned (ADR-008 §8: recv() -> Option<Result<StreamEvent>>, save_offset(&mut self) -> Result<()>) but the v1 error taxonomy (ADR-008 §5) never says which variants the Err arms can carry, what closes a pg-side receiver, and ADR-019 §6's save-offset bullet says "returns Result<()> — refusal (regression) is a no-op, not caller-actionable" without saying what the Err arm ever is.

All five resolve from the starting artifact, the pinned posture, and the existing POC ground — the same "decidable from decided material" basis the second round's findings were resolved on.

Decision

1. Tx-side reads: four methods join the TxHandle trait

ADR-019 §1's sentence is corrected rather than retracted — the capability stays, the mechanism is now pinned. The trait gains:

trait TxHandle {
    enqueue_tx(queue, opts, payload) -> job_id
    publish_tx(stream, payload) -> offset
    publish_with_key_tx(stream, key, payload) -> offset
    notify_tx(channel, payload)
    save_offset_tx(stream, consumer, offset)
    get_job_tx(queue, job_id) -> Option<Job>          // added
    get_offset_tx(stream, consumer) -> i64            // added
    read_since_tx(stream, offset, limit) -> Vec<StreamEvent>          // added
    read_from_consumer_tx(stream, consumer, limit) -> Vec<StreamEvent> // added
    outbox_enqueue_tx(outbox, opts, payload) -> job_id
    commit(self: Box<Self>) -> Result<()>
}
  • The tx read forms carry the same shapes as the auto-commit forms (Queue::get_job, StreamHandle::read_since / read_from_consumer / get_offset, ADR-019 §1) — pure reads, no ops boxed off them. They take the mechanism name as their first parameter (not a handle), mirroring save_offset_tx / enqueue_tx / publish_tx's name-taking shape.
  • The property pinned: in-tx reads read the caller's own transaction — a get_job_tx/read_since_tx issued on a handle sees exactly the rows that handle's transaction has written (the pg POC's read-your-writes probe, in-tx direction); after rollback those reads' subjects no longer exist (no-ghosts, uniformly, for reads as for writes).
  • The line stays where it was: reads and saves are tx-shaped; claim/maintenance ops are not. There is no claim_tx — claiming inside the caller's business transaction would tie a visibility deadline's lease to a business commit the caller controls, tangling the at-least-once machinery with an unrelated lifetime. Absence here is deliberate design, recorded so an implementer does not "fix" it.
  • Correction posture for the record: ADR-019 §1's original sentence asserted these methods as already-pinned surface; the assertion was wrong (the trait it cites never carried them, and honker's escape hatch was rejected in ADR-014 §3). The sentence stands corrected in place there, citing this ADR.

2. Job gains claimed_at: Option<i64> — None pre-claim

Corrected inline in ADR-019 §3's pinned struct (the field list there restored to what ADR-010 §1 promised and the fork's schema carries — ADR-012 §5):

  • claimed_at: Option<i64> — unix seconds at the row's most recent claim, None before any claim. Set on every successful claim (fresh or reclaim — a reclaim updates it), which is what makes it the dual-execution window's diagnostic (which worker's claim deadline was live when a job died) and the pairing field for claim_expires_at (meaningless without it).
  • The pairing closes a real decomposition hazard: ADR-010 §1's text and ADR-019 §3's struct were both "pinned" descriptions of one struct, disagreeing; tasks written off either text would have built different value types. Both now say the same thing; dead-row visibility (get_job on dead jobs, "dead rows included") carries the field with the rest of the stamps.

3. schedule()'s queue argument rejects the reserved prefix

The queue argument gets exactly the directly-supplied-queue-name validation (ADR-008 §4): non-empty else InvalidName, reserved-prefix else ReservedName, rejected at the entry point (registration), no engine round trip.

  • This closes the third write path the review found: a boundary fire can no longer enqueue into __alkstore_outbox:* because the only way to store that name in a schedule row is through the (now rejecting) entry point. ADR-014's stated guarantee — the backing queue is reachable only through the outbox surface — becomes true of the scheduler's fires as well; annotated there.
  • Unschedule/registration semantics untouched (ADR-009 §1); the schedule-name validation pinned there is unchanged, this pins its queue argument.

4. Dropping a TxHandle without commit or rollback rolls it back

Pinned: drop = rollback. A Box<dyn TxHandle> dropped without commit (or rollback) executes rollback on its way down; the no-ghosts property holds through Rust's RAII paths, not only through the explicit ones.

  • SQLite: the writer-slot lease releases with ROLLBACK before the slot frees (ADR-003's lease mechanics — a dropped handle cannot park the writer forever).
  • Postgres: the held pooled client's transaction rolls back; the client is usable and returns to the pool on the drop path (no leaked budget line; the max_size + 1 accounting in deployment.md stays accurate under error paths).
  • Precedent is the starting artifact's documented Drop behavior — honker-rs' Transaction rolls back on drop without commit/rollback (/workspace/honker packages/honker-rs/src/lib.rs @ f4e53c6, "on drop without either, the transaction rolls back") — inherited where every other tx-seam behavior was inherited.
  • The other three candidates were weighed and rejected: drop = commit (silently landing business writes on an error path — contradicts no-ghosts and every durable-transaction convention), drop = leak (pooled-client exhaustion on error paths; not honest RAII), drop = panic (panics in library code are banned family- wide). The rejected-with-reasons list is the record's value here — an implementer improvising a Drop impl would otherwise pick exactly one of them.

5. EventReceiver close/error arms pinned

The pinned shape's unpinned arms:

  • recv()'s Err arm carries Database only — the one variant a stream read can produce. Payloads cross the trait as raw bytes; payload_as<T> (where Codec lives) is a decode-side convenience on the yielded StreamEvent, so no Codec exists at recv; Closed is the None arm by construction (ADR-008 §3's close semantics, §5's taxonomy); no third variant can occur.
  • Close is terminal; what causes it is the documented engine asymmetry: SQLite's receiver closes on watcher death (POC-pinned, the WatcherDeathGuard surface); the pg engine's forwarder reconnects indefinitely (the reconnect-wake is its recovery channel — ADR-004), so its receivers close only at engine shutdown. Same shape as the wake-coalescing asymmetry: one call site (recv() -> None), documented occurrence difference, Closed/None uniform. A closed receiver never reopens; the recovery recipe is a fresh subscribe(consumer), which resumes from the saved checkpoint — the mechanism replay exists for.
  • save_offset's Err arm carries Database only — the monotone rule makes regression refusal a silent no-op (pinned semantics, ADR-019 §6), so the Result exists for storage failures; closes that ADR's dangling return type. A receiver asked to save before any event has been yielded performs a no-op save (nothing to checkpoint is not an error state; the stored checkpoint is untouched — get_offset's absent-consumer = 0 rule, ADR-019 §1, already defines the pre-save state).

Consequences

Positive

  • The contract surface's last known claim-vs-text mismatch closes: ADR-019's sentence, the Job struct, and the pinned trait text now say the same thing.
  • In-tx read-your-own-writes becomes a real capability (the outbox/ streams consumer reading back what it just wrote in the business transaction) instead of an ADR promise with no method behind it.
  • The drop path is pinned before implementation — the honest-RAII answer is also the no-ghosts answer and the honker-inherited answer, so nothing was traded away.
  • The outbox guarantee closes its last hole; the schedule surface cannot reach reserved names.
  • Receiver error matching is now writable engine-agnostically: match Database (and nothing else) in the Err arm, None as terminal.

Negative

  • Four more TxHandle methods for both engines to implement — the class-2-locked trait cost every tx method carries, priced in ADR-017 §5; they ride read_* / get_* SQL shapes both engines already own, so implementation is composition, not new machinery.
  • Job's field list widens again (one field) — non_exhaustive-governed growth from here, but the pinned initial text grows.
  • Drop = rollback makes forgotten handles silently undo work whose side effects may have already run inside the transaction — the same honesty every Drop-rollback transaction API carries; the loud alternative (drop = commit) is the dangerous one and was rejected for it.
  • Contract-suite surface grows: the new backlog rows below.

Verification backlog additions

  • In-tx read-your-own-writes on both engines (§1) — reads issued on the tx handle see the transaction's own writes (get_job_tx a row the same tx enqueued; read_since_tx an event the same tx published); another engine connection sees none of it pre-commit; post-rollback the reads' subjects are gone (no-ghosts for reads).
  • Drop = rollback on both engines (§4) — a handle dropped before commit leaves no job/event/notify/offset residue (no-ghosts via RAII); the SQLite writer slot is reusable and the pg client re-poolable afterward.
  • schedule() queue-argument validation (§3) — reserved-prefix and empty queue names rejected with ReservedName/InvalidName at registration, identically on both engines; the stored schedule fires into only non-reserved queue names.
  • Receiver error arms (§5) — recv()'s Err carries Database only; save_offset's Err carries Database only; regression saves are silent no-ops; SQLite receiver closes on watcher death (None, terminal); pg receiver stays open across forwarder reconnects (reconnect-wakes arrive; no close) — close pinned to shutdown only.

References

  • ADR-019 — §1's corrected sentence (the origin of finding 1), §3's corrected Job struct (finding 2), §1's get_offset absent-consumer rule and §6's monotone-save rule (§5 of this ADR builds on both).
  • ADR-008 — §2 (the trait shape this ADR's §1 extends), §4 (the reserved-prefix rule §3 extends to the schedule queue argument), §5 (the taxonomy the Err arms are pinned against), §8 (EventReceiver's shape whose arms §5 pins).
  • ADR-010 §1 — the get_job timestamp list whose claimed_at §2 restores into the struct.
  • ADR-014 — §3 (the raw-Transaction rejection that made finding 1 a real hole, and the guarantee §3 of this ADR completes), §1 (the name-taking tx-method shape the read forms mirror).
  • ADR-009 §1 — the schedule() entry point whose queue argument §3 validates.
  • ADR-007 — the seam the drop pin and the tx reads ride; the no-ghosts property they extend.
  • ADR-003 — the writer-slot lease the drop pin releases; ADR-004 — the pooled client and forwarder the close asymmetry and drop pin ride.
  • Honker-rs Transaction (/workspace/honker packages/honker-rs/src/lib.rs @ f4e53c6) — the raw-escape capability (reads without a tx method, §1's capability evidence) and the documented drop-rolls-back Drop behavior (§4's precedent), read and respectively re-mechanized / inherited.
  • core-contract.md — the spec this ADR's §1–§5 pin into place.