Skip to content

fix(server): a mid-turn usage update no longer hides a failed probe - #15116

Open
andrewwbuilds wants to merge 5 commits into
pingdotgg:mainfrom
andrewwbuilds:fix/usage-limits-sparse-update-after-failed-probe
Open

andrewwbuilds wants to merge 5 commits into
pingdotgg:mainfrom
andrewwbuilds:fix/usage-limits-sparse-update-after-failed-probe

Conversation

@andrewwbuilds

@andrewwbuilds andrewwbuilds commented Oct 3, 2026 •

Copy link
Copy Markdown

Fixes #12170. The earlier fix, #12294, was closed during the V2 rewrite with a request to open a fresh PR against the new code once V2 landed. The V2 Claude and Codex adapters still feed live rate-limit events through applyUsageLimitsUpdate, so the bug is unchanged on main.

Problem

When a usage probe fails, the provider publishes { windows: [], unavailable: { reason: "probeFailed" } } and Limits shows "Could not read limits." During the next turn, Claude's rate_limit_event names one window, usually seven_day. applyUsageLimitsUpdate merged that window onto the empty list and rebuilt the snapshot without unavailable. Limits then drew a lone Weekly bar as if it were the account's full read. The next failed probe kept that one-window list as the last good snapshot, because resolveUsageLimitsAfterProbe only replaces a published snapshot that still carries unavailable. It stayed that way until a probe succeeded.

A merge onto no snapshot at all had the same result. That happens when a turn reports usage before the first probe finishes, or after a probe that stopped before producing usageLimits: Claude's capability probe failing, or a Codex app-server timeout.

Codex goes through the same merge, because account/rateLimits/updated is a partial view too (see mergeCodexRateLimits).

Change

applyUsageLimitsUpdate in packages/provider-core/src/server/usageLimits.ts now:

  • copies previous.unavailable onto the rebuilt snapshot, the same way it already copies resetCredits;
  • marks a merge onto previous === undefined as probeFailed;
  • returns previous unchanged whenever no window moved. The old check skipped that early return whenever previous.unavailable was set, which forced a rebuild that dropped the marker. Keeping the identity matters for Codex, which repeats the same numbers beside every token-usage tick. Without it, a marked snapshot would be republished on every tick.

Streamed windows are still stored. A successful probe replaces the whole snapshot as before, and unsupported is still rejected outright. A merge onto a complete snapshot is unchanged.

There is no client change. Web, desktop and mobile already show "Could not read limits." instead of bars for any snapshot with unavailable set (limitsNotice, collectLimitAccounts). The triage called that the correct interim. As the triage suggested, I left out a partial reason (bars plus a notice) and hydrating usageLimits from the status cache on restart.

One visible difference: if a CLI's usage read always fails, its usage used to appear the first time a turn streamed it, and that partial list then stuck. Now it shows "Could not read limits." until a probe succeeds. The affected cases are a claude binary without the get_usage call, or a Codex build whose account/rateLimits/read errors.

Scope and approval

#12170 is a triaged bug labeled accepted. The triage comment describes this fix: keep the marker on merge, including the no-op rebuild, and mark previous === undefined as probeFailed. The change is one server function plus tests, with no contract or client change.

Verification

  • New tests fail on main and pass on this branch.
    • usageLimits.test.ts, "keeps a failed probe marked when a sparse update lands on it": also checks that repeating the same update returns the same object.
    • usageLimits.test.ts, "marks a sparse update with nothing published as a failed read".
    • On main, both of those return a snapshot with no unavailable.
    • managedProvider.test.ts, "does not publish a window streamed after a failed probe as the full list": runs the issue's sequence through the real managed provider. The sequence is failed probe, streamed weekly window, failed probe, successful probe. On main, the second failed probe still publishes the lone weekly window:
      - "checkedAt": "2026-04-10T00:00:01.000Z", "unavailable": { "reason": "probeFailed" }, "windows": []
      + "checkedAt": "2026-04-10T00:00:02.000Z", "windows": [{ "id": "seven_day", "usedPercent": 20, ... }]
      
  • Focused tests: vp test run src/server/usageLimits.test.ts src/server/managedProvider.test.ts in packages/provider-core: 19 passed. After merging main, which moved these files into provider-core, the full provider-core suite (190) and the server usage-limit and registry tests (142) also pass. One of the 19 is the existing "keeps live usage windows across a failed probe and a stale enrichment" test, which covers a successful probe, then a live update, then a failed probe. It still passes, so bars from a good read still survive a failed probe.
  • Wider suite: vp test run src/provider plus the Claude and Codex V2 adapter tests: 1,553 passed. 4 tests in OpenCode2Client.events.test.ts and CodexAdapterV2.test.ts failed, and they fail the same way on main in my checkout: the space in its path gets URL-encoded into a fixture path. They pass in CI on main.
  • Static checks: tsc --noEmit -p . in apps/server: no errors. vp check and vp run knip:check: pass.
  • Callers checked: Claude and Codex are the only drivers that stream usage updates, through ClaudeDriver, CodexDriver and CodexManagedProvider. Managed Codex publishes unsupported, so it ignores updates before and after this change. Reset credits are set only by a successful probe, so the redeem flow never starts from a marked snapshot.
  • Environment: macOS 26 (arm64), Node 24.16.
  • Not checked: a live run. Reproducing this needs a Claude get_usage probe that fails while a turn still streams rate_limit_event. I couldn't trigger that on demand without patching the probe. The tests drive the same applyUsageLimits and publish path that the adapters call.

applyUsageLimitsUpdate rebuilt the snapshot without `unavailable`, so a
single streamed window landing on a failed probe, or on no probe yet, was
published as the account's complete limits. The next failed probe then kept
that partial list as the last good one.

The merge now carries `probeFailed` forward, and marks a merge onto nothing
as `probeFailed`, until a probe reads the account. An unchanged update still
returns the same snapshot.

Refs: pingdotgg#12170
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 3, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 3, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at d62c03f

Macroscope's review found this PR approvable — This is a small, well-tested server bug fix that keeps failed usage probes marked when partial runtime updates arrive, preventing incomplete windows from being shown as a full account read. Successful probes and existing provider behaviors remain unchanged, with no schema, configuration, or sensitive-area impact.

No code changes detected at 702bb7a. Prior analysis still applies.

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

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 3, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 3, 2026 09:28

Dismissing prior approval to re-evaluate 6175bbd

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 3, 2026
@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8619f12c-87b3-41fe-9130-ea348daf32d8

📥 Commits

Reviewing files that changed from the base of the PR and between 95ef0da and 702bb7a.


📒 Files selected for processing (1)
  • packages/provider-core/src/server/managedProvider.test.ts

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



📝 Walkthrough

Walkthrough

Usage limit updates now retain probeFailed when streamed windows are merged into a failed or absent snapshot. Tests cover unchanged updates, repeated failed probes, and a later successful probe.

Changes

Usage limit updates

Layer / File(s) Summary
Sparse usage update handling
packages/provider-core/src/server/usageLimits.ts, packages/provider-core/src/server/usageLimits.test.ts, packages/provider-core/src/server/managedProvider.test.ts
Changed windows retain the previous unavailable status, or receive probeFailed when no snapshot exists. Unchanged updates return the existing snapshot. Tests cover streamed updates after failed probes and later probe results.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge


Merge Risk | ⚪ Minimal · up to 702bb

Merge Risk: ⚪ Minimal · up to 702bb

This change keeps a failed usage-limit read marked as failed when streamed windows arrive mid-turn, instead of showing a partial list as complete. It is narrow and tested, and I found no merge-blocking risk.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 702bb

The change preserves failure status and prevents partial usage data from appearing complete. The reviewed implementation introduces no new access path or privileges and leaves account-switch protections intact.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated changed outcome is provider usage-status publication and downstream account-limit presentation. The modified helper does not introduce an identity transition, credential operation, tool execution, or new privileged sink.

Trust Boundaries and Controls

  • observed — The runtime-update API carries no account or workspace provenance. Its existing caller merges into the current snapshot, while account-switch checks apply to probe results. Delayed-event attribution remains an unverified pre-existing limitation, not an introduced or worsened boundary change established by this review.

Resilience and Maintainability Implications

  • observed — Refresh serialization and enrichment-generation checks remain in place. Snapshot mutation precedes publication, so strict externally observed ordering during overlapping refresh and runtime updates is not established by the inspected tests; this ordering mechanism is unchanged.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title is concise, specific, and accurately describes the primary server-side fix for failed probes and mid-turn usage updates.
Description check Passed The description includes the required Problem, Change, Scope and approval, and Verification sections. It links the triaged issue, explains the behavior change, documents focused and wider test results…
Linked Issues check Passed Issue #12170 requires a streamed Claude usage update after a failed probe to remain unavailable instead of appearing as a complete window list. The change preserves previous.unavailable, marks a spa…
Out of Scope Changes check Passed The changes are limited to usage-limit merge behavior, its documentation, and focused tests. These changes directly support issue #12170. No unrelated client, contract, or product behavior is added.

✨ 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.

Main renamed AlwaysRunTestLayer to layerAlwaysRunTest (pingdotgg#16282), and the merge
moved this test into provider-core without picking up the new name.
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 10, 2026 08:30

Dismissing prior approval to re-evaluate d62c03f

@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Hi! We are cleaning up open PRs, and this one does not say which model or harness was used to create it. If this change is really important, we recommend rebuilding the PR with a newer model and noting the model and harness in the PR description.

@maria-rcks maria-rcks closed this Oct 11, 2026
@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Reopening, this was closed by mistake. Sorry for the noise!

@maria-rcks maria-rcks reopened this Oct 11, 2026
AdEx-Partners-DE added a commit to AdEx-Partners-DE/t3code that referenced this pull request Oct 11, 2026
… newer test layer

The case from upstream pingdotgg#15116 uses AlwaysRunTestLayer, which this base does
not have yet. The usageLimits unit test for the same fix stays.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

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

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews 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.

Claude usage limits: a rate_limit_event after a failed probe is published as a complete window list

3 participants