diff --git a/docs/architecture/decisions/023-fourth-review-round.md b/docs/architecture/decisions/023-fourth-review-round.md index 094472b..ed9bfd2 100644 --- a/docs/architecture/decisions/023-fourth-review-round.md +++ b/docs/architecture/decisions/023-fourth-review-round.md @@ -170,7 +170,69 @@ offsets are total.** Each disposition answers to a decided principle: checkpoint are the [ADR-019](019-mechanism-handle-surfaces.md) §6 monotone no-ops — rejections were never the behavior, so pinning the duration row's shape here would have contradicted all three - engines.)* + engines.) + +*(Annotation, 2026-10-11 — the pg stamp-arithmetic guard decision +([ADR-017](017-contract-versioning.md) §2 class 1, the window +verified open — no tag exists, no `cargo publish` has occurred; the +review-004 item-5 pre-fix precedent). Review 004's post-round finding: +pg's server-side stamp arithmetic — the claim's +`$2 + visibility_timeout_s` (claim deadline), the heartbeat's +`$3 + extend`, the retry-reschedule's `$2 + delay_s`, the retention +sweep's `$2 − dead_letter_retention_s`, and the lock stamp/renew's +`$3 + ttl` (`lock.rs:92/217`) — overflow-failed on plain bigint +("bigint out of range") and the failure is *sticky per row*, one +extreme-stamped row poisoning every later claim/heartbeat/sweep that +sums against it. The side question is here because the boundary row's +totality lives on the *argument* values these sums consume, and the +two candidate guards differ in what they touch: an enqueue-seam +clamp/validate owns the guard at the entry of the consumer-supplied +i64s; a server-expression guard clamps the arithmetic *result*. +**Decided: the server expression owns it — the clamp domain is the +sum's result, no input is clamped or validated at any seam.** The +enqueue seam cannot own the guard: four of the six arguments are +per-call (`heartbeat(extend)`, the `retry(err, Some(d))` override, +`try_lock`/`renew(ttl)` ×2) and have no enqueue to clamp at; and +where an enqueue does exist, a *result* saturation is what the +pinned posture already computes, not a new input rule — enqueue's +own stamps resolve in Rust `saturating_add` (`resolution.rs`, both +engines), the boundary row pins argument totality ("any i64 is +well-defined… no clamping, no rejection" — a caller's +`heartbeat(i64::MAX)` or `retry(err, Some(i64::MAX))` is a value, +not a violation), and `QueueOpts` stamps enter rows unvalidated +verbatim by the recorded review-002 disposition, so validating them +at entry would be a *posture change*, not a guard. The guard form, +precise: **every server-side stamp expression saturates its result +to the i64 domain** — `LEAST(GREATEST(, +-9223372036854775808), 9223372036854775807)::bigint`, subtraction +sites analogously — verified green against a Postgres 16 harness: an +overflowing sum clamps at either edge, a representable result rides +untouched (`1700000000 + i64::MIN` stays exact, matching Rust's +`saturating_add`), `now − i64::MIN` clamps to `i64::MAX` (matching +mem's retention `saturating_sub`). The extreme-stamped row's +observable semantics, per site: a row stamped +`visibility_timeout_s = i64::MAX` claims to the `i64::MAX` deadline +(the claim never lapses through expiry; effectively-never, the same +shape as sqlite's REAL-approximation and mem's saturation); a row +stamped `dead_letter_retention_s = i64::MIN` sweeps every dead row +(`sat(now − MIN) = i64::MAX`); `heartbeat(extend)` extremes renew to +the saturated edge (a past saturated deadline voids through the +validity predicate on the next op, the annotation above's negative +semantics preserved); `retry(err, Some(i64::MAX))` reschedules to +`i64::MAX` (ready ~never, the caller's value honored to the edge); +`ttl = i64::MAX` stamps an `i64::MAX` expiry. No contract text +changes: the table pins these dispositions already — what the +annotation completes is the *arithmetic form*, correcting the +"plain SQL integer add" parenthetical above: on pg the pinned +saturating arithmetic is realized as the server-side guard, not a +plain add, and the "bigint out of range" failure leaves the +reachable surface (no opaque-Err arm covers arithmetic overflow; +the duration row's rejection stays `ttl <= 0` only). Sites of +record: pg `queue.rs:386/912/1000/715`, `lock.rs:92/217`; enqueue +itself stays Rust-resolved (no SQL-side sum at enqueue). The +implementation rides `tasks/pg-fix-bigint-stamp-overflow.md` — +`cargo test -p alkstore-postgres` live-server legs pin the +sticky-poison retirement.)* This is a **core-contract amendment** (doc text on `claim_batch`, both `read` forms, `try_lock`/`renew`, and a shared domain-rule diff --git a/tasks/pg-decide-stamp-arithmetic-side.md b/tasks/pg-decide-stamp-arithmetic-side.md index 528394c..87a6772 100644 --- a/tasks/pg-decide-stamp-arithmetic-side.md +++ b/tasks/pg-decide-stamp-arithmetic-side.md @@ -1,7 +1,7 @@ --- id: pg-decide-stamp-arithmetic-side name: Decide which side guards pg's server-side stamp arithmetic (review 004 item 5, decision) -status: pending +status: completed depends_on: [] scope: narrow risk: low @@ -49,15 +49,15 @@ observable behavior of an extreme-stamped row becomes. ## Acceptance Criteria -- [ ] The decision of record: which side owns the guard, with the +- [x] The decision of record: which side owns the guard, with the ADR-023 §2 totality argument and the sibling-engine uniformity check written down -- [ ] The window state verified and recorded; a dated annotation +- [x] The window state verified and recorded; a dated annotation landed if class-1, or user sign-off if post-publish -- [ ] The guard form is precise (clamp domain, sites, the +- [x] The guard form is precise (clamp domain, sites, the extreme-stamped row's observable semantics) — enough for `pg-fix-bigint-stamp-overflow` to implement without re-deciding -- [ ] The decision recorded where the family keeps such records (ADR +- [x] The decision recorded where the family keeps such records (ADR annotation / task Notes + contract text if the wording needs it) ## References @@ -72,8 +72,76 @@ observable behavior of an extreme-stamped row becomes. ## Notes -> To be filled by implementation agent +**The site set is six, not four** — the lock arithmetic is two +expressions, and the review's `lock.rs:91` line ref was the try_lock +insert only: `lock.rs:92` (insert) and `lock.rs:217` (renew) both sum +`$3::bigint + $4::bigint` over the per-call `ttl`; and the retention +sweep's subtraction is `queue.rs:715` (the `:714` ref was the +`IS NOT NULL` line above it). The guard form must cover the *renew* +arm too. + +**Per-call vs stamped arguments split the option-(a) story.** Four of +the six arguments are per-call (`heartbeat(extend)`, the +`retry(err, Some(d))` override, `try_lock`/`renew(ttl)` ×2) — there +is no enqueue to clamp at. The other two ride enqueue/stamped +sources (`visibility_timeout_s`, `dead_letter_retention_s` — +`QueueOpts`, stamped verbatim per the review-002 disposition, which +recorded *unvalidated* stamping as the posture, not a defect). So +option (a) as written decomposes into two different entry rules — +one of which (validating QueueOpts) contradicts the recorded +review-002 posture, and the other (validating per-call boundary +args) contradicts ADR-023 §2's totality wording itself ("no +clamping, no rejection" — `heartbeat(i64::MAX)` is a value, not a +violation). Option (b) needs neither posture change: it realizes +what the pinned totality already computes. + +**The pinned arithmetic's form, not its totality, is what was +wrong.** ADR-023 §2's 2026-10-11 annotation cites "saturating_add / +plain SQL integer add" as the three engines' code of record and +calls it total — correct for the *non-positive* values that +annotation classified, but pg's plain bigint add was never total in +the overflow domain (Postgres errors; SQLite's overflow→REAL +approximates; Rust saturates). The decision therefore needs **no +contract-text change** — the table pins the dispositions already; +the annotation *corrects the form* on pg. ## Summary -> To be filled on completion \ No newline at end of file +Decision of record: **(b), the server expression owns the guard**; +clamp domain = the arithmetic *result*, saturated to the i64 domain; +no input clamped, validated, or rejected at any seam. Guard form: +every server-side stamp expression computes its sum in `numeric` and +clamps before the bigint cast — +`LEAST(GREATEST(, -9223372036854775808), 9223372036854775807)::bigint` +— the SQL realization of the same saturating arithmetic the pinned +code of record runs. Sites of record: pg `queue.rs:386/912/1000/715` +(claim, heartbeat, retry-reschedule, retention sweep) and +`lock.rs:92/217` (try_lock insert, renew); enqueue itself stays +Rust-resolved (`resolution.rs`'s `saturating_add` — never a SQL-side +sum). Extreme-stamped-row observables (all three engines uniform): +`visibility_timeout_s = i64::MAX` → claim deadline `i64::MAX` +(effectively never lapsing); `dead_letter_retention_s = i64::MIN` → +swept-before `i64::MAX` (sweeps everything, matching mem's +`saturating_sub`); negative-extreme `extend`/delay → a past +deadline voided by the uniform validity predicate on the next op; +`ttl = i64::MAX` → `i64::MAX` expiry. "bigint out of range" leaves +pg's reachable surface; the sticky-poison class dies; the duration +row's opaque rejection stays `ttl <= 0` only. + +Recorded as the dated 2026-10-11 annotation in +`docs/architecture/decisions/023-fourth-review-round.md` §2 +(amending the boundary-row annotation's arithmetic parenthetical in +place — ADR-017 §2 class 1). **Window state verified open** at +decision time (2026-10-11): no git tags exist, no `cargo publish` +has occurred, and ADR-017 §2's termination of record pins the class +permanently at the first core-crate publish — so the annotation +landed pre-publish, legal. + +The guard form was validated against the live Postgres 16 harness +server before pinning: clamping legs green (overflow both edges +clamps; `1700000000 + i64::MIN` rides exact, matching Rust +`saturating_add`; `now − i64::MIN` clamps to `i64::MAX`), so +`pg-fix-bigint-stamp-overflow` can implement the form verbatim +without re-deciding. No code changed (a decision task); no crate +touched, so no build/test gates re-run — the doc-only diff carries +no rustdoc surface. \ No newline at end of file