Skip to content

Protect critical usage boundary behavior - #246

Merged
tsouth89 merged 2 commits into
mainfrom
test/boundary-regression-tests
Aug 9, 2026
Merged

Protect critical usage boundary behavior#246
tsouth89 merged 2 commits into
mainfrom
test/boundary-regression-tests

Conversation

@tsouth89

@tsouth89 tsouth89 commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • pin quota-run observed-drop behavior at and just below the 20-point reset threshold
  • preserve the last pre-reset usage as the completed run endpoint and record the low reading as post-reset usage
  • cover Claude OAuth's five-minute refresh skew with a deterministic clock
  • cover usage-pace will-last, exhaustion ETA, zero-usage, and invalid-window boundaries
  • exercise multi-domain browser-cookie fallback across App-Bound Encryption, empty headers, and readable later domains

Closes SBS-641.
Closes SBS-643.
Closes SBS-646.
Closes SBS-647.

Validation

  • cargo test --manifest-path rust/Cargo.toml (859 library tests, CLI test, and doc tests passed)
  • cargo test --manifest-path apps/desktop-tauri/src-tauri/Cargo.toml (483 passed)
  • focused Claude OAuth suite (28 passed)
  • focused usage pace suite (6 passed)
  • focused browser cookies suite (10 passed)
  • focused quota run history suite (10 passed)
  • cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings
  • cargo clippy --manifest-path apps/desktop-tauri/src-tauri/Cargo.toml --all-targets -- -D warnings
  • cargo fmt --all --manifest-path rust/Cargo.toml
  • git diff --check

Summary by CodeRabbit

  • Bug Fixes
    • Improved usage tracking when consumption drops, accurately closing completed usage periods and starting new ones when appropriate.
    • Improved cookie retrieval across multiple domains, allowing fallback when a domain returns no cookies or a supported encryption-related result.
    • Improved token expiration handling, including the expected five-minute expiration buffer.
    • Improved usage-pace calculations for exhaustion estimates, zero usage, and invalid time-window inputs.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
ceiling 51c4e9d Commit Preview URL

Branch Preview URL
Aug 09 2026, 02:30 PM

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 49 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f0e52d04-2998-4c4a-81c0-927b533fd184

📥 Commits

Reviewing files that changed from the base of the PR and between a86c748 and 51c4e9d.

📒 Files selected for processing (4)
  • apps/desktop-tauri/src-tauri/src/commands/providers.rs
  • apps/desktop-tauri/src-tauri/src/quota_run_history.rs
  • rust/src/providers/claude/oauth/mod.rs
  • rust/src/providers/claude/web_api.rs
📝 Walkthrough

Walkthrough

The PR updates quota run finalization, adds multi-domain cookie lookup fallback, expands usage pace coverage, and makes Claude OAuth expiry checks deterministic through an explicit-time helper.

Changes

Quota run history

Layer / File(s) Summary
Observed-drop finalization
apps/desktop-tauri/src-tauri/src/quota_run_history.rs
Threshold-triggered drops preserve the pre-drop usage as end_used_percent and record the post-drop reading as after_reset_used_percent. Tests cover threshold and sub-threshold drops.

Cookie domain fallback

Layer / File(s) Summary
Multi-domain cookie lookup
rust/src/browser/cookies.rs
Adds get_cookie_header_for_domains and tests fallback after App-Bound Encryption errors, all-ABE failures, and final non-ABE error preservation.

Usage pace coverage

Layer / File(s) Summary
Usage pace cases
rust/src/core/usage_pace.rs
Tests cover on-track behavior, ahead-of-pace exhaustion ETA, zero usage, and invalid weekly-window boundaries.

OAuth expiry evaluation

Layer / File(s) Summary
Deterministic expiry checks
rust/src/providers/claude/oauth/mod.rs
is_expired delegates to is_expired_at. Tests cover the five-minute expiry skew, expired tokens, and missing expiry metadata.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: finesssee

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary focus on protecting critical usage-boundary behavior, including quota and timing boundaries.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/boundary-regression-tests

Comment @coderabbitai help to get the list of available commands.

@tsouth89
tsouth89 force-pushed the test/boundary-regression-tests branch from acf17b1 to a86c748 Compare August 9, 2026 14:19

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
apps/desktop-tauri/src-tauri/src/quota_run_history.rs (1)

208-222: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Process classified capacity events before recording the snapshot.

When one sample causes both an observed drop and a classified reset event, record_snapshot closes the existing run and opens a new run before record_capacity_events processes the event. The event then finalizes that new run with event.previous_used_percent, creating a spurious second run with zero duration. Add a regression test for this production call order.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop-tauri/src-tauri/src/quota_run_history.rs` around lines 208 -
222, Update record_snapshot and record_capacity_events so classified capacity
events are processed before the snapshot-driven observed-drop finalization,
preventing the event from closing the newly opened run with
event.previous_used_percent. Add a regression test covering the production call
order where one sample contains both an observed drop and a classified reset
event, and assert no spurious zero-duration second run is created.
rust/src/browser/cookies.rs (1)

716-733: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Route Claude cookie lookup through this selector.

rust/src/providers/claude/web_api.rs lines 263-283 still loops over get_cookie_header directly and discards every CookieError. If an App-Bound Encryption error occurs and no domain returns a header, that runtime path returns ProviderError::NoCookies instead of the actionable ABE error preserved here.

Use browser_cookie_header(&domains) or get_cookie_header_for_domains(&domains) in that path. This keeps domain fallback and error selection consistent.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/src/browser/cookies.rs` around lines 716 - 733, Update the Claude cookie
lookup path in the web API flow to call browser_cookie_header or
get_cookie_header_for_domains with the domain list instead of looping over
get_cookie_header directly. Preserve domain fallback while propagating the
selector’s chosen CookieError, including AppBoundEncryption, rather than
converting all failures to ProviderError::NoCookies.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rust/src/providers/claude/oauth/mod.rs`:
- Around line 457-473: Add an exact five-minute boundary assertion in
token_expiry_honors_the_five_minute_refresh_skew, setting expires_at to now +
ChronoDuration::minutes(5) and asserting credentials.is_expired_at(now) is true.
Keep the existing six-minute, one-minute, past-expiry, and missing-expiry cases
unchanged.

---

Outside diff comments:
In `@apps/desktop-tauri/src-tauri/src/quota_run_history.rs`:
- Around line 208-222: Update record_snapshot and record_capacity_events so
classified capacity events are processed before the snapshot-driven
observed-drop finalization, preventing the event from closing the newly opened
run with event.previous_used_percent. Add a regression test covering the
production call order where one sample contains both an observed drop and a
classified reset event, and assert no spurious zero-duration second run is
created.

In `@rust/src/browser/cookies.rs`:
- Around line 716-733: Update the Claude cookie lookup path in the web API flow
to call browser_cookie_header or get_cookie_header_for_domains with the domain
list instead of looping over get_cookie_header directly. Preserve domain
fallback while propagating the selector’s chosen CookieError, including
AppBoundEncryption, rather than converting all failures to
ProviderError::NoCookies.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e80b00f0-7866-491a-807b-9913f2ba09cc

📥 Commits

Reviewing files that changed from the base of the PR and between 695e2d5 and a86c748.

📒 Files selected for processing (4)
  • apps/desktop-tauri/src-tauri/src/quota_run_history.rs
  • rust/src/browser/cookies.rs
  • rust/src/core/usage_pace.rs
  • rust/src/providers/claude/oauth/mod.rs

Comment thread rust/src/providers/claude/oauth/mod.rs
@tsouth89
tsouth89 merged commit bc9076c into main Aug 9, 2026
10 checks passed
@tsouth89
tsouth89 deleted the test/boundary-regression-tests branch August 9, 2026 14:34
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