diff --git a/alkstore-sqlite/src/substrate/PROVENANCE.md b/alkstore-sqlite/src/substrate/PROVENANCE.md index ece5c4b..f4b67c8 100644 --- a/alkstore-sqlite/src/substrate/PROVENANCE.md +++ b/alkstore-sqlite/src/substrate/PROVENANCE.md @@ -82,8 +82,10 @@ only, not committed — the wave-2 review's primary input), and every divergence it surfaced is either a register entry below (D-21..D-26, appended; D-12 annotated in place) or a fixed omission (the lineage-less placeholder test in `ops.rs`'s pressure suite, removed). -No unregistered divergence remains. Cherry-picks: empty at scaffold, -append-only forever. +No unregistered divergence remains. The wave-2 review gate +(`review-wave-2`) re-ran the full-coverage lineage diff (clean — +everything register-addressable) and appended D-27/D-28 for its two +fixes. Cherry-picks: empty at scaffold, append-only forever. | ID | Category | What | Where | Why | Lineage | |----|----------|------|-------|-----|---------| @@ -113,6 +115,8 @@ append-only forever. | D-24 | hygiene | `real_to_i64` boundary error text re-owned ("honker:" prefix → "alkstore:"; the lineage's better-sqlite3-facing sentence elided per D-15) | ops.rs (`real_to_i64`) | D-19's re-owning pattern at the error-string layer | ours | | D-25 | hygiene | watcher thread name "honker-update-poll" → "alkstore-update-poll" | watcher.rs (spawn path) | D-19's re-owning pattern at the thread-name layer | ours | | D-26 | hygiene | `volume_serial_number as u64` → `u64::from(volume_serial_number)` in `VolumeIdentity` (clippy lint conformance, zero behavior change) | watcher.rs (`stat_identity` region) | family clippy `-D warnings` gate | ours | +| D-27 | re-derivation | wave-2 review fix — `scheduler_tick`'s fire enqueue resolved the schedule row's relative `expires_s` into the absolute row `expires_at` (`now + s` at the fire transaction, one clock read) instead of passing the relative value through; regression test pins a fired job's `expires_at = fire-instant + s` and its claimability. The pre-fix pass-through would have produced rows expired in 1970 under any `expires_s` schedule | `queue_ops.rs` (`scheduler_tick`, fire-expiry resolution + test) | ADR-020 §2 (relative `expires` → resolved absolute row value, resolved at enqueue against the single clock) — D-12's landed state carried the lineage's `expires_s` field shape without carrying the lineage's enqueue-side resolution; wave-2 review (task review-wave-2) re-derivation spot-check found it | ours | +| D-28 | hygiene | wave-2 review fix — the cross-mechanism pressure test's lock-churn branch exercised a per-producer lock name (literal `"p{producer}"` had no interpolation, so all producers contended on one name) with its result swallowed via `.unwrap_or(0)`; now interpolates, releases periodically, propagates errors | `ops.rs` (pressure test, lock branch) | test-quality only (no lineage counterpart, no library code touched); `fork-provenance-and-floor`'s ported-surface pressure intent | ours | ### Cherry-picks diff --git a/alkstore-sqlite/src/substrate/ops.rs b/alkstore-sqlite/src/substrate/ops.rs index 8b55e82..6e07b1b 100644 --- a/alkstore-sqlite/src/substrate/ops.rs +++ b/alkstore-sqlite/src/substrate/ops.rs @@ -567,13 +567,23 @@ mod pressure_tests { ) .unwrap(); if seq % 13 == 0 { - let _: i64 = conn + let lock_name = format!("pressure-l-{producer}"); + let locked: i64 = conn .query_row( - "SELECT alkstore_lock_acquire('pressure-l', 'p{producer}', 30)", - [], + "SELECT alkstore_lock_acquire(?1, ?2, 30)", + rusqlite::params![lock_name, format!("p{producer}")], |r| r.get(0), ) - .unwrap_or(0); + .unwrap(); + if locked == 1 && seq % 39 == 0 { + let _: i64 = conn + .query_row( + "SELECT alkstore_lock_release(?1, ?2)", + rusqlite::params![lock_name, format!("p{producer}")], + |r| r.get(0), + ) + .unwrap(); + } } if seq % 11 == 0 { std::thread::sleep(Duration::from_millis(1)); diff --git a/alkstore-sqlite/src/substrate/queue_ops.rs b/alkstore-sqlite/src/substrate/queue_ops.rs index 6c510f5..2ec773a 100644 --- a/alkstore-sqlite/src/substrate/queue_ops.rs +++ b/alkstore-sqlite/src/substrate/queue_ops.rs @@ -373,17 +373,19 @@ pub(crate) fn scheduler_tick(conn: &Connection, now: i64) -> rusqlite::Result= SCHEDULER_MAX_CATCHUP_FIRES { next_fire_at = skip_forward(next_fire_at, now, interval); break; } - let job_id = enqueue(conn, &queue, &payload, now, priority, expires_s, stamps)?; + let fire_at = next_fire_at; + let job_id = enqueue(conn, &queue, &payload, now, priority, expires_at, stamps)?; out.push(json!({ "name": name, "queue": queue, - "fire_at": next_fire_at, + "fire_at": fire_at, "job_id": job_id, })); fires += 1; @@ -1895,6 +1897,52 @@ mod queue_tests { assert_eq!(jobs, 64, "the skipped boundaries never enqueue"); } + #[test] + fn tick_resolves_the_fires_expires_sticker_to_an_absolute_row_value() { + let conn = db(); + let now: i64 = now_unix(&conn).unwrap(); + scheduler_register( + &conn, + "t", + "jobs", + "@every 60s", + "\"work\"", + fire(0, Some(30)), + stamps_cfg(300, 3), + ) + .unwrap(); + conn.execute( + "UPDATE __alkstore_scheduler_tasks SET next_fire_at = ?1 - 5 WHERE name = 't'", + [now], + ) + .unwrap(); + scheduler_tick(&conn, now).unwrap(); + + let expires_at: Option = conn + .query_row( + "SELECT expires_at FROM __alkstore_live WHERE queue = 'jobs' LIMIT 1", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!( + expires_at, + Some(now + 30), + "the fire's relative expiry resolves against the fire instant \ + (ADR-020 §2), not passed through as a raw unix value" + ); + + let unexpired: i64 = conn + .query_row( + "SELECT count(*) FROM __alkstore_live + WHERE queue = 'jobs' AND (expires_at IS NULL OR expires_at > unixepoch())", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(unexpired, 1, "the fired job must still be claimable"); + } + #[test] fn tick_ignores_disabled_and_future_tasks() { let conn = db(); diff --git a/tasks/review-wave-2.md b/tasks/review-wave-2.md index 7f37ff0..dfd39ed 100644 --- a/tasks/review-wave-2.md +++ b/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 \ No newline at end of file +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. \ No newline at end of file