fix: heartbeat extends on alternate beats to stop expiry drift (v0.3.1)

[amavashev]

Summary

P1 liveness, fleet-wide (same bug in all four SDKs). The protocol's extend_by_ms extends relative to the reservation's current expires_at_ms (cycles-protocol-v0.yaml: "relative to its current expires_at_ms (i.e., not relative to request time)"), but src/heartbeat.rs sent ExtendRequest::new(ttl_ms) on every ttl/2 beat — drifting the expiry outward by ttl/2 per beat.

Consequences of the drift:

• Kill the process after a long run and the reserved budget stays locked until the drifted expiry — a zombie window that scales with runtime, capped only by the server's max_extensions (default 10 → up to ~6×ttl).
• Extensions burn twice as fast as needed, so runs longer than ~max_extensions × ttl/2 exhaust the allowance and lose heartbeat protection mid-flight.

Fix — alternate-beat extension

Same sleep interval ((ttl/2).max(1s)); the first beat extends (only ttl/2 of lifetime remains at that point); after each successful ACTIVE extend, exactly one beat is skipped; after a failed extend (transport/API error, or an HTTP-success response whose status is not ACTIVE — forward-compat Unknown), the next beat retries immediately. Extend amount stays ttl_ms. The client clock is deliberately never compared against the server's expires_at_ms — clock skew makes that unsafe.

Net: no drift; expiry lead oscillates within [ttl/2, 1.5·ttl]; extension consumption halved.

The guard does not track expiry from extend responses (it stores the reserve-time expires_at_ms once), so no lifecycle behavior changed there.

Tests

New tests/heartbeat_test.rs — three wiremock tests running a real short-TTL (1 s) heartbeat and asserting extend-call counts per beat (each request body checked for extend_by_ms == ttl_ms):

• Alternate beats: beat 1 extends, beat 2 skipped, beat 3 extends (pins against the old every-beat behavior).
• Failure retry: beat 1's extend 500s → beat 2 retries immediately → beat 3 skipped after the successful retry.
• Non-ACTIVE status: an HTTP-200 extend with unrecognized status earns no skip → next beat retries; skip resumes after ACTIVE.

Verification

• cargo test — full suite green (incl. 8 doc-tests)
• cargo clippy --all-targets --all-features -- -D warnings — clean
• cargo fmt --check — clean
• cargo tarpaulin — 95.81% (617/644 lines; heartbeat.rs +35.45%)

Housekeeping

• Version 0.3.0 → 0.3.1 (Cargo.toml + Cargo.lock)
• CHANGELOG [0.3.1] - 2026-07-27; AUDIT.md dated entry

[amavashev]

Redesign: alternate-beat -> lead-estimate heartbeat (ba8be61)

Adversarial self-review of the alternate-beat scheme shipped in 2aad4f8 found confirmed liveness regressions, so this PR now replaces it with a lead-estimate scheduler.

What was wrong with alternate-beat

1. Single-failure lapse at zero margin — after a failed extend, exactly one retry beat remained before expiry; a second hiccup lapsed the reservation.
2. Guaranteed lapse for spec-legal ttl_ms in (1000, 2000) — the 1-second interval floor put the first beat at 1000 ms for ttl 1200, then a skip cycle past expiry. (The old tests pinned TTL_MS = 1000 — inside the broken regime.)
3. Beat slip by one RTT per cycle — sleep after each awaited extend pushed every subsequent beat later by the request round trip.
4. Non-ACTIVE 2xx treated as failure — a 2xx means the server DID apply the extension (expires_at_ms is authoritative proof), so this branch would re-extend every beat against any server sending a newer status string: exactly the drift the fix was for.
5. Permanent failures retried forever — MAX_EXTENSIONS_EXCEEDED / RESERVATION_EXPIRED etc. produced doomed traffic and log noise indefinitely.
6. Fresh idempotency key per retry — an applied-but-lost extension followed by a fresh-key retry could double-extend.

The v2 design

• Interval is exactly ttl/2 — floor removed (spec min ttl 1000 -> 500 ms beats).
• Each beat computes lead = (known_expiry - initial_expiry) + ttl - elapsed (signed): server-frame minus server-frame, monotonic minus monotonic — the client wall clock is never compared against the server's, so skew cannot corrupt it, and the estimate is conservative (extends early, never late). The reserve response's expires_at_ms is threaded from the guard into start_heartbeat.
• lead >= 1.5*ttl -> skip; else extend by ttl_ms. Any shortfall (failure, clamped grant, small ttl) extends on the very next beat — no fixed cadence to lapse through; steady state still halves extension consumption vs the original every-beat drift.
• Beats are anchored with tokio::time::interval_at + MissedTickBehavior::Skip (no RTT slip; realigns after stalls).
• Any 2xx counts as applied: known_expiry updates from the response expires_at_ms; an unrecognized status only warns (reverses item 4).
• Transient failures keep and reuse the idempotency key next beat (server replay dedupes a lost response); fresh key only after a resolved outcome (fixes 6).
• Permanent codes stop the loop: RESERVATION_EXPIRED, RESERVATION_FINALIZED, MAX_EXTENSIONS_EXCEEDED, or any HTTP 410 — logged once, heartbeat self-terminates (fixes 5).
• Cancellation semantics unchanged (CancellationToken; guard cancels on commit/release/drop).

Tests

tests/heartbeat_test.rs rewritten (wiremock, dynamic expires_at_ms responders that grant prev + step per call):

• v2 cadence under full grants: extend@1, extend@2, skip@3 (lead exactly 1.5·ttl), extend@4;
• 500 -> retry reuses the same idempotency key (asserted from received request bodies), fresh key after success;
• 409 MAX_EXTENSIONS_EXCEEDED stops the heartbeat — no further requests across subsequent beats;
• ttl = 1200 stays alive across four 600 ms beats (pins the floor removal — regime 2 above);
• clamped grants (+ttl/4) extend every beat (signed lead math, no lapse);
• 200 with unknown status (SUSPENDED) counts as applied — beat 3 skips, pinning the reversal of item 4.

Plus unit tests for the lead math (inclusive 1.5·ttl threshold, negative lead on stall, clamped-grant never-skip) and permanent-failure classification in src/heartbeat.rs.

Verification

• cargo test — all green (6 wiremock heartbeat tests + 5 new unit tests; full suite passes)
• cargo clippy --all-targets -- -D warnings — clean
• cargo fmt --check — clean
• tarpaulin coverage 95.70% (gate: >= 95%)
• CHANGELOG [0.3.1] and AUDIT.md rewritten for the redesign

---

Follow-up: effective-TTL seeding + expanded permanent set (2aaf337)

Spec review confirmed two more issues, folded into the same branch:

1. Tenant policy silently caps the granted TTL

max_reservation_ttl_ms (governance default 1 hour) caps the grant at reserve, and the create response has no effective-TTL field — so seeding the heartbeat from the requested TTL schedules the first beat far too late (24 h request capped to 1 h -> first beat at 12 h, 11 h after expiry). Fix:

• reserve() now captures the HTTP Date response header (new ApiResponse::date_ms, parsed with httpdate — already in the tree via hyper, promoted to a direct dependency);
• effective_ttl = clamp(expires_at_ms - date_ms, 1000 ms, requested) when both samples are available, else the requested TTL. Both terms are server-frame -> still clock-skew-free; the Date header's 1-second resolution is negligible against the spec's 1000 ms TTL minimum;
• the effective TTL drives everything: the beat interval (effective/2), the lead formula's ttl term, the 1.5x skip threshold, and the per-beat extend amount.

2. Two more permanent stop codes

TENANT_CLOSED (tenant closure is irreversible without administrative action) and NOT_FOUND / raw HTTP 404 (a 404'd reservation never returns) now terminate the heartbeat, joining RESERVATION_EXPIRED / RESERVATION_FINALIZED / MAX_EXTENSIONS_EXCEEDED / HTTP 410.

Tests added

• capped scenario: requested 8000 / granted 2000 (reserve Date + expires_at pair) -> first beat at 1000 ms (not 4000 ms), extends by 2000 (not 8000), full extend/extend/skip cadence derived from the effective TTL;
• garbage Date header -> fallback to the requested TTL (no beat at 500 ms, first beat at requested/2);
• 409 TENANT_CLOSED and 404 NOT_FOUND stop the loop — no further requests (shared scaffold with the MAX_EXTENSIONS_EXCEEDED test);
• unit tests: effective-TTL (capped grant recovery, never exceeds requested, clamps to the 1000 ms minimum, fallbacks) and Date parsing (IMF-fixdate exact ms, garbage/empty/invalid -> None);
• shared reserve mocks (tests/common/mod.rs) now stamp a Date consistent with their fixed expires_at_ms, keeping heartbeats quiescent in unrelated suites.

Verification (follow-up)

• cargo test — all green (10 wiremock heartbeat tests, 64 lib unit tests)
• cargo clippy --all-targets -- -D warnings — clean; cargo fmt --check — clean
• tarpaulin coverage 95.77%
• CHANGELOG [0.3.1] and AUDIT.md extended

[amavashev]

v2.2 (spec review round 3): the HTTP Date header is demoted from correctness input to first-beat cadence hint. RFC 9110 Date is a whole-second, best-effort origination timestamp that intermediaries may replace — and in cycles-server expires_at_ms is stamped from Redis TIME while Date comes from the HTTP layer, so expires_at_ms − Date is not a lease measurement, and the old 1000 ms upward clamp could fabricate lease the server never granted.

Correctness now rests on a grant ledger: lead_min = grants_sum − elapsed (signed, starts 0) is a rigorous lower bound built only from same-frame arithmetic — each grant is the difference of successive server-frame expires_at_ms values. A beat skips only when lead_min ≥ 1.5·last_grant; otherwise it extends by the requested ttl_ms. First beat at min(requested/2, 30 s, date_hint/2) (hint raw and unclamped, dropped when underivable); after each success the delay is clamp(last_grant/2, 500 ms, requested/2), so the cadence tracks the server's observed grants (clamped grants speed it up). Per-beat computed sleeps replace interval_at, scheduled from intended instants with stall realignment (Skip-equivalent). Same-key transient retry and the permanent stop set (6 codes + HTTP 410/404) are unchanged.

Wiremock suite retraced (cadence extend@1..4 / skip@5 / extend@6; capped grant: requested 8000 with +2000 grants → 1000 ms beats, extend_by_ms stays 8000; unknown-status 200 still applied); the pure delay/lead computations are extracted and unit-tested, including the 30 s first-beat cap. Coverage 95.73%; tests, clippy -D warnings, fmt green. Same version 0.3.1 — CHANGELOG/AUDIT amended in place. Commit 2346011.

[amavashev]

v2.3 (spec review round 4) — pushed 5f1f7ba, same 0.3.1 release, CHANGELOG/AUDIT amended in place.

Two confirmed findings, both fixed in src/heartbeat.rs:

1. First beat is now immediate. Any bounded first-beat delay can outlive a small tenant-capped lease (the 30 s cap is still 28 s late for a 2 s grant), so the first extend fires at delay 0 — it costs one extension but provably beats an arbitrarily small lease, and it primes the grant ledger with a real sample. All Date-derived effective-TTL/hint scheduling is removed from the heartbeat path; ApiResponse::date_ms + httpdate parsing remain as general response utilities (unit + e2e tested, unused by the heartbeat). A transient failure on the immediate beat retries at the held cadence min(requested/2, 30 s) with the same idempotency key — never a zero-delay hot loop.

2. Lead-clamp regime. Under a maximum-LEAD clamp (extend re-stamps expires_at ≈ now + L), successive expires_at_ms differences measure elapsed time, not lease — grant-derived cadence self-collapses to the 500 ms floor and burns max_extensions in seconds. Each success is classified: grant ≤ 0, or < 0.9·requested and within [0.75, 1.25]× elapsed-since-last-success, holds the cadence at min(requested/2, 30 s) (never tightened) and warns once per heartbeat. Note the lower 0.75× band arm is a v2.3 refinement of the review's rule: without it, a real per-extend grant observed across a skip-doubled gap (grant = 2·cadence·… ⇒ grant ≈ elapsed exactly) is misclassified as lead-clamped, loosens to the held cadence, and decays the lease to a lapse — the band lets it re-tighten, keeping the required 'grant-clamp (+ttl/4) still tightens' behavior sustainable.

Everything else unchanged (skip rule, requested-amount wire extends, key reuse, permanent stops, 2xx-as-applied, cancellation).

Verification: full suite green (13 test targets incl. doc-tests), clippy -D warnings clean, cargo fmt --check clean, tarpaulin 95.12% (682/717). Wiremock suite reworked: immediate first beat, recomputed extend/skip pattern (E@0/1000/2000, skip@3000, E@4000), lead-clamp echo responder (cadence holds at ttl/2, no 500 ms collapse), zero-grant immediate prime → held cadence, grant-clamp +ttl/4 still tightens, 503-on-first-beat held-cadence retry, permanent stops, unknown-status 2xx — plus unit tests on the extracted pure cadence/regime functions (30 s cap, band boundaries).