Wave-2 review gate: scheduler fire expires resolution fix (D-27, ADR-020 §2), pressure-test lock quality (D-28), lineage diff re-verified clean (task review-wave-2)

tasks/review-wave-2.md

@@ -1,7 +1,7 @@
 ---
 id: review-wave-2
 name: Review gate — wave 2 (substrate fork)
-status: pending
+status: completed
 depends_on: [fork-provenance-and-floor]
 scope: narrow
 risk: low
@@ -45,12 +45,12 @@ Check:
 
 ## Acceptance Criteria
 
-- [ ] Lineage diff reviewed; every divergence registered or fixed
-- [ ] Re-derivation matches ADR-010's pinned semantics (spot-checked
+- [x] Lineage diff reviewed; every divergence registered or fixed
+- [x] Re-derivation matches ADR-010's pinned semantics (spot-checked
       against the ADR text, not just the tests)
-- [ ] Provenance register accurate
-- [ ] All gates green
-- [ ] Findings recorded; wave 3 decomposition may proceed
+- [x] Provenance register accurate
+- [x] All gates green
+- [x] Findings recorded; wave 3 decomposition may proceed
 
 ## References
 
@@ -61,8 +61,83 @@ Check:
 
 ## Notes
 
-> To be filled by implementation agent
+Review performed over a fresh full-coverage lineage diff (subagent,
+honker-core @ `f4e53c6` via `git show` vs the substrate tree) plus an
+ADR-010/020/019 spot-check of the re-derivation against the ADR text
+(not the tests).
+
+**Fidelity — clean.** Every kept-half divergence is register-addressable:
+the table-family rename, W-1 (backoff tick counter verified at
+`MAX_RECONNECT_TICKS`; initial-open path lineage-identical), W-2
+(fallible spawn; baseline-wait carried verbatim), dead-man's switch
+(panic → log+`return`; WatcherDeathGuard byte-faithful), W-4 keep-RW
+(open flags identical to lineage), the `pragma_table_info` race re-key,
+the D-16 attach-prune cap, `cron_expr` → `spec`, schema/migration
+extensions (column-order invariant pinned), and hygiene deltas. All
+kept internal names preserved. Drops verified absent (no cron symbols,
+no kernel/shm watcher, no rate-limit/result surfaces, no scheduler
+pause/resume/list/update). queue_ops.rs smuggle-scan: no `.ok()`
+swallows, no lineage-identical defect-carrying code.
+
+**Re-derivation vs ADR-010 — one real defect found and fixed this
+session (D-27)**: `scheduler_tick` passed the schedule row's *relative*
+`expires_s` directly into the re-derived `enqueue`'s *absolute*
+`expires_at` parameter — the lineage resolved `expires_s → now + s`
+inside its enqueue, and ADR-020 §2 pins the relative→absolute
+resolution at the enqueue instant. A schedule with `expires_s: Some(30)`
+would have fired jobs with `expires_at = 30` (expired since 1970):
+swept straight to dead, silent work loss. No prior test covered expiry
+through the tick. Fixed at the fire site in the tick's transaction
+(one clock read, `now + s`); regression test
+`tick_resolves_the_fires_expires_sticker_to_an_absolute_row_value` pins
+the resolved value and the fired job's claimability. This was the
+review's only code-behavior fix.
+
+**Minor, fixed inline (D-28)**: the cross-mechanism pressure test's
+lock-churn branch used a literal `'p{producer}'` (no interpolation —
+all producers contended on one lock name) and swallowed its result via
+`.unwrap_or(0)`; now per-producer names, periodic release, errors
+propagate. Test-quality only.
+
+**Minutiae, no action needed (recorded here, not as register rows)**:
+`column_present`'s prepare-error → "absent" fallback produces a
+different error message than the lineage on a missing table (bootstrap
+still errors either way); bootstrap/`poll_data_version` doc-comment
+trims (D-15-adjacent); the concurrent-open race test's tuned magnitudes.
+None change behavior.
+
+**Register**: D-27 (re-derivation fix) and D-28 (test hygiene) appended
+after the review's fixes; header updated. Cherry-picks stay empty.
+
+**Gates**: `cargo build` ok; workspace `cargo test` — 23 core + 3
+contract-suite + 84 sqlite (83 inherited + 1 new regression) — green in
+one run; `cargo clippy --all-targets -- -D warnings` clean;
+`cargo fmt --check` clean. The flake watch-record of
+`fork-provenance-and-floor` (one unreproducible transient) did not
+recur in this session's runs.
+
+**Boundary arithmetic check (ADR-012 §2)**: the equal-jitter curve's
+arithmetic is absent from the substrate — `retry` takes a caller-
+resolved literal delay; no `backoff`/curve symbols exist. The schedule
+boundary math present (`parse_every_interval`, `next_every_boundary`,
+`skip_forward`) is the ADR-010 §6 / fork-rederive-queue-ops-Notes
+sanctioned re-derivation surface (`@every`-only numeric advance per
+ADR-009 §2; cron machinery and local-TZ arithmetic absent).
+
+**Wave 3 watch-item (no action this wave)**: the W-2 `Result<_, String>`
+error is untyped — the engine-layer mapping into the `Database` variant
+is wave-3 work per fork-port-connection-watcher's Notes; nothing here
+blocks decomposition.
 
 ## Summary
 
-> To be filled on completion
+Wave-2 review gate complete. A fresh full-coverage lineage diff
+(honker-core @ `f4e53c6`) found every kept-half divergence
+register-addressable — no unregistered divergence. The ADR-spot-check
+of the re-derivation found one real contract-semantics defect
+(scheduler fire's relative `expires_s` passed through as an absolute
+`expires_at`; silent expiry-at-1970 work loss under
+`expires_s`-bearing schedules) — fixed inline with a regression test,
+registered D-27; one test-quality wart fixed and registered D-28.
+Gates: build, workspace test (110 green), clippy `-D warnings`, fmt —
+all clean. Wave 3 decomposition may proceed.