alkstore/tasks/release-findings-2-3-disposition.md

id, name, status, depends_on, scope, risk, impact, level, tags

id	name	status	depends_on	scope	risk	impact	level	tags
release-findings-2-3-disposition	Review 003 Findings 2–3 — the parked pre-release dispositions	completed		narrow	low	component	implementation	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

• 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
• Finding 3's rollback-fault test landed covering both drop arms (or its not-taken disposition recorded)
• The disposition record written so a future general review reads the inventory as closed, not re-derivable
• 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.