alkdev/alkstore
6
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ADR-021: third review round — tx-read methods, Job.claimed_at, schedule() queue validation, drop=rollback, receiver arms

Browse Source
This commit is contained in:
glm-5.3-flash committed 2026-10-07 06:25:39 +00:00
1 parent 8323a7853e
commit 08dc1bf011
15 files changed
+499 -29
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

docs/architecture/decisions/021-tx-reads-and-value-shape-fixes.md
+331
View File
@@ -0,0 +1,331 @@
# 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](008-contract-v1-pinning.md) §2/§8,
[ADR-019](019-mechanism-handle-surfaces.md) §1/§3/§6, and
[ADR-009](009-scheduler-collapse.md) §1 in place, pre-implementation
([ADR-017](017-contract-versioning.md) 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.)
## 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](008-contract-v1-pinning.md) §2, extended by
[ADR-014](014-outbox-tx-enqueue.md)/[ADR-015](015-streams-depth.md))
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](014-outbox-tx-enqueue.md) §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](010-queue-semantics-depth.md) §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](012-forked-substrate-design.md) §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](008-contract-v1-pinning.md)
§4), and [ADR-014](014-outbox-tx-enqueue.md) 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](009-scheduler-collapse.md)
§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 `commit`ed nor `rollback`ed —
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](007-transactional-seam.md)), 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](008-contract-v1-pinning.md) §8:
`recv() -> Option<Result<StreamEvent>>`, `save_offset(&mut self)
-> Result<()>`) but the v1 error taxonomy
([ADR-008](008-contract-v1-pinning.md) §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:
```text
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](019-mechanism-handle-surfaces.md) §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](014-outbox-tx-enqueue.md) §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](019-mechanism-handle-surfaces.md) §3's
pinned struct (the field list there restored to what
[ADR-010](010-queue-semantics-depth.md) §1 promised and the fork's
schema carries — [ADR-012](012-forked-substrate-design.md) §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](008-contract-v1-pinning.md) §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](014-outbox-tx-enqueue.md)'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](009-scheduler-collapse.md) §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](003-sqlite-driver.md)'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](../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](008-contract-v1-pinning.md) §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](004-postgres-driver.md)), 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](019-mechanism-handle-surfaces.md) §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](019-mechanism-handle-surfaces.md) §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](017-contract-versioning.md) §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](019-mechanism-handle-surfaces.md) — §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](008-contract-v1-pinning.md) — §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](010-queue-semantics-depth.md) §1 — the `get_job`
timestamp list whose `claimed_at` §2 restores into the struct.
- [ADR-014](014-outbox-tx-enqueue.md) — §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](009-scheduler-collapse.md) §1 — the `schedule()` entry
point whose queue argument §3 validates.
- [ADR-007](007-transactional-seam.md) — the seam the drop pin and
the tx reads ride; the no-ghosts property they extend.
- [ADR-003](003-sqlite-driver.md) — the writer-slot lease the drop
pin releases; [ADR-004](004-postgres-driver.md) — 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](../core-contract.md) — the spec this ADR's
§1–§5 pin into place.