Skip to content

fix(server): preserve known Claude usage-limit resets - #15694

Closed
ashx-j wants to merge 1 commit into
pingdotgg:mainfrom
ashx-j:fix/issue-15665-claude-usage-reset
Closed

ashx-j wants to merge 1 commit into
pingdotgg:mainfrom
ashx-j:fix/issue-15665-claude-usage-reset

Conversation

@ashx-j

@ashx-j ashx-j commented Oct 4, 2026

Copy link
Copy Markdown

A repeated Claude rejected rate-limit event without resetsAt erased a future reset timestamp already reported for the same window. Activity could therefore show a countdown while the terminal failure lost its reset and recovery controls.

Preserves the prior future timestamp only when the same named window omits resetsAt. Recovery events still clear it. Expired or explicitly invalid timestamps, unidentified windows, and separate exhausted windows with unknown resets still prevent scheduling. Automatic recovery remains controlled by the user's setting.

Refs #15665. The reporter's raw SDK events are unavailable, so this fixes a demonstrated cause consistent with the report without claiming its exact run is fully resolved.

Validation: ten new terminal-failure cases produced three failures and seven passes against the original adapter. With the patch, vp test run apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts passed all 140 tests. The server typecheck passed with GOMAXPROCS=2 GOMEMLIMIT=3GiB ./node_modules/.bin/tsc --noEmit -p apps/server/tsconfig.json. The two issue typechecks ran sequentially. Targeted formatting, lint, and whitespace checks passed, with one unchanged existing lint warning for unused layer. A separate GPT-6-Astra reviewer at medium reasoning approved the patch with no findings. No live provider session or browser verification was performed.

Implemented by GPT-6.1-Sol at xhigh reasoning through the Codex harness.

@github-actions github-actions Bot added size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Oct 4, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 699f3ee

Macroscope's review found this PR approvable — This is a localized server bug fix that preserves an already reported future Claude usage-limit reset only when a repeated event omits the timestamp, while retaining existing recovery and invalid-data handling. The accompanying tests cover the reset, expiry, recovery, and multi-window cases without introducing schema or deployment changes.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
docs/internals/effect-services.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d1da612f-5a65-472b-910e-75df45ee5c63
📥 Commits

Reviewing files that changed from the base of the PR and between 4ee6bfd and 699f3ee.

📒 Files selected for processing (2)
  • apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts
  • apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The Claude adapter now preserves a prior future reset timestamp when a blocked rate-limit update omits one. It validates supplied reset values. Parameterized tests cover reset selection and usage-limit failure classification.

Changes

Claude rate-limit reset tracking

Layer / File(s) Summary
Reset timestamp handling and validation
apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts, apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts
For a blocked limit window, the adapter preserves a prior reset only when the update omits resetsAt and that reset is still in the future. It accepts supplied values only when they are finite, positive, and below 8.64e15; otherwise it stores null. Tests cover repeated and multiple windows, recovery, unidentified windows, invalid or expired values, and missing rate-limit updates. They assert the terminal failure class and resetAt.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 699f3

No actionable merge-blocking risk is identified; the change is ready for normal merge checks.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 699f3

The fix preserves a previously reported reset only for the same named limit window while that reset remains in the future. Recovery still clears the stored value, and an exhausted window with an unknown reset still prevents reporting a combined reset. No new access path, privilege, or weakened security control was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed behavior is bounded to reset metadata for the active Claude turn. The inspected change does not expand tenant, credential, tool, network, or data-store authority.

Trust Boundaries and Controls

  • observed — The adapter validates provider-supplied timestamps before formatting them. Retention requires an explicitly named window, an omitted reset, and a prior future value for that same key; unidentified windows cannot use the retention branch.

Resilience and Maintainability Implications

  • observed — Recovery clearing, sequential SDK event handling, interruption suppression of failures, and active-turn cleanup remain unchanged. The retained value is an absolute timestamp, not an extension of the reset interval. The new tests do not advance time between retention and terminal emission.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preserving known Claude usage-limit reset timestamps.
Description check ✅ Passed The description explains the problem, change, and verification in detail. It references issue #15665, but does not state whether the issue was triaged or approved, or explain why this focused fix qual…
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant