alkblobs/tasks/review-wave-4.md

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

id	name	status	depends_on	scope	risk	impact	level	tags
review-wave-4	Wave 4 review gate — the postgres engine vs the contract	completed	suite-column-postgres	narrow	low	phase	review	wave-4 review

Description

The wave-4 review gate — the engine-wave review shape (docs/plans/implementation.md §Review gates: "engine-vs-contract conformance"), before wave 5 decomposes. The gate reads the implementation against the pinned ADR text line by line (the wave-1/2/3 gates' method).

Gate checks:

• Engine-vs-contract conformance: the suite column green against dockerized postgres (server-less runs skip cleanly — a skip never substitutes for acceptance); the engine-state clause families contract-tested per the applicability row (ADR-012 §5's postgres row — every family yes); virgin-store semantics; the size metadata-only discipline (the size-column probe never loads the value blob).
• The postgres posture vs ADR-007: the B5 deltas carried as requirements, not hedges — the synchronous_commit parity knob (configurable, per-connection wiring, default on); pooled connections with per-connection prepared-statement discipline (statements die with sessions); autovacuum + fillfactor tuning stated in the module doc; the ~40× small-tier storage overhead accepted and documented; the B5 tuning numbers recorded as POC-derived posture defaults (ADR-007 §6's deferred-cost pattern).
• The sweep-safety proof vs ADR-007 §5: list() complete and well-formed as a stable MVCC-snapshot cursor over a vacuuming table; the batch-delete helper (key = ANY($1)) contract-tested; the exact-count sweep-outcome rows green against the server.
• The seam families vs ADR-010 §2 / ADR-012 §4/§5: the full clause families on postgres (the fleet host) — ensure_gc_schema (both tables, the embedder's liveness table never created), window_stage/window_take (the sqlite semantics' dialect twin), the joint tx ownership rule honored in the JointTx shape (the store holds; the engine contributes), pin_commit's atomicity (publish + protection in one step, zero residue on failure), pin_renew's never-resurrect semantics, pin_check's classification-only discipline, arbitrate's finger-lock fold, the advisory lock's ADR-013 §2 lease semantics (atomic test-and-set, session-scoped holdership, same-connection release). Fleet activation absent by construction — no fleet: parameter, no store-core fleet wiring (wave 6); the clause bodies exist and are contract-tested per applicability.
• The non-fleet posture vs ADR-012 §3: the constructors are implicitly fleet: false — the in-process protocol verbatim; the recorded posture names wave 6's slotting-in as a non-event.
• Async/blocking discipline: tokio-postgres natively async — no spawn_blocking seam (the sqlite discipline does not carry); no blocking I/O on the async path; the pool is the concurrency posture; no unwrap/expect outside tests.
• Named design points audited as resolved (each recorded in its task's Notes): the schema name + quoted-identifier boundary; the pool default size; the prepared-statement discipline; the DSN + opts constructor shape; the virgin-store tolerance split; the list-cursor fetch shape; the timestamp encoding; the JointTx concrete type; the pin_renew successor-time source; the lock key + scope; the lease's connection holding.
• Family conventions: no comments in code (doc comments on public API fine), thiserror, tokio-only async, no unwrap/expect outside tests, naming, one-module-per-file re-export shape, the #![allow(dead_code)] scope notes matching their remaining scope (the batch-delete helper's wave-6 note intact).
• Gates green: cargo test (default), cargo test --features mem, cargo test --features postgres (server-less: skipping), cargo test --all-features, cargo check --no-default-features --features postgres (the pg-pure build), cargo clippy --all-targets -- -D warnings (default + --features postgres + --all-features), cargo fmt --check.

Findings are fixed inline where cheap, or recorded as wave-5 amendments where they shape the next decomposition. The gate's completion is what unblocks wave 5's decomposition.

Acceptance Criteria

• Every gate check above read and verified against the cited ADR text (not from memory of it)
• Named design points each decided, recorded, and consistent with the specs
• Findings fixed inline or recorded as wave-5 amendments with owners
• All gates re-run green after any fixes
• Wave 5 decomposition unblocked (recorded in this task's Summary)

References

• docs/plans/implementation.md (§Wave 4, §Review gates — engine wave reviews)
• docs/architecture/decisions/007-two-kv-engines-sqlite-and-postgres.md
• docs/architecture/decisions/010-backend-io-seams-and-fleet-state-home.md §2
• docs/architecture/decisions/012-pre-decomposition-consistency-rulings.md §3/§4/§5/§6.6/§6.7
• docs/architecture/decisions/013-family-composition-reactive-boundary-and-feature-graph.md §2/§4
• docs/architecture/store-api.md
• tasks/review-wave-3.md (the gate method; the structural twin's gate)

Notes

Gate completed 2026-10-12, the wave-1/2/3 method: every gate check read against the pinned text (ADR-007 §4/§5/§6, ADR-010 §2, ADR-012 §3/§4/§5, ADR-013 §2/§4, store-api.md, the seam trait in src/gc.rs), then verified against the landed diff (1efc46f…0fb05fc) and re-run gates. Findings fixed inline:

• PG-1 — in-instance sweep-lease reaping after session loss (fixed, ADR-013 §2). sweep_lock_acquire answered Ok(false) from the mutex even when the held lease's session had died — blocking re-acquisition on the same backend until an explicit release, against the holdership rule ("holdership ends at … engine-session loss"). Fixed: the acquisition answers against a live session only (dead checkouts are reaped in-instance); new test sweep_lock_in_instance_session_loss_reaps_the_lease (terminates the lease's own held backend via pg_terminate_backend and re-acquires without a release). The foreign-session reaping test (the wave-4 test) is unchanged.
• PG-2 — the PgJointTx mid-flight-drop hazard (doc fix + amendment A1 below). deadpool's Fast recycle never resets transaction state, so a PgJointTx dropped between begin_joint_tx and pin_commit returns a pooled dirty session to the pool. The clause bodies' own failure paths all ROLLBACK before surfacing (wave-4 contract holds — the zero- residue atomicity test proves it); the hazard is the store core's future use, where Rust ? propagation between the two calls is the natural error shape. Not fixable inline without async-drop or a seam reshape — recorded on [PgJointTx]'s doc and as amendment A1 for wave 6.
• PG-3 — engine-level pg tests leaked per-test schemas on the shared harness db (fixed). 380 abt_* schemas had accumulated (the suite column's alkblobs_suite_* teardown was verifiably clean — 0 residue — the leak was src/postgres.rs's + src/store.rs construction tests' per-test schemas). Added crate-internal test_support::SchemaJanitor (a drop-guard running DROP SCHEMA IF EXISTS … CASCADE synchronously over a std thread + a one-shot current-thread runtime — no async-drop exists), the Harness deref-guard wrapper for the engine tests', and fresh_opts carrying a janitor in the construction tests. A one-time manual cleanup dropped the legacy residue (the 16 hostile-name stragglers dropped via quote_ident — incidentally exercising the boundary). Post-fix: 0 schemas residue after full --features postgres and --all-features runs.

Amendments for the next waves (owners recorded here, no spec text changed this gate):

• A1 (wave 6, owner: the fleet-GC mechanism task) — the joint-tx drop discipline: the store core's joint-tx driver owes pin_commit on every path from begin_joint_tx onward (PG-2). Design it catch-propagate-terminate, or grow an explicit abort verb via a new ADR (the seam shape is pinned; pin_commit's terminal-consume reading stays).
• A2 (wave 5, owner: the pg-lo engine task) — pg-lo's engine tests share the pglo-poc cluster: adopt SchemaJanitor (or the column's own teardown pattern) from the start — the wave-4 lesson is not to re-learn it.

Minor observations, no action: sweep_lock_acquire's mutex check is not atomic with its checkout (two racing acquires both check out; the advisory lock itself arbitrates and exactly one stores its checkout) — sound by server arbitration. list rides the collected query rather than query_raw — the recorded design point; the bounded-memory claim holds for the keys-only materialization.

Verification (post-fixes, all live unless noted): cargo test 69/19/3 ok; cargo test --features mem 124/54/4 ok; cargo test --features postgres 103/36/3 ok against the pglo-poc server, 51 clean skip notes server-less; cargo test --all-features 158/71/4 ok against the server; cargo test --no-default-features --features postgres (pg-pure) 76/19/3 ok; clippy -D warnings green (default / --features postgres / --all-features / pg-pure); cargo fmt --check green; cargo doc --no-deps clean; shared-db schema residue 0.

Summary

Completed 2026-10-12 (wave 4). The engine-wave gate read the landed postgres engine against the contract line by line and found it conformant: the suite column green against the harness server (36 conformance rows live; server-less runs skip cleanly — a skip never substituted), every clause family of ADR-012 §5's postgres row contract-tested (both-tables schema + never-the-embedders, stage-idempotence/take-atomicity/the <= boundary, joint-tx atomicity with zero residue, pin_renew never-resurrect, two-variant pin_check, the folded FOR UPDATE arbitrate, the advisory lease's ADR-013 §2 semantics), the B5 deltas carried as requirements (the per-connection synchronous_commit knob tested, per-connection prepare_cached, fillfactor 90 stamped and reloption-asserted, autovacuum/40× overhead stated in the module doc), the list MVCC-snapshot cursor + the batch-delete helper exact-count-gated vs server, the schema-less virgin-store split, the size metadata-only EXPLAIN gate, and the non-fleet/posture recordings verbatim where wave 6 needs them. All 11 named design points audited as decided and recorded in the wave-4 task Notes. Three findings fixed inline (in-instance lease reaping — new test; the PgJointTx mid-flight-drop hazard — doc + amendment A1; the engine-test schema janitor — 380 leaked schemas → 0), two amendments recorded for waves 5/6 with owners (A1 the joint-tx drop discipline; A2 pg-lo adopts the janitor). All gates re-run green after the fixes (the verification block is in Notes). Wave 5 (pg-lo large engine) decomposition is unblocked; its review gate is the engine-wave shape again.