Files
alkstore/tasks/release-findings-2-3-disposition.md
T
glm-5.3-flash c3a0c54df0 tests: review 003 Findings 2-3 — pre-release dispositions
Finding 2(a) landed: a one-shot cfg(test) generation-top LISTEN-pause
seam on the pg forwarder (listen_pause in seam.rs) lets the LISTEN-window
death test kill the fresh reconnect generation's backend before its
snapshot LISTEN — the LISTEN then fails honestly (57P01 terminating) and
the production fall-through arm (listen_ok == false) runs unmodified.
The harness test pins the honest mid-reconnect transient, full recovery
(wake, re-issue recovery of the refused mid-gap registration), and the
exactly-one-wake differentiator; a server-less unit pin covers the flag.
Finding 2(b) + the defensive arms stay parked with the code-read as pin,
recorded in the task file with current line refs.

Finding 3 landed: the rollback_fault seam (commit-fault mechanics) feeds
SQLITE_FULL-shaped errors into both ROLLBACK failure drop paths —
SqliteTxHandle::drop and the cancelled op's ConnLease::drop — exercised
in one test (phases: open-handle drop; single-poll-then-drop mid-op
cancellation), pinning slot replenish via reopen, no ghosts, and the
store staying usable.
2026-10-11 00:48:21 +00:00

246 lines
13 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
---
id: release-findings-2-3-disposition
name: Review 003 Findings 2–3 — the parked pre-release dispositions
status: completed
depends_on: []
scope: narrow
risk: low
impact: component
level: implementation
tags: [wave-8, tests, sqlite-engine, postgres]
---
## Description
Review 003's Findings 2–3, parked to the pre-release pass by the
wave-6 gate (`docs/reviews/003-wave-5-general-review.md`; the park
trail: `tasks/review-wave-6.md` §Rider closed). The review's own
recommendation shapes the task — the cheap seams land, the
race-shaped arm stays parked with its disposition recorded, so the
next general review does not re-derive the inventory:
- **Finding 2 (LOW, pg forwarder narrow untested arms)**: land the
cheap seam for (a) — the "connection died before/during the LISTEN"
fall-through to the reconnect path (`forwarder.rs:693-698`) — using
the house `cfg(test)` seam pattern (`pg-fix-forwarder-reconnect`'s
config seam; `sqlite-commit-error-arm`'s one-shot flag). The
mid-recv-loop stale-command abandonment race arm (b)
(`forwarder.rs:719-727`) and the small defensive arms
(`mid_listen_error` ack, forwarder-gone, shutdown abort/await) stay
parked with the code-read as their pin — **record that disposition
explicitly** (the review's "either way the disposition should be
recorded").
- **Finding 3 (LOW, SQLite rollback-failure-on-drop arms)**: a
rollback-fault arm on the existing commit-fault seam
(`alkstore-sqlite/src/seam.rs` — the `sqlite-commit-error-arm`
machinery) covering both drop arms in one small test:
`SqliteTxHandle::drop`'s failed-`ROLLBACK` path (`tx.rs:439-447`) and
`ConnLease::drop`'s (`tx.rs:166-175`) — pin the same properties the
commit test pins (error surfaced/printed, connection dropped, writer
slot replenished, store usable).
If a seam proves more invasive than the review estimated, the
acceptable fallback is the code-read-as-pin disposition for that arm —
recorded, not silent. The line between "landed" and "parked with
disposition" is this task's judgment call, made explicitly in Notes.
## Acceptance Criteria
- [x] Finding 2's (a) arm seam-tested (or its not-taken disposition
recorded with the invasiveness reason); the race arm + defensive
arms' parked disposition recorded
- [x] Finding 3's rollback-fault test landed covering both drop arms
(or its not-taken disposition recorded)
- [x] The disposition record written so a future general review reads
the inventory as closed, not re-derivable
- [x] `cargo test -p alkstore-sqlite` green server-less; `cargo test
-p alkstore-postgres` green server-less (SKIP posture) and — if
the pg seam landed — green against the harness server; clippy
`-D warnings`; fmt clean
## References
- docs/reviews/003-wave-5-general-review.md §Finding 2, §Finding 3
- tasks/review-wave-6.md (the park record)
- tasks/sqlite-commit-error-arm.md (the seam pattern + the commit-arm
test this mirrors)
- tasks/pg-fix-forwarder-reconnect.md (the pg config-seam precedent)
## Notes
### Finding 2(a) — landed: the generation-top LISTEN-pause seam
**Both Findings 2(a) and 3 taken — landed, not parked. The parked
dispositions below are recorded so the next general review reads this
inventory as closed.**
- **Finding 2(a) — taken.** The "connection died before/during the
LISTEN" fall-through (`alkstore-postgres/src/forwarder.rs`'s
`listen_ok == false` arm, review-cited at 693–698 pre-diff) is
seam-tested. The seam is a one-shot `cfg(test)` flag
(`seam.rs`'s `listen_pause`, the commit-fault shape):
`Forwarder::arm_generation_listen_pause()` arms it, and the loop
sleeps a 500 ms window at the **next** generation's top *before*
issuing its snapshot LISTEN. The test kills the fresh reconnect
generation's backend inside that window — the LISTEN batch then
executes against a dead session and fails **honestly** (real driver
error — `Error::Db(code 57P01)`, "terminating connection due to
administrator command"; only the timing is fabricated), and the
production fall-through arm runs unmodified
(`forwarder_recovers_from_connection_death_during_the_listen_window`
in `store/open_tests.rs`, run against the harness server).
- **Why this seam shape (not a blind kill-loop)**: the honest
connect→LISTEN window is microseconds, so killing in a loop hoping
to land there is sampling luck, not a test. Replay-proofed both
ways: with the pause window zeroed, the test's key differing pin
becomes non-deterministic and fails; with the 500 ms pause, 6/6
runs green.
- **The fall-through's differentiating pin — the wake count.** The
arm's observables (fall-through → honest transient
`connected=false` state → reconnect → recovery) are pinned by:
a mid-gap `register` failing the transient `Database`, its
registry-entry recovered by the re-issue (its NOTIFY delivers
post-recovery), and — the differentiator — **exactly one**
reconnect-wake on a dedicated pre-kill counter subscription (a
generation whose LISTEN succeeded broadcasts a wake; the
LISTEN-failed one never verifies and never broadcasts; a kill
landing *after* a healthy LISTEN would produce a second). The
counter must be consumed first, then silence pinned for 1 s — the
delivery-loop receiver would otherwise swallow the wake before any
count (the first draft of this pin did exactly that; replay caught
it).
- **Small server-less unit pin** rides alongside:
`generation_listen_pause_consumes_once_per_arm` (the flag's
one-shot mechanics).
### Finding 2(b) + defensive arms — parked with the code-read as pin
Recorded per the review's "either way the disposition should be
recorded" — line numbers current, with the review's pre-diff sites
in parentheses (the diff added ~15 lines to `forwarder.rs`):
- `forwarder.rs:763-771` (was 719–727) — the mid-recv-loop
stale-command abandonment (`command.generation() < generation`
inside the serve loop): **parked — race-shaped.** Deterministic
reach needs a send-racing-the-reconnect seam (a queued command
delivered mid-generational swap); the fix's *other* half is already
unit-pinned (`stale_generation_commands_drop_and_current_generation_
replay` pins the reconcile helper, both drop outcomes, queue-order
preservation, and the dropped-LISTEN's transient ack via
`abandon_stale`), and this arm consumes the same helper's decision
at a second site — the code-read plus the shared-helper pin is the
disposition of record.
- `forwarder.rs:340` (was 330–338) — `run_command`'s
failed-`batch_execute` ack (connection dies mid-LISTEN command):
**parked — defensive.** Needs a command racing session death;
behaviorally the same transient ack `abandon_stale` already pins.
- `forwarder.rs:456,460` (was 444) — `register`'s forwarder-gone
arms: **parked — defensive.** Reachable only with the loop dead
without a shutdown flip; the wave-4 `FanoutRelease` hardening makes
exactly that the guarded anomaly, and `register` itself is
exercised live in the LISTEN-window test's mid-gap arm.
- `forwarder.rs:757,789` (was 744–745/762) — shutdown abort / poll
await arms: **parked — defensive residue.** The shutdown flip's
loop-level exit is pinned end-to-end by the existing shutdown
tests; these are the intra-select abort/await edges of the same
exit.
None is consumer-facing; none guards a wave-4-fix property that
isn't already pinned elsewhere (the findings-class caution the
review applied to Finding 1 was about *untested hardening* — these
are the tested-elsewhere edges, which is why parked-with-code-read
is the honest line here).
### Finding 3 — taken: the rollback-fault seam, both drop arms
- **Taken:** a `cfg(test)` `rollback_fault` module in
`alkstore-sqlite/src/seam.rs` — the commit-fault mechanics
re-exported (`Flag`/`arm`/`take`/`disarmed`/`failure`) with the
`SQLITE_FULL`-shaped substituted error feeding the drop paths.
No PRAGMA probe was run for this one: SQLite has no deterministic
in-process failure shape for `ROLLBACK` itself (its reachable
failure is an environment I/O error), so the same seam conclusion
the commit arm reached after probing holds trivially — recorded
rather than re-probed.
- **Plumbing** mirrors `sqlite-commit-error-arm` exactly
(`#[cfg(test)]` fields on `SqliteStore`/`SqliteTxHandle`/
`ConnLease`, the `arm_rollback_fault` store surface, cfg'd
branches in both `Drop` impls). The one-shot arm is consumed by
whichever drop path fires first — the test arms twice, once per
arm.
- **One test, both arms** (`store/tx_tests.rs::
failed_rollback_on_drop_replenishes_the_writer_slot`): phase 1
arms and drops an open handle (`SqliteTxHandle::drop`'s
failed-`ROLLBACK` path, was tx.rs:439–447); phase 2 arms and
cancels a tx-op **mid-flight** — exactly one poll of the op future
(fires its `spawn_blocking`; a blocking join's first poll is
always Pending, pinned by the test) then `drop` — so the lease
dies holding the connection (`ConnLease::drop`'s, was tx.rs:
166–175). Both phases pin: slot replenished via `reopen` (next
`begin_tx` proceeds under timeout), no ghost rows (the dropped
connection's uncommitted tx rolled back at connection close even
though the `ROLLBACK` statement "failed"), and the store stays
usable (clean commits + auto-commit `notify` afterward). The
error-printed pin is the *executed* production message (verified
live via `--nocapture`: both eprintlns fired with the
`SQLITE_FULL` source) — output capture to assert it in-test is
not a house posture, so the behavioral pins carry the acceptance.
- One first-draft flaw worth noting for future seam authors: dropping
the op future **without polling** never engages the lease (the
`spawn_blocking` only fires on first poll) — the writer slot then
strands because the connection dies inside the never-polled future
with `ConnLease` never constructed. The single-poll shape is what
makes the mid-flight cancellation honest.
## Summary
> What landed, verified how:
- **Finding 2(a) landed** — `alkstore-postgres/src/seam.rs`: the
one-shot `cfg(test)` `listen_pause` module (flag + 500 ms
`WINDOW`); `alkstore-postgres/src/forwarder.rs`: the `listen_pause`
flag on `Forwarder` and `LoopCtx` (cfg'd — dead in production), the
generation-top pause branch, and
`Forwarder::arm_generation_listen_pause`.
`alkstore-postgres/src/store/open_tests.rs`:
`forwarder_recovers_from_connection_death_during_the_listen_window`
(harness server) — the pause-armed kill pair puts the fresh
reconnect generation's LISTEN against a dead session (honest
`57P01` terminating-connection failure); pins the fall-through's
honest transient (mid-gap `register` errs `Database`), full
recovery (reconnect-wake, the pre-death channel's delivery back,
the mid-gap registration recovered by the re-issue, fresh
registrations healthy), and the exactly-one-wake differentiator (a
dedicated pre-kill counter consumed, then 1 s of silence — a second
wake would mean the kill landed after a healthy LISTEN). Also the
server-less unit pin
`generation_listen_pause_consumes_once_per_arm`. Replay-proofed:
window zeroed ⇒ the one-wake pin becomes non-deterministic and
fails (verified live); pause restored ⇒ 6/6 green. The kill rides a
shared `kill_listener` helper (the repeated `pg_terminate_backend`
inline shape of this file's reconnect tests, extracted).
- **Finding 2(b) + the defensive arms parked, disposition recorded
above** — mid-recv-loop stale-command abandonment (race arm),
`run_command`'s failed-batch ack, `register`'s forwarder-gone arms,
shutdown abort/await edges — each with its current line ref, the
reach reason, and the tested-elsewhere coverage it overlaps.
- **Finding 3 landed** — `alkstore-sqlite/src/seam.rs`: the
`rollback_fault` module (commit-fault mechanics pointed at the drop
paths); `alkstore-sqlite/src/tx.rs`: cfg'd fields on
`SqliteTxHandle`/`ConnLease` + cfg'd fault branches in both drop
paths; `alkstore-sqlite/src/store.rs`: `arm_rollback_fault` + the
`begin` plumbing.
`alkstore-sqlite/src/store/tx_tests.rs`:
`failed_rollback_on_drop_replenishes_the_writer_slot` — both drop
arms in one test (open-handle drop; mid-op cancellation via the
exactly-one-poll-then-drop shape), pinning slot replenish via
`reopen`, no ghosts, store usable. The fault arms' production
eprintlns verified firing live (`--nocapture`).
- **Gates**: `cargo build`; workspace `cargo test` server-less green
(no failures: core 25 + harness 3, postgres 127 lib + 25 suite + 9
schema server-less, sqlite 193 lib + 25 suite); the pg column live
against the harness server green (127 lib + 25 suite + 9 schema);
pg server-less green (the documented SKIP posture); `cargo clippy
--all-targets -- -D warnings`; `cargo fmt --check`; the fuzz corpus
gate (`cargo test --manifest-path fuzz/shared/Cargo.toml`) — all
green.