diff --git a/docs/plans/implementation.md b/docs/plans/implementation.md index 348567c..127c2bb 100644 --- a/docs/plans/implementation.md +++ b/docs/plans/implementation.md @@ -821,5 +821,33 @@ decomposes. Specific gates: forwarder TOCTOU/hang/diagnosability arms, the pg BIGINT server-side stamp overflow, the mem growth/outbox notes, the pg identifier-truncation doc note, and smaller records. All gates - re-run green after the fixes (build, workspace tests, live pg - column, clippy, fmt, fuzz replay 6/6, wasm32, doc gate). + re-run green after the fixes (build, workspace tests, live pg + column, clippy, fmt, fuzz replay 6/6, wasm32, doc gate). +- **Review-004 fix batch** (2026-10-11) — the sixteen recorded findings + decomposed into sixteen tasks (items 2+3 and 4+6 bundled; item 5 + split into the decision + its implementation; the pg + scheduler-classification twin added mid-batch at the sqlite task's + implementation): the sqlite auto-commit seams' panic-safety armor + (RAII leases, drop-not-release, replenish/abandon dispositions); + the dead-watcher subscribe fail-closed + the identity-unknown + baseline; the listen bridge aligned to the stream bridge's + std-thread shape; the run_schedules loss-vs-failure classification + landed on both engines (dated ADR-009 §1 annotation, engine-neutral) + with the `LeadershipLost`-at-a-dead-store misdirection gone; the + substrate's ten UDF `// SAFETY` comments riding delta D-40; the pg + forwarder TOCTOU closed (decide-and-queue under the registry guard) + and both hang arms bounded (`bounded_wait`, the black-hole proxy + harness test); the pg BIGINT stamp sums saturated server-side (the + signed decision, `saturating_stamp_expr` at six sites, ADR-023 §2 + annotation); `tracing` adopted engine-only (ADR-025, user sign-off; + all 19 `eprintln!` sites migrated, D-41) with both forwarder + diagnosability arms surfaced; core's pre-yield `offset()` verified + equal across engines and pinned (contract text + trait doc + suite + row); `Display` made control-char-safe (incl. `Error::Codec`); the + mem growth/outbox and pg identifier-truncation doc pins; the tokio + dev-dep features normalized. The batch's review gate (`review-004-fixes`) + verified every disposition by code-read against the review's failure + mechanism, re-spot-checked the inline fixes' pinned postures, and + annotated review 004's recorded-findings items with their + dispositions; residual task-file hygiene cleaned (stale placeholder + lines, AC checkboxes). diff --git a/docs/reviews/004-pre-release-general-review.md b/docs/reviews/004-pre-release-general-review.md index a3058e9..8a8e3db 100644 --- a/docs/reviews/004-pre-release-general-review.md +++ b/docs/reviews/004-pre-release-general-review.md @@ -52,6 +52,11 @@ or distributed machinery whose repair exceeds a review fix, or are conscious postures to record. None blocks the release; each names its shape so a future session can decompose without re-deriving. +*(Dispositions, 2026-10-11: all sixteen items decomposed into the +`review-004-fixes` fix batch — the sixteen `tasks/*` files below each +carry the per-item decisions of record and verification — and the +batch's gate audited them all landed. The record reads closed.)* + 1. **sqlite seam panic-safety (defense-in-depth)** — `with_writer`/`with_reader`/`blocking_read` (`alkstore-sqlite/src/seam.rs`, `substrate/schema.rs`) release their slot/pool connection after the @@ -66,6 +71,9 @@ shape so a future session can decompose without re-deriving. needs abandonment plumbing on both `Writer` (drop + path-based replenish) and `Readers` (an outstanding decrement), which is a deliberate small design task, not an inline patch. + *Disposition: landed via `sqlite-fix-seam-panic-safety` (RAII + `WriterLease`/`ReaderLease` guards, drop-not-release, replenish/ + abandon dispositions; injected-panic tests in `store/seam_tests.rs`).* 2. **sqlite post-watcher-death subscribe hang** — after the watcher thread exits (db-file replacement), `subscribe()` still enrolls new senders with no poller alive (`substrate/watcher.rs:391-396`): a @@ -74,6 +82,9 @@ shape so a future session can decompose without re-deriving. closes" posture (enforced only for pre-death subscribers). Needs a dead-store/dead-watcher sign that new subscription paths consult — a state-machine change across `store.rs`/`notify.rs`/`stream.rs`. + *Disposition: landed via `sqlite-fix-watcher-death-subscribe` arm + (a) — the dead sign set before the fatal exit, fallible `subscribe` + failing closed under the senders lock.* 3. **sqlite watcher initial-stat failure self-kill** — an identity-`stat` failure at watcher start seeds `(0, 0)` (`watcher.rs:164-170`); the first *successful* later stat mismatches @@ -81,6 +92,9 @@ shape so a future session can decompose without re-deriving. on recovery from a transient stat hiccup. Rare, but the fix (an identity-unknown state instead of a false identity) is another watcher-state change — bundle with item 2 if either is taken up. + *Disposition: bundled with item 2 in `sqlite-fix-watcher-death- + subscribe` arm (b) — the `Option<(u64, u64)>` identity-unknown + baseline; the first successful stat captures without driving death.* 4. **pg forwarder register/unregister TOCTOU** — the registry refcount and the LISTEN/UNLISTEN command emission are not one critical section (`forwarder.rs:131-176` vs `438-482`): interleaved @@ -90,6 +104,10 @@ shape so a future session can decompose without re-deriving. re-issue. The fix is a decide-and-queue restructure inside the forwarder — forwarder surgery that should ride its own session with the existing test-seam vocabulary. + *Disposition: landed via `pg-fix-forwarder-hardening` (a) — + `ChannelSet::enqueue_register`/`enqueue_unregister` make the + refcount decision and the command enqueue one critical section + under the registry guard; replay-proven both ways.* 5. **pg server-side BIGINT overflow on consumer stamps** — the claim, heartbeat, retry-reschedule, and retention-sweep statements compute server-side sums over consumer-supplied i64 values @@ -102,6 +120,10 @@ shape so a future session can decompose without re-deriving. guard the server expressions) — but the *which side* question is contract-adjacent (ADR-023 §2's totality wording) and deserves its own decision, not an inline patch. + *Disposition: `pg-decide-stamp-arithmetic-side` decided the server + expression owns the guard (result-domain saturation, no input + clamp); `pg-fix-bigint-stamp-overflow` landed it as + `saturating_stamp_expr` at the six sites, pinned live.* 6. **pg forwarder hang-shaped arms** — two narrow ones: a failed generation-verify/LISTEN batch is treated as connection death and parks on `poll_task.await` forever even though the session may be @@ -110,8 +132,13 @@ shape so a future session can decompose without re-deriving. awaiting a command on a black-holed connection (the `run_command` await sits outside the shutdown `select`, `forwarder.rs:781`, no timeout on this connection). Both want the same treatment family — - timeouts/bounded waits on the listener path — bundle into the - forwarder-hardening session (item 4). + timeouts/bounded waits on the listener path — bundle into the + forwarder-hardening session (item 4). + *Disposition: both bundled into `pg-fix-forwarder-hardening` (b/c) + — the `bounded_wait` primitive bounds every listener-path await + (`LISTENER_QUERY_TIMEOUT` 5 s / `LISTEN_SETTLE_TIMEOUT` 2 s, + shutdown-selected), the failed-verify settle aborts at the bound; + the black-hole proxy harness test pins the close-time reclaim.* 7. **pg forwarder diagnosability** — the reconnect arm's connect failures are swallowed with `.ok()` and the loop's exit discards the poll error (`forwarder.rs:825/855-869`): an operator sees nothing @@ -119,22 +146,36 @@ shape so a future session can decompose without re-deriving. log facade and 17 library-path `eprintln!` sites carry its diagnostics (verified secret-free — no DSN/password material ever printed). A `log`/`tracing` adoption decision is a family-level - call; recorded rather than made here. + call; recorded rather than made here. + *Disposition: `decide-log-facade-adoption` adopted `tracing` + (ADR-025, engine crates only, user sign-off 2026-10-11; all 19 + library-path `eprintln!` sites migrated, substrate sites riding + delta D-41) and `pg-fix-forwarder-diagnosability` surfaced the two + arms (`warn` on failed reconnect connects with attempt/backoff + context, `error` on the poll-exit ending).* 8. **sqlite listener-bridge asymmetry** — `listen()` parks one `spawn_blocking` thread per subscription (`notify.rs:98-104`, tokio blocking-pool ceiling ~512) while the stream bridge does the identical job on a dedicated `std::thread` (`stream.rs:368-379`); past ~500 concurrent listeners the *whole* engine's seams queue behind parked bridges. Aligning the two - bridges (std-thread shape) is a small contained change deferred - alongside item 2's subscription-path work. + bridges (std-thread shape) is a small contained change deferred + alongside item 2's subscription-path work. + *Disposition: landed via `sqlite-fix-listener-bridge-alignment` — + the listen bridge takes the stream bridge's named std-thread shape + exactly (including the spawn-failure fail-closed arm).* 9. **sqlite run_schedules error flattening** — `lease.renew(...).await.unwrap_or(false)` maps any renew failure (including the closed-store `Database` error) to `LeadershipLost` (`scheduler.rs:205-211`), misdirecting the documented respawn recipe at a dead store. Small, but the right arm set (closed vs lost vs storage failure) is a contract-adjacent error-classification change - on a stable surface — record, don't rush. + on a stable surface — record, don't rush. + *Disposition: `sqlite-fix-scheduler-error-classification` landed the + loss-vs-failure arm set (`Ok(false)` stays `LeadershipLost`; a + failed renewal propagates its opaque classification) with the dated + ADR-009 §1 annotation; the pg twin discovered mid-batch landed via + `pg-fix-scheduler-error-classification`, implementing to match.* 10. **mem growth postures (N-1 family)** — two perf/posture notes to carry into the mem spec's growth section when next touched: the subscribe path drains the full backlog into the receiver's pending @@ -143,20 +184,32 @@ shape so a future session can decompose without re-deriving. "drains to the tail then parks" shape, but unbounded on the consumer side); the claim path is an O(total-live-rows) scan with full-row clones per claim (`queue.rs:122-181`) — blessed economics - for the ephemeral tier, worth one sentence in `engine-mem.md` if - ever revisited. + for the ephemeral tier, worth one sentence in `engine-mem.md` if + ever revisited. + *Disposition: landed via `mem-doc-growth-outbox-notes` arm (a) — + the growth postures pinned in `engine-mem.md`'s new growth + section, anchored to the sites.* 11. **mem outbox outcome swallow** — `outbox run_once` drops the ack/ retry row-refs' booleans and returns `Ok(true)` even when delivery outlived the visibility window and nothing landed (`outbox.rs:104-122`). Inherent to the no-heartbeat outbox design (at-least-once reclaim is the recovery), but a `debug_assert!` or doc sentence pinning the "processed means delivered-or-reclaimable" - semantic would prevent future confusion. + semantic would prevent future confusion. + *Disposition: bundled into `mem-doc-growth-outbox-notes` arm (b) + — the "processed means delivered-or-reclaimable" semantic pinned + in `engine-mem.md`'s outbox section; the `debug_assert!` declined + with the false-positive rationale recorded in the task.* 12. **core `offset()` initial value** — `EventReceiver::offset()` pins the post-yield value only; the pre-yield return (the seeded checkpoint vs 0) is unpinned in the contract text. A doc-only fix - once the three engines' pre-yield values are confirmed equal (not - verified this session; left as a named gap). + once the three engines' pre-yield values are confirmed equal (not + verified this session; left as a named gap). + *Disposition: landed via `core-pin-offset-initial-value` — the + three engines verified equal (0 on every pre-yield arm, live on + all three columns) and the doc-pin branch applied; pinned in + core-contract.md, the trait doc, and the suite row + `receiver_offset_pre_yield_is_zero`.* 13. **pg identifier truncation cross-channel collision** — `LISTEN`/`UNLISTEN` interpolate the channel as a quoted identifier (truncated at 63 bytes server-side) while `pg_notify` carries it as @@ -166,12 +219,21 @@ shape so a future session can decompose without re-deriving. is deliberate). The honest cheap fix is a deployment-matrix/ docs note ("names longer than the PG identifier limit may collide"); deferred here because the matrix was just finalized by - the gate — fold it into whichever doc pass runs next (or into the - publish session's reading). + the gate — fold it into whichever doc pass runs next (or into the + publish session's reading). + *Disposition: landed via `docs-pg-identifier-truncation-note` — + the note in deployment.md (dated) and engine-postgres.md, the + mechanism sharpened (the server *rejects* ≥ 64-byte `pg_notify` + channels; the collision is on the registration side), the no-cap + verdict untouched.* 14. **core error `Display` log-forging** — `InvalidName`/\ `ReservedName` / `InvalidSpec` interpolate raw consumer strings into `Display`; control characters can forge log lines in naive - consumers. Typical-idiom risk, low; recorded. + consumers. Typical-idiom risk, low; recorded. + *Disposition: landed via `core-fix-display-log-forging` — the + `display_safe` escape helper in the three variants' `Display` + arms (plus `Error::Codec`, the one same-class neighbor); taxonomy + untouched.* 15. **unsafe without `// SAFETY` in the substrate** — the eight `unsafe { ctx.get_connection() }` UDF blocks (`substrate/ops.rs:147-236`) carry no justification comments. The @@ -180,13 +242,20 @@ shape so a future session can decompose without re-deriving. a non-obvious safety constraint would otherwise be missed") says this is where they'd live. Not added this session: the substrate is lineage-port code under ADR-012 §3's diff-fidelity posture — - comments must ride a PROVENANCE delta. Recorded for whenever the - substrate is next touched. + comments must ride a PROVENANCE delta. Recorded for whenever the + substrate is next touched. + *Disposition: landed via `sqlite-substrate-safety-comments` — all + ten same-class sites (the eight + `schema.rs`'s notify + registration + the test-module probe) carry concrete-invariant + `// SAFETY` comments, registered as delta D-40.* 16. **Cosmetic metadata asymmetry** — the three engines' dev-dep tokio features disagree (`rt-multi-thread+macros+time` / `test-util` / `rt+macros`); harmless (the contract suite carries the test - runtime) but an easy-to-trip asymmetry when copying test code. - Recorded, not normalized mid-release. + runtime) but an easy-to-trip asymmetry when copying test code. + Recorded, not normalized mid-release. + *Disposition: landed via `normalize-tokio-dev-deps` — all three + engines normalized to `rt-multi-thread + macros + time + + test-util`, dev-dep metadata only.* ## False positives caught on verification (for the record) diff --git a/tasks/mem-doc-growth-outbox-notes.md b/tasks/mem-doc-growth-outbox-notes.md index 3bf8313..e70e545 100644 --- a/tasks/mem-doc-growth-outbox-notes.md +++ b/tasks/mem-doc-growth-outbox-notes.md @@ -41,11 +41,11 @@ Doc-only plus at most one `debug_assert!` — no behavioral change. ## Acceptance Criteria -- [ ] `engine-mem.md` carries the two growth-posture sentences, +- [x] `engine-mem.md` carries the two growth-posture sentences, anchored to the code sites -- [ ] The outbox "processed means delivered-or-reclaimable" semantic +- [x] The outbox "processed means delivered-or-reclaimable" semantic is pinned (doc sentence; `debug_assert!` per the recorded call) -- [ ] Doc gate green (`RUSTDOCFLAGS="-D warnings" cargo doc +- [x] Doc gate green (`RUSTDOCFLAGS="-D warnings" cargo doc --no-deps`); mem tests green (the `debug_assert!` arm, if added, exercised by an existing outbox test) diff --git a/tasks/normalize-tokio-dev-deps.md b/tasks/normalize-tokio-dev-deps.md index f266706..e7767a6 100644 --- a/tasks/normalize-tokio-dev-deps.md +++ b/tasks/normalize-tokio-dev-deps.md @@ -28,11 +28,11 @@ change. ## Acceptance Criteria -- [ ] The three engines' `[dev-dependencies]` tokio feature sets are +- [x] The three engines' `[dev-dependencies]` tokio feature sets are identical -- [ ] Each engine's own test target green (`cargo test -p` per engine; +- [x] Each engine's own test target green (`cargo test -p` per engine; the pg lib target's server-less SKIP posture is fine) -- [ ] Workspace gates green (build, clippy `-D warnings`, fmt) +- [x] Workspace gates green (build, clippy `-D warnings`, fmt) ## References diff --git a/tasks/pg-fix-scheduler-error-classification.md b/tasks/pg-fix-scheduler-error-classification.md index 31a9fd5..b4f977d 100644 --- a/tasks/pg-fix-scheduler-error-classification.md +++ b/tasks/pg-fix-scheduler-error-classification.md @@ -49,18 +49,18 @@ a mid-run close exits with the closed `Database` classification, never ## Acceptance Criteria -- [ ] The pg renew arm implements the landed arm set: `Ok(false)` → +- [x] The pg renew arm implements the landed arm set: `Ok(false)` → `LeadershipLost` unchanged; `Err(e)` propagates unchanged (no flattening) — no re-deciding; any discrepancy with the sqlite task's decisions of record recorded, not deviated around -- [ ] Both arms pinned by tests against the harness server (the +- [x] Both arms pinned by tests against the harness server (the closed-store arm and the genuine-loss arm, modeled on the sqlite task's two pins) -- [ ] The ADR-009 §1 / core-contract annotations verified to read +- [x] The ADR-009 §1 / core-contract annotations verified to read engine-neutral (they were landed as family-wide; cite them — only pg-specific doc text needs touching, e.g. engine-postgres.md's scheduler row if it describes the arm) -- [ ] `cargo test -p alkstore-postgres` (harness server), clippy +- [x] `cargo test -p alkstore-postgres` (harness server), clippy `-D warnings`, fmt clean; workspace gates green server-less ## References diff --git a/tasks/release-readme.md b/tasks/release-readme.md index 5f750d1..95cc14b 100644 --- a/tasks/release-readme.md +++ b/tasks/release-readme.md @@ -71,7 +71,6 @@ are the wave's other doc-shaped tasks for exactly that reason. ## Notes -> To be filled by implementation agent Per-crate readme wiring — **decision: pointer-style per-crate readmes.** Each of the four published crates gets its own short @@ -180,7 +179,6 @@ pointed to, not copied). ## Summary -> To be filled on completion Landed (2026-10-11): root `README.md` (intro, seven-mechanism interface list, family table with per-engine honest postures, diff --git a/tasks/review-004-fixes.md b/tasks/review-004-fixes.md index 2ee07ce..4fd64c4 100644 --- a/tasks/review-004-fixes.md +++ b/tasks/review-004-fixes.md @@ -1,7 +1,7 @@ --- id: review-004-fixes name: Review gate — review-004 fix batch (recorded-findings dispositions) -status: pending +status: completed depends_on: [sqlite-fix-seam-panic-safety, sqlite-fix-watcher-death-subscribe, sqlite-fix-listener-bridge-alignment, sqlite-fix-scheduler-error-classification, sqlite-substrate-safety-comments, pg-fix-forwarder-hardening, pg-fix-forwarder-diagnosability, decide-log-facade-adoption, pg-decide-stamp-arithmetic-side, pg-fix-bigint-stamp-overflow, pg-fix-scheduler-error-classification, mem-doc-growth-outbox-notes, core-pin-offset-initial-value, core-fix-display-log-forging, docs-pg-identifier-truncation-note, normalize-tokio-dev-deps] scope: broad risk: low @@ -13,7 +13,10 @@ tags: [review-004-fixes, review-gate] ## Description The fix batch's own gate (the `review-wave-4-fixes` pattern): review -004's sixteen recorded findings were decomposed into fifteen tasks; +004's sixteen recorded findings were decomposed into sixteen tasks +(items 2+3 and 4+6 bundled; item 5 split into the decision task and +its implementation; the pg scheduler-classification twin added at the +sqlite task's implementation); verify each disposition is *real* — code-read against the review's failure mechanism, not test-name-trust — and that the fixes didn't damage the postures review 004's inline fixes pinned. @@ -79,13 +82,13 @@ the doc gate. ## Acceptance Criteria -- [ ] Every finding's disposition verified by code-read/live run +- [x] Every finding's disposition verified by code-read/live run against the review's failure mechanism (the per-finding checks above) -- [ ] No regressions in the inline fixes' pinned postures -- [ ] Records landed (review-004 annotations, implementation.md +- [x] No regressions in the inline fixes' pinned postures +- [x] Records landed (review-004 annotations, implementation.md review-rounds entry, task-file audit) -- [ ] All gates re-run green this session (incl. live pg column, fuzz +- [x] All gates re-run green this session (incl. live pg column, fuzz replay, wasm32, doc gate) ## References @@ -99,8 +102,179 @@ the doc gate. ## Notes -> To be filled by implementation agent +Review performed 2026-10-11 (opencode, glm-5.3-flash). Gate checklist +record — every disposition verified against the review's failure +mechanism by code-read, not test-name-trust: + +- **1 (seam panic-safety)**: the armor is real, and the test *injects* + rather than simulates — `store/seam_tests.rs` passes panicking + closures directly into the `pub(crate)` seams; `WriterLease`/` + ReaderLease` (seam.rs) own the connection via `Option` + `Drop`, and + the unwind arm is `Writer::replenish()` (path-based fresh bootstrapped + open, matching the tick's established arm; failed replenish tolerated + with a diagnosability event) and `Readers::abandon()` (outstanding + decrement — the accounting precises out because every connection + carries its fresh-open increment for life — with the waiter notify + under the pool lock). Post-unwind assertions are behavioral: slot + re-acquired, `max_readers: 1` pool re-acquires, full store op + succeeds. The Notes' design of record matches what landed. +- **2 (watcher death + initial-stat)**: `run_poll_loop` sets the dead + sign before the identity-replacement fatal return, and `subscribe` + fails closed on it *and* the taken watcher slot atomically with + enrollment under the senders lock (the belt-and-braces second arm + recorded in Notes, matching). The identity baseline is + `Option<(u64, u64)>` — a failed startup stat leaves it unknown and + the first successful stat captures without driving death. Four new + tests (substrate + engine-level fail-closed, the injected-failure + aliveness drive unix-gated because non-unix `stat_identity` cannot + fail), and the pre-death close-at-death pins checked green. +- **3 (bridge alignment)**: `listen`'s bridge is now a named + `std::thread::Builder` thread (`alkstore-listen-bridge-{channel}`) + — the stream bridge's shape exactly, including the spawn-failure + unsubscribe-and-fail-closed arm; no residual difference is left + unjustified (the Notes name the spawn-failure arm as adopted, not + residual). `listen_after_watcher_death_fails_closed` surfaces + identically through the aligned bridge. +- **4 (scheduler classification)**: both engines' loop-top renew is a + three-way match — `Ok(false)` keeps `Err(LeadershipLost)` + return-before-tick, `Err(e)` propagates its opaque classification + (closed store arrives pre-classified `Database`, engine-wide posture) + — the `.unwrap_or(false)` flattening gone on both, the two pin tests + (`closed_store_run_reports_the_closed_classification_not_leadership_ + lost`, `mid_run_lease_loss_still_returns_leadership_lost`) present on + both engines, and the dated 2026-10-11 ADR-009 §1 annotation of + record verified engine-neutral. +- **5 (substrate SAFETY comments)**: all **ten** `unsafe { ctx. + get_connection() }` sites carry concrete-invariant comments (the + eight review-scoped + the same-class `schema.rs:104` notify + registration + the test-module probe), and the diff-fidelity posture + is honored — PROVENANCE delta **D-40** (category `hygiene`) is + registered with the derived rusqlite #643 soundness argument. This + was the check that mattered most here; it passes. +- **6 (forwarder hardening)**: the TOCTOU restructure is real + decide-and-queue — `ChannelSet::enqueue_register`/ + `enqueue_unregister` make the refcount decision, the `connected` + read, the generation-tag load, and the command enqueue one critical + section under the registry guard (poison-recovered), with the ack + awaited *outside* the guard (never a std lock across an await); + the queue's order is thus the registry transitions' linearization. + The two hang arms are bounded by `bounded_wait` (biased toward the + shutdown flip): the verify batch and the serve/replay commands ride + `LISTENER_QUERY_TIMEOUT` (5 s), the failed-verify settle rides + `LISTEN_SETTLE_TIMEOUT` (2 s) with an unsettled reaper aborted + (the connection closes with it). Preserved postures re-verified + green: the register-failure/subscribe-failure unregisters in place + (notify.rs both arms), the poison-recovery idiom throughout, the + replay-proofs recorded in the task's Notes (the pin fails on the + review's exact assert with the fix reverted). The Notes' thread- + preemption window analysis matches the landed guard placement. +- **7 (log facade + diagnosability)**: ADR-025 exists with the user + sign-off recorded in its status (tracing adopted, engine crates + only, core dep-free and emit-free); zero library-path `eprintln!` + remain (grep), both engines carry `tracing = "0.1"`; the two + diagnosability arms emit in the decided form (`warn` on failed + reconnect connects with attempt/backoff context, `error` on the + poll-exit endings); the secret-free grep re-run clean (the remaining + `.ok()`s are tests' env plumbing). +- **8 (BIGINT stamps)**: the decided guard at the decided sites as one + owner — `resolution.rs::saturating_stamp_expr` called at exactly the + decision's six sites (queue claim/retention/retry/heartbeat, lock + insert/renew) with the `($N::bigint)::numeric` chain (the live-caught + `WrongType` refinement recorded in the owner's doc); the decision + task (ADR-023 §2 dated annotation, window verified open) and the + implementation task both green against the harness server, no + re-deciding — the one refinement is recorded, not deviated around. +- **9 (mem doc notes)**: `engine-mem.md` carries the new growth-postures + section (both postures anchored to `stream.rs`/`queue.rs` sites) and + the outbox-semantics subsection ("processed means delivered-or- + reclaimable"); the `debug_assert!` declined with a real + false-positive rationale recorded (in-closure handle ops would + trip it). +- **10 (offset pre-yield pin)**: the three-way verification evidence + exists — the suite row `receiver_offset_pre_yield_is_zero` (four + arms incl. the ADR-023 §2 negative-seed arm and the first-observation + composition leg), green on all three columns; the contract text + pinned in three places (core-contract.md's streams row + verification + backlog, the trait doc — no divergence to raise, so the AC's + divergent branch correctly unticked as N/A). +- **11 (Display forging)**: `display_safe` (per-char `escape_debug`) + in the four `Display` arms (`InvalidName`/`ReservedName`/`InvalidSpec` + + `Codec`, the same-class neighbor the task's grep found); the + escaping test pins zero emitted control bytes; the taxonomy + (variants/fields/match sites) byte-identical. +- **12 (identifier note)**: the note is in the release text — + deployment.md (dated, at the pg co-tenancy paragraph) and + engine-postgres.md — with the accuracy sharpening recorded (the + server rejects ≥ 64-byte `pg_notify` channels; the collision is on + the registration side); the no-cap validation verdict untouched. +- **13 (tokio dev-deps)**: the three engines' `[dev-dependencies]` + tokio feature sets are identical + (`rt-multi-thread + macros + time + test-util`); tests green. + +**No regressions (re-spot-checked at the touched sites)**: the tick's +`drop-+-replenish` disposition intact (`(out, conn, poisoned)` + the +commit/rollback-fault seams); the tx-form negative seed intact (pg +`save_offset_tx` stores raw, no `GREATEST`; mem's `has_offset_row` +probe distinguishes the absent checkpoint); the sleep caps (`MAX_SLEEP_S` +30-day cap on all three engines); the sqlite claim stamp's +`saturating_add` (scheduler.rs:346); the register-failure/subscribe- +failure unregisters; the poison-recovery lock idiom; `encode_payload`'s +127-container depth limit; the doc corrections. + +**Records landed this gate**: review 004's sixteen +`Findings recorded, not fixed` items annotated with their dispositions +and task pointers (the record reads closed); +`implementation.md`'s review-rounds entry for the batch; the task-file +audit — all sixteen dependencies `completed`, Notes and Summary filled. + +**Minor records issues found and fixed inline (comment/metadata only)**: +- stale placeholder lines (`> To be filled …`) left above real content + in two batch task files (`sqlite-fix-watcher-death-subscribe`, + `sqlite-fix-listener-bridge-alignment`) — removed; the identical + pre-existing residue in three earlier completed tasks + (`review-wave-3`, `release-readme`, and — noted by `review-wave-5` + itself as the class's precedent — same shape) cleaned in the same + stroke. +- completed-AC checkboxes left unticked on five batch tasks + (`sqlite-fix-seam-panic-safety`, `sqlite-substrate-safety-comments`, + `mem-doc-growth-outbox-notes`, `normalize-tokio-dev-deps`, `pg-fix- + scheduler-error-classification`) — ticked after this gate verified + each; the one intentionally-conditional divergence AC + (`core-pin-offset-initial-value`'s) left unticked as N/A. +- the gate task's own description said "fifteen tasks"; corrected to + the honest sixteen (the pg twin added mid-batch). ## Summary -> To be filled on completion \ No newline at end of file +**Review gate passed: all sixteen dispositions verified against +review 004's failure mechanisms by code-read; the inline fixes' +pinned postures re-spot-checked with no regressions; records landed; +all gates green.** + +Highlights (full checklist in Notes): + +- Findings 1–4: the armor/state-machine changes are structurally what + the tasks' Notes of record pin (lease Drop arms, the dead sign, the + two bridges' aligned shapes, the twin three-way renew matches) — and + every injected-panic / death-injection / interleave test drives the + real mechanism, not a simulation. +- Finding 5's PROVENANCE delta D-40 (the diff-fidelity posture's + purpose) verified; Findings 6–11's behavioral shapes confirmed + (decide-and-queue under the registry guard, `bounded_wait` at every + listener await, the `saturating_stamp_expr` six-site owner, ADR-025's + decided emission form, the three-place offset pin, the display + escaping). +- The batch's own honest-record trail held up under audit — every + Notes section (design of record, form refinements, declined + alternatives like mem's `debug_assert!`) matches what landed — + with three minor record-hygiene defects fixed inline (placeholders, + AC checkboxes, the description's task count). + +Gates this session: `cargo build`; server-less `cargo test +--workspace` (core 28+3+1 doc, mem 102+26, sqlite 203+26, pg lib +SKIP-posture 135+26+9, all green); the full pg column live vs the +harness server (`pglo-poc` :15432 — 135 lib + 26 suite + 9 schema); +`cargo clippy --all-targets -- -D warnings`; `cargo fmt --check`; +the fuzz corpus replay (`fuzz/shared`, 6/6); the wasm32 mem check; +the doc gate (`RUSTDOCFLAGS="-D warnings" cargo doc --no-deps`, zero +warnings); `taskgraph validate` (96 tasks). \ No newline at end of file diff --git a/tasks/review-wave-3.md b/tasks/review-wave-3.md index f97b9ca..d48e04b 100644 --- a/tasks/review-wave-3.md +++ b/tasks/review-wave-3.md @@ -67,7 +67,6 @@ Check: ## Notes -> To be filled by implementation agent Two findings, both fixed inline (2026-10-08). Neither needed a new session or scope decision: diff --git a/tasks/sqlite-fix-listener-bridge-alignment.md b/tasks/sqlite-fix-listener-bridge-alignment.md index 9e40f39..4297721 100644 --- a/tasks/sqlite-fix-listener-bridge-alignment.md +++ b/tasks/sqlite-fix-listener-bridge-alignment.md @@ -51,8 +51,6 @@ stream bridge does. ## Notes -> To be filled by implementation agent - Decisions of record the description didn't pin: - The two bridges' shapes are now exactly aligned: a named @@ -78,8 +76,6 @@ Decisions of record the description didn't pin: ## Summary -> To be filled on completion - `listen()`'s watcher-fanout bridge now runs on a dedicated named `std::thread` instead of a tokio `spawn_blocking` task per subscription — the stream bridge's shape, eliminating the blocking-pool diff --git a/tasks/sqlite-fix-seam-panic-safety.md b/tasks/sqlite-fix-seam-panic-safety.md index 0e4e71a..46a33f2 100644 --- a/tasks/sqlite-fix-seam-panic-safety.md +++ b/tasks/sqlite-fix-seam-panic-safety.md @@ -50,18 +50,18 @@ restored and a later reader acquires. ## Acceptance Criteria -- [ ] The abandonment design of record is decided and written into +- [x] The abandonment design of record is decided and written into Notes (mechanism per seam, which dispositions, which precedents it follows) before implementation lands -- [ ] A panic inside the `with_writer` closure leaves the writer slot +- [x] A panic inside the `with_writer` closure leaves the writer slot grantable after the unwind — pinned by a test with an injected panic (slot re-acquired, full store operation succeeds after) -- [ ] A panic inside the `with_reader`/`blocking_read` closures leaves +- [x] A panic inside the `with_reader`/`blocking_read` closures leaves the reader pool's outstanding count restored and a later reader acquires — pinned by test -- [ ] Normal (non-panic) paths are behaviorally unchanged: the +- [x] Normal (non-panic) paths are behaviorally unchanged: the existing seam/slot/pool tests green unchanged -- [ ] `cargo test -p alkstore-sqlite`, clippy `-D warnings`, fmt +- [x] `cargo test -p alkstore-sqlite`, clippy `-D warnings`, fmt clean; workspace gates green server-less ## References diff --git a/tasks/sqlite-fix-watcher-death-subscribe.md b/tasks/sqlite-fix-watcher-death-subscribe.md index 1ce8563..5d10fdc 100644 --- a/tasks/sqlite-fix-watcher-death-subscribe.md +++ b/tasks/sqlite-fix-watcher-death-subscribe.md @@ -69,8 +69,6 @@ subscribers stay open. ## Notes -> To be filled by implementation agent - Decisions of record the description didn't pin: - The dead sign lives on `SharedUpdateWatcher` as an @@ -114,8 +112,6 @@ Decisions of record the description didn't pin: ## Summary -> To be filled on completion - Both defects landed with tests: - (a) Post-watcher-death subscribe hang: the poll loop sets the dead diff --git a/tasks/sqlite-substrate-safety-comments.md b/tasks/sqlite-substrate-safety-comments.md index 0df5941..4d7a678 100644 --- a/tasks/sqlite-substrate-safety-comments.md +++ b/tasks/sqlite-substrate-safety-comments.md @@ -41,16 +41,16 @@ if not, leave them and record why. ## Acceptance Criteria -- [ ] Each of the eight UDF blocks (plus any same-class neighbors +- [x] Each of the eight UDF blocks (plus any same-class neighbors included) carries a `// SAFETY` comment stating the concrete soundness invariant -- [ ] `src/substrate/PROVENANCE.md` carries a dated delta entry +- [x] `src/substrate/PROVENANCE.md` carries a dated delta entry registering the comment-only change (diff-fidelity posture honored; the entry classifies the delta per the register's conventions) -- [ ] No behavioral change: the substrate floor tests and the engine's +- [x] No behavioral change: the substrate floor tests and the engine's tests green unchanged -- [ ] `cargo test -p alkstore-sqlite`, clippy `-D warnings`, fmt +- [x] `cargo test -p alkstore-sqlite`, clippy `-D warnings`, fmt clean; workspace gates green server-less ## References