alkstore/tasks/review-wave-6.md

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

id	name	status	depends_on	scope	risk	impact	level	tags
review-wave-6	Review gate — wave 6 (mem engine)	completed	mem-engine-integration pg-fix-scheduler-tick-seam	moderate	low	phase	review	wave-6 review

Description

The wave-6 review gate (the plan's §Review gates, wave 6; the born-pinned gate). Code-read the alkstore-mem engine against the contract and the pinned ADR-024 text, and audit:

1. Conformance read: engine-vs-contract line by line, mirroring the wave-3/4 gate pattern — every trait surface against core-contract.md and the pinning ADRs (007/008/009/010/015/019/ 020/021/023), the tx overlay against ADR-024 + engine-mem.md §The tx overlay (commit-merge atomicity, wake-at-commit, drop = discard, panic-through-unwind).
2. The third column as the compatibility instrument born pinned: the mem column's wiring read row-by-row against the discharge map (which rows carry, which engine-shaped rows omit and why — the asymmetry classification, ADR-024 §3); PayloadTooLarge non-occurrence arm present; three-way arithmetic equivalence actually asserted green (backoff curve, opts resolution — the ADR-012 §2 one-owner rule's third owner), plus the derived-queue naming row composing identically.
3. wasm32 acceptance audit (ADR-024 §5): the compile gate clean, tokio feature subset verified on the manifest, no blocking calls in mem internals, the current-thread flavor actually enforced by the column's tests.
4. engine-mem.md read against the implemented engine; the deployment matrix's mem rows (deployment.md, landed with ADR-024) still true as implemented (honesty carriers: identity + docs + matrix — nothing runtime invented). Gate may flip engine-mem.md to stable if the read is clean.
5. The rider closed: pg-fix-scheduler-tick-seam's arms verified (review 003 Finding 1 discharged into this window; Findings 2–3 confirmed still parked for wave 7).
6. Contract-window discipline: the class-1 amend-in-place window is still open — any wobble the third implementation surfaced must appear as resolved ADR text (or be recorded as a finding with its disposition), not as a silent divergence between suite row and engine behavior.

Acceptance Criteria

• Full conformance code-read recorded (findings + dispositions inline in this task's Notes, mirroring review-wave-3/4 style)
• Suite mem column green on host, 25-row set wired per the classification; three-way equivalence rows green
• wasm32 compile gate + feature-subset + current-thread flavor verified, recorded
• The rider's arms verified; Finding 1 disposition recorded
• Workspace gates re-run green this session (cargo build, cargo test, clippy -D warnings, fmt)
• engine-mem.md disposition decided (stable flip or follow-up) and implementation.md's wave-6 row updated

References

• docs/plans/implementation.md §Wave 6, §Review gates (wave 6 entry)
• docs/architecture/decisions/024-mem-engine.md (the charter this gate audits code against)
• docs/architecture/engine-mem.md
• tasks/review-wave-3.md / tasks/review-wave-4.md (the gate pattern)
• docs/reviews/003-wave-5-general-review.md (Finding 1's disposition trail; Findings 2–3's wave-7 park)

Notes

Gate verdict: passed. No blocker, no major live defect; three edge-domain fixes landed inline (below); the remaining observations are recorded with dispositions, none mem-divergent from the engines.

Fixed inline at the gate (this session, verified clippy/fmt/test green after each)

1. Stream cursor read overflow (stream.rs read_events): the cursor advanced by plain from + 1, panicking on debug build for a caller-supplied offset == i64::MAX (direct read_since, or indirect via save_offset(i64::MAX) + read_from_consumer) and wrapping to a from-the-beginning read in release. Fixed with Bound::Excluded(from) — boundary-total at every i64, the ADR-023 §2 read-cursor classification this fix encodes.
2. trim_to horizon endpoint: horizon.saturating_add(1) clipped i64::MAX and left a row at offset i64::MAX undeleted — the exact-<= boundary broke at the one unreachable endpoint anyway. Fixed with Bound::Included(horizon); unreachable in practice (2⁶³ publishes), fixed because the template cost nothing.
3. Scheduler register overflow alignment (conformance divergence found by the read): mem registered a parseable interval whose first boundary overflows (@every 9223372036854775807s) with a saturating next_fire_at — accepted-never-fires — where the SQLite register path rejects the same spec with InvalidSpec. Class-1-window divergence between engines on a register-time semantics; aligned mem to reject (checked_add → InvalidSpec, no row stored) with a new pin test distinguishing it from the unparsable-too-large spec the parser already refuses. The tick's i64::MAX overflow sentinel stays identical in both engines.
4. Scaffold-residue cleanup: clock.rs's stale #![allow(dead_code)] on the now-wired accessor + scaffolding comment removed; tx.rs's commit-wake comment corrected to what the code does (one wake per staged stream — receivers drain by cursor, the feed is plumbing); wakes.rs module doc's "mechanisms arrive with their tasks" updated to the wired mechanisms; stream.rs module doc no longer claims "a fresh stream's first event does not carry offset 1" for mem (mem's fresh count is 1; the ADR-015 §4 note is engine-internal and never compared cross-engine — the doc now says exactly that).

Recorded findings, no code change (dispositions)

• Multi-thread publish→wake ordering tolerance (streams): the publish path allocates the offset, then inserts under the guard, then fires the wake after the guard drops — on a multi-thread runtime two concurrent publishes can complete insert+wake out of offset-allocation order, and a subscriber's cursor can skip an event whose wake fires later (the event is never lost from the log, but a pre-insert drain's checkpoint passes it). The SQL engines share the shape in principle (offset assigned pre-commit, wake post-commit — the "pg-shaped story" the mem docs cite), and nothing contract-side pins wake-fire order vs offset order. Disposition: this is a contract-level tolerance nobody pinned, not a mem-local divergence — recorded for the wave-7 pre-release pass to either document as tolerated or amend as ADR text (wake order per stream = offset order). Mem's declared posture is current-thread-clean (engine-mem.md) and the suite's mem column proves it under the current-thread flavor, which is why nothing here fails today.
• Mem arithmetic is a transplant, not an independent derivation (one-owner-rule note): resolution.rs's backoff curve and opts resolution, scheduler.rs's @every parser and skip_forward, are byte-level copies of the SQLite-side implementations (semantics re-verified against the ADR text during this read). ADR-012 §2's letter — each engine owns its implementation, equivalence suite-pinned — is satisfied (mem owns its modules; the backoff_curve_equivalence and enqueue_opts_resolution rows run green on all three columns). Recorded so the gate's record doesn't overstate the independent-signal value of the third copy; the suite is the honesty instrument.
• heartbeat(extend) unclassified (ADR-023 §2): the domain table classifies try_lock/renew durations but not the queue heartbeat's seconds-shaped extend argument. Both engines treat it boundary-total (saturating_add/SQL integer add — a negative shortens/voids the claim). Recorded as a class-1-window doc annotation candidate for wave 7's pre-release pass; no engine diverges.
• save_offset's argument unclassified: negatives accepted (stored, reads resume forward), monotone no-op below the checkpoint — both engines, contract pins monotonicity only (ADR-019 §6). Same annotation candidate.
• EventReceiver::try_recv's closed shape unpinned: Err(Closed) (mirrors WakeReceiver::try_recv, ADR-008 §3); taxonomy-consistent, mem-test-pinned, but ADR-021 §5 pins only recv()'s arms. Same annotation candidate.
• get_job is id-global, not queue-filtered — uniform on all three engines (the handle carries the queue, the lookup the id; get_job_tx is queue-filtered on both engines with tx). Recorded so the record doesn't misread core-contract's "queue-scoped" handle phrasing as a filter; the ambiguity is contract-prose, not code.
• Mem outbox run_once ignores a trailing ack/retry refusal (deadline lapsed mid-delivery): identical to SQLite; the uniform validity predicate makes the refusal the at-least-once honesty — conformant, noted for the boolean's silent discard.
• Wake-bus attach-vs-close race (defense-in-depth note): WakeBus::attach/fire_commit_wakes check closed before taking the registry guard, so a listen racing the engine's drop could in principle mint a feed whose sender post-dates the registry clear. Unreachable through safe APIs (a live &MemStore borrow pins the last reference — borrowck; the bus is reachable only through the store), so no change; recorded as a hardening candidate if the bus ever gains a second entry path.

Audit record (verified items)

• Conformance read: queue state machine (claim predicate/ordering, visibility, reclaim-eats-attempt, uniform validity predicate, heartbeat, retry/fail/cancel/ack_batch, dead-letter + no-stranded sweep, queue-scoped get_job incl. dead rows); scheduler (grammar parser, register-time stamp resolution, one-guard fire-and-advance tick in the schedules→queues order, 64-cap catch-up + grid-aligned skip-forward, leadership acquire/renew/loss/clean-stop on the reserved lock with runner-unique owner); outbox (derived reserved backing queue unreachable by every consumer entry point, 60/5/5 stamps, run_once ack/retry curve, tx half stamping identically); streams (offset-ASC reads, absent-consumer 0 head read, monotone saves, trim exact-<= never-renumbering and waking nothing, attach-before-read subscribe, engine-drop close arms); locks (contention loser Ok(None), ttl <= 0 refused on try + renew, silent lapse, non-owner release = false, renew-past-expiry refused, no lock_tx); clock (one accessor, all stamps read it, freeze seam cfg(test) only); validation (every entry routed through core's validate_shared_name/validate_local_name, no engine-local rules); tx overlay (eleven *_tx staged with entry validation before effect, one clock read, ADR-020 §3 stamps incl. per-job override, merged reads queue-scoped + dead-visible + offset-ASC + extent-clamped, one-op commit merge + wake-at-commit, drop = discard through unwind, stage-time id/offset allocation with legal gaps); wake substrate (channel-scoped, per-notify never coalesced, attach-after-commit no-replay, close disconnects all, dropped-feed detach, commit-ordered FIFO). Guard discipline audited: canonical order (schedules → queues → streams) holds at every two-guard site; no guard across an .await or user code; zero panics/unwrap/ expect in library code.
• Third column as compatibility instrument: the mem target wires exactly the 24 engine-shared rows + mem's payload_too_large_never_produced_on_mem (stamped ADR-016 §5 + ADR-024 §3) + the factory-shape test; the reconnection-shaped rows stay per-engine wired (absent here, per ADR-024 §3's classification); the three-way equivalence (backoff_curve_equivalence, enqueue_opts_resolution) green on all three columns; 25/25 green on the host.
• wasm32 acceptance: cargo check --target wasm32-unknown-unknown -p alkstore-mem clean; manifest tokio subset sync+time (dev-deps rt/macros only); no spawn_blocking/blocking-call in mem source; current-thread flavor enforced on every mem-column test (a blocking call would hang, not pass); on-target suite execution stays scoped out behind the named adapter-crate collapse condition (ADR-024 §5 — recorded, not an OQ).
• Spec + matrix: engine-mem.md read against the implemented engine (tx overlay, asymmetry classification, isolation, wasm identity — all true as landed; the stable flip records the gate in its frontmatter); deployment.md's mem rows read true as implemented.
• Rider closed: pg-fix-scheduler-tick-seam's two arms (one_faulted_tick_is_retried_and_the_job_fires, tick_retry_exhaustion_exits_typed_and_leaves_the_lock_to_lapse) verified present against the real retry loop; review 003 Finding 1 discharged; Findings 2–3 confirmed still parked for wave 7 (implementation.md §Wave 7); the fire-wake gap (pg-fix-scheduler-fire-wake) closed pre-wave and its retired gap records verified; the wave-5 flake ledger's §6 update recorded.
• Workspace gates re-run green this session: cargo build; cargo test --workspace green (core 25, suite harness 3, mem 101 lib + 25 suite, SQLite 191 + 25, pg 124 + 25 + 9, pg column server-less per the SKIP posture — the pg-side verification was the rider tasks' own live runs); clippy --all-targets -D warnings clean; cargo fmt --check clean; taskgraph validate green (61 tasks).

Summary

The wave-6 review gate passed and flipped engine-mem.md to stable. The conformance read was clean in mechanism substance; the findings class was edge-domain and hygiene: three inline fixes (the stream cursor's i64::MAX overflow, trim_to's horizon endpoint, and the scheduler register-overflow alignment with SQLite's InvalidSpec — the one true engine-vs-engine divergence the read found, resolved into the class-1 window) and the expected residue cleanups. The suite's third column is born pinned as ADR-024 required — 25 green under the current-thread flavor, the three-way arithmetic equivalence actually asserted — and the wasm32 acceptance items verified. The unpinned contract-adjacent observations (the multi-thread wake-ordering tolerance, three ADR-023 §2 table classifications, the receiver try_recv closed shape) are recorded above with their dispositions for the wave-7 pre-release pass; none blocks. The rider (pg-fix-scheduler-tick-seam) closed Finding 1. Task files audited: all eleven dependency tasks completed with Notes/Summary filled.