Skip to content

test(auth): stop starting the real-clock rate-limit test near a window edge - #823

Merged
jason931225 merged 1 commit into
mainfrom
fix/otp-rate-limit-window-flake
Aug 19, 2026
Merged

test(auth): stop starting the real-clock rate-limit test near a window edge#823
jason931225 merged 1 commit into
mainfrom
fix/otp-rate-limit-window-flake

Conversation

@jason931225

Copy link
Copy Markdown
Collaborator

The flake

otp_redeem_rate_limit_wires_up_on_real_clock_path drives ten requests expecting 401, then asserts the eleventh is 429:

assertion `left == right` failed: the trusted ingress identity must select the first per-IP bucket

Why it fails

The limiter buckets into a fixed one-minute tumbling window, not a sliding one — floor_to_window in crates/platform/auth-rest/src/lib.rs:

let floored = unix - unix.rem_euclid(window_secs);   // WINDOW = Duration::minutes(1)

So all eleven requests must land inside the same window. Each round-trips to PostgreSQL, so the test occupies a real fraction of that window — and whenever the loop straddles a minute boundary the counter resets mid-loop and the eleventh request becomes the new window's first, returning 401.

It did exactly that on run 32225725163, failing #802 — a PR that changes nothing near auth — and blocking it behind an unrelated red.

The fix

The sibling named in this test's own doc comment, rate_limit_trips_at_cap_and_resets_after_window, drives now directly and has no such exposure. This test exists to prove the real clock path is wired, so it cannot inject a clock — but it can decline to start near a boundary:

let into_window = OffsetDateTime::now_utc().unix_timestamp().rem_euclid(60);
if into_window > WINDOW_SECS - NEEDED_SECS {   // NEEDED_SECS = 30
    tokio::time::sleep(...).await;
}

The assertion is unchanged. This removes a timing dependency the test never meant to have, not a behaviour it meant to check.

Verification

cargo check -p console-app --tests, cargo fmt --check, cargo clippy --tests — all clean.

Unblocks

#802.

🤖 Generated with Claude Code

…w edge

`otp_redeem_rate_limit_wires_up_on_real_clock_path` drives ten requests
expecting 401 and asserts the eleventh is 429. The limiter buckets into a
FIXED one-minute TUMBLING window -- `floor_to_window` in
crates/platform/auth-rest/src/lib.rs floors to `unix - unix.rem_euclid(60)`
-- not a sliding one, so all eleven must land inside the SAME window. When
the loop straddles a minute boundary the counter resets mid-loop and the
eleventh request is the new window's first, returning 401:

  assertion `left == right` failed: the trusted ingress identity must
  select the first per-IP bucket

Each of the eleven round-trips to PostgreSQL, so on a loaded runner the
test occupies a real fraction of the window and fails whenever it starts
late in one. It did exactly that on run 32225725163, failing #802 -- a
pull request that changes nothing near auth -- and blocking it behind an
unrelated red.

The sibling named in this test's own doc comment,
`rate_limit_trips_at_cap_and_resets_after_window`, drives `now` directly
and has no such exposure. This one exists to prove the REAL clock path is
wired, so it cannot inject a clock -- but it can decline to start near a
boundary. It now waits for a fresh window when fewer than 30 seconds
remain in the current one.

The assertion itself is unchanged: this removes a timing dependency the
test never meant to have, not a behaviour it meant to check.

Verified: cargo check -p console-app --tests, cargo fmt --check and cargo
clippy --tests all clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jason931225
jason931225 force-pushed the fix/otp-rate-limit-window-flake branch from 45cf186 to 45e7cd5 Compare August 19, 2026 07:35
@jason931225
jason931225 added this pull request to the merge queue Aug 19, 2026
Merged via the queue into main with commit a9aa94b Aug 19, 2026
32 checks passed
@jason931225
jason931225 deleted the fix/otp-rate-limit-window-flake branch August 19, 2026 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant