alkstore/tasks/sqlite-commit-error-arm.md

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

id	name	status	depends_on	scope	risk	impact	level	tags
sqlite-commit-error-arm	SQLite engine — commit-error-arm coverage (wave-3 review's deferred test)	completed		narrow	medium	isolated	implementation	wave-5 sqlite-engine tests

Description

Land the wave-3 review gate's deferred coverage item (tasks/review-wave-3.md Notes: "Coverage of the commit-error arm — no test induces a failing COMMIT ... natural home is wave 5's suite hardening"): a test that drives a failing COMMIT through the SQLite engine's commit path and pins the fix the review verified by code-read — the writer slot is replenished via the reopen closure on the failed COMMIT (no stranding), the error surfaces as the opaque Database, and the store remains fully usable afterward (subsequent begin_tx/auto-commit ops work).

The injection mechanism is this task's to find — candidates: a PRAGMA max_page_count squeeze forcing SQLITE_FULL on a space-consuming commit, or a cfg(test) fault seam in the seam layer if the PRAGMA route cannot hit the COMMIT arm deterministically. The wave-4 fix batch's cfg(test) config-seam precedent (pg-fix-forwarder-reconnect) is the house pattern if a seam is needed. Prefer the least-invasive mechanism that deterministically reaches the arm; document the choice.

If the same session has cheap room for it, the waves-1–2 review's optional add — the watcher reconnect-success path test (upstream's W-1 test drives only the failure path; "a reconnect success test would require a file appearing mid-run") — rides here only if the engine kept the current watcher shape and the test is genuinely cheap; otherwise leave it recorded as not-taken in Notes (it was a suggestion, not an order).

Acceptance Criteria

• A test induces a failing COMMIT on the SQLite engine and pins: error surfaces (Database), writer slot replenished (a subsequent begin_tx succeeds), no partial-commit residue
• The injection mechanism documented in Notes (PRAGMA vs seam, and why)
• The watcher reconnect-success test either landed or explicitly recorded as not-taken with the reason
• cargo test -p alkstore-sqlite green server-less; clippy -D warnings; fmt clean

References

• tasks/review-wave-3.md (Notes: the deferred commit-error-arm coverage)
• tasks/sqlite-engine-seam-tx.md (the commit replenish fix the test pins)
• tasks/pg-fix-forwarder-reconnect.md (the cfg(test) seam precedent)
• docs/reviews/001-waves-1-2-general-review.md §2 (the watcher reconnect-success suggestion)

Notes

Decisions of record the implementation made that the description didn't pin:

• Injection mechanism: the cfg(test) commit-fault seam, not the PRAGMA route — decided by live probe. Both PRAGMA candidates were tested against real SQLite before choosing: in both WAL and DELETE journal modes, PRAGMA max_page_count squeezed to the current page count fails the first growth statement (SQLITE_FULL / DiskFull at page-allocation time), leaving "no transaction is active" for the subsequent COMMIT — the commit arm is unreachable through PRAGMA-space. (Also probed: max_page_count is per-connection, so arming on the writer conn pre-begin_tx would have ridden into the tx connection — the route failed on placement of the error, not on delivery.) The seam substitutes a SQLITE_FULL-shaped rusqlite::Error for one commit and feeds it into the production commit error arm — the test pins the real drop + reopen replenish code, not a replica.
• Scoping design (the seam's cross-test-safety shape): the fault flag is a per-SqliteStore Arc<AtomicBool> (born disarmed, armed via the #[cfg(test)] arm_commit_fault, disarmed on first consumption — exactly one commit faults and every later commit is real, which the test also pins). Parallel test stores can never interfere. Plumbing follows the pg-fix-forwarder-reconnect precedent exactly: #[cfg(test)] field on SqliteStore and on SqliteTxHandle, #[cfg(test)] param on SqliteTxHandle::begin, cfg'd statement branches in store.rs::begin_tx and src/tx.rs::commit — production builds compile the plain path.
• Replay-proofed live: with the commit error arm's writer_reopen replenish temporarily removed, the new test fails (the next begin_tx parks past its 5 s timeout — the stranding the review identified by code-read); reverted, it passes.
• The watcher reconnect-success add rides here — taken. The vanished-file route the waves-1–2 review sketched could not reach the success body (on file reappearance the identity check reads a fresh (dev, ino) and the watcher dies through the dead-man's switch before the reconnect would matter). Instead the test drives run_poll_loop directly through the open_conn_fn seam the loop already takes — the same instrument as the W-1 failure test — with the db file present throughout (identity stable): initial open and first two reconnects injected as failures, third reconnect succeeds, and a commit from a writer connection then wakes on_change (proving the success arm re-baselines data_version and restores delivery). Watcher shape untouched.
• cargo build; cargo test -p alkstore-sqlite green server-less (191 lib + 25 suite, both new tests included); workspace cargo test 399 passed / 0 failed (core 25 + harness 3; postgres 121 + 25 suite + 9 schema; sqlite 191 + 25 suite); cargo clippy --all-targets -- -D warnings; cargo fmt --check — all green.

Summary

What landed, verified how:

• alkstore-sqlite/src/seam.rs: the #[cfg(test)] commit_fault module — per-store Flag (Arc<AtomicBool>), disarmed/arm/take (disarm-on-consume), and the SQLITE_FULL-shaped fabricated rusqlite::Error — with the mechanism-choice rationale (the PRAGMA probes) in its doc comment.
• alkstore-sqlite/src/tx.rs: #[cfg(test)] fault field on SqliteTxHandle + begin param; commit's blocking body gains a cfg'd branch that feeds the fault into the existing production match — the error arm (drop + reopen replenish, sqlite_error mapping) runs unmodified.
• alkstore-sqlite/src/store.rs: #[cfg(test)] commit_fault field on SqliteStore, the arm_commit_fault surface, and the cfg-branched begin_tx call.
• alkstore-sqlite/src/store/tx_tests.rs: failed_commit_replenishes_the_writer_slot — failed COMMIT errs opaque Database with the SQLITE_FULL source chain; the next begin_tx proceeds (slot replenished, no stranding); no partial-commit residue; the post-failure commit is real and clean (fault disarmed); auto-commit notify works afterward.
• alkstore-sqlite/src/substrate/watcher.rs: reconnect_success_resumes_wake_delivery — the waves-1–2 review's optional reconnect-success add, driven through the loop's open_conn_fn seam (injected failed opens, then a successful reconnect; wake delivery resumes and is pinned). Recorded as taken per the task's optional clause.
• Gates: cargo build, cargo test (server-less: core 25 + core suite 25 + sqlite 191 + postgres 121 + 9 schema — 399 passed / 0 failed, postgres harness skipped server-less), cargo clippy --all-targets -- -D warnings, cargo fmt --check — all green.