Throttle background Claude CLI usage spawns to a 15-minute floor - #2923
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Codex review: needs changes before merge. Reviewed August 14, 2026, 1:57 AM ET / 05:57 UTC. ClawSweeper reviewWhat this changesThe PR adds a 15-minute in-memory cache for successful background Claude CLI usage fetches so repeated app refreshes avoid relaunching the Claude CLI, while manual and recovery fetches remain immediate. Merge readinessKeep this owner-authored PR open: the current diff still has two concrete P2 blockers—its cache omits web-enrichment setting inputs, and it edits the release-owned changelog outside the release flow. Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s Claude provider selects a usage source and invokes the Claude CLI during refreshes. This throttle intercepts eligible background CLI refreshes, returns a recent successful usage result when safe, and otherwise proceeds to the normal probe and menu/provider display. flowchart LR
A[Background app refresh] --> B[Claude usage strategy]
B --> C{Eligible cached result?}
C -->|Yes| D[Return prior usage snapshot]
C -->|No| E[Run Claude CLI probe]
E --> F[Build usage result]
F --> G[Cache successful result]
D --> H[Provider and menu display]
G --> H
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Fingerprint all non-sensitive web-enrichment inputs or invalidate enriched cache entries when those settings change, add regression coverage for that transition, and leave release-note ownership to the release flow. Do we have a high-confidence way to reproduce the issue? Yes, from the supplied current diff: cache lookup is keyed without the web-enrichment settings that feed enriched Claude results, so changing those settings within 15 minutes can reuse the old snapshot. Is this the best way to solve the issue? No: throttling is a focused approach, but the cache must account for every result-shaping enrichment input and should not include a normal PR edit to the release-owned changelog. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a0a514fdaf30. LabelsLabel justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (2 earlier review cycles)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0ea74ba20
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ClaudeCLIUsageSpawnThrottle.key( | ||
| binary: $0, | ||
| environment: context.env, | ||
| useWebExtras: self.useWebExtras, | ||
| includePrepaidBalance: self.includePrepaidBalance && context.includeOptionalUsage) |
There was a problem hiding this comment.
When Claude CLI usage is enriched with web extras or prepaid balance, the fetcher below uses self.manualCookieHeader and context.settings?.claude?.organizationID, but this throttle key only varies by the two booleans. For the same CLI account, if the manual/auto web cookie or target org changes after a successful enriched fetch, background refreshes can return the previous enriched ProviderFetchResult for up to 15 minutes, so cost/extra-window data may come from the old web context instead of the current settings. Include a non-sensitive fingerprint of the web enrichment inputs in the key, or skip caching enriched results.
Useful? React with 👍 / 👎.
Background refresh ticks with a CLI-sourced Claude usage snapshot relaunched the full ~280 MB Claude Code binary every cycle (~288 spawns/day at the 5-minute default), paging in over 100 MB per launch. Cache the last successful CLI fetch result per binary/account-scope/options key and serve it to background app refreshes within a 15-minute floor, matching the existing minimumTokenFetchTTL precedent for expensive local work. User-initiated refreshes, CLI-runtime invocations, and the owner-CLI recovery pass always spawn; a crossed rate-window reset boundary or an account/profile swap invalidates the cached entry, and failures are never cached. Fixes #2916
c0ea74b to
9aff91a
Compare
1 participant
Fixes #2916
What actually spawns, and how often
The issue claims CodexBar spawns the ~272 MB Claude Code CLI on every 5-minute refresh. Verified against the code — this is accurate, but only for a specific configuration slice:
always, or a prior successful foreground CLI fetch establishing the background availability marker — the fix: gate CLI background usage fetch on confirmed-absent OAuth creds #2813 gate). OAuth- and web-sourced users never hit this path.ClaudeStatusProbe.fetch()runs the interactive CLI through a PTY for/usageand again for/status(identity enrichment), so one refresh is typically 2 launches of the ~280 MB Node binary (~/.local/share/claude/versions/…), 3 on a startup-looking retry. At the 5-minute default that's ~576 process launches/day.claude --versionprobes were fixed by the 30-minute fingerprinted cache inProviderVersionDetector(plus fix: refresh Claude CLI version after user-initiated usage fetch #2911's one-shot recovery);claude auth statusonly runs on the CLI-runtime path and Auto planning; the rate-limit gate only bounds the failure lane.On real disk I/O the issue's arithmetic (~34 GB/day) overstates the steady-state cost: repeated reads of the same binary are served from the macOS unified page cache, so most cycles do not hit disk. What is real: the first read after eviction, the constant page-cache pressure of re-faulting a ~280 MB image every few minutes (which evicts other data and causes I/O elsewhere), and the CPU/energy cost of cold-starting Node+V8 ~576×/day. The PR body of the fix is honest about this: the win is process-spawn cost and cache pressure, with true disk-read savings dependent on memory pressure.
The fix
ClaudeCLIUsageSpawnThrottlecaches the last successful CLI fetch result keyed by (binary path, identified account scope, web-extras flag, prepaid-balance flag) and serves it to background app refreshes within a 15-minute floor — the same floor the repo already uses for expensive local work (UsageStore.minimumTokenFetchTTL).The cached result is bypassed/invalidated when it must be:
codexbar usage) always spawn.claudeOwnerCLIRecoveryOnly) always spawns — it exists to get an authoritative read after an OAuth account mismatch.ClaudeAccountProfile.identifiedSessionScope), so stale-account data can never be served; unidentified profiles are never cached.updatedAt, so menu staleness display stays honest.Tradeoff
Users on CLI source with a 1–5-minute refresh interval now get background CLI data at most every 15 minutes (manual refresh unaffected). Rationale: the CLI path is already the lowest-fidelity source (rendered percentages only,
percentOnlyconfidence), the PTY probe itself takes 12–60 s per attempt, and the repo already applies exactly this 15-minute floor to token-cost scans for the same energy reason. Reset boundaries — the moment short-interval users actually care about — bypass the cache. OAuth/web users (the recommended sources) keep their configured cadence entirely.Effect for affected users: ~576 CLI launches/day → ~96/day at default settings (~6× fewer), and proportionally less page-cache churn.
Proof
swift test --filter ClaudeCLIUsageSpawnThrottleTests— 8 new regression tests through the strategy seam (stubbedClaudeStatusProbe.fetchOverride, spawn counting across simulated refresh cycles, clock override for the floor, profile-swap invalidation, reset-boundary invalidation).make test— full sharded suite: 857 selections, 72/72 groups green, 0 retries.make check— SwiftFormat + SwiftLint strict: clean.