Repository navigation
Conversation
…etry-After parsing Sets ureq's http_status_as_error(false) so 401/429/5xx responses reach fetch_usage_limits's status-code branches instead of being converted to Err(Error::StatusCode) beforehand and silently downgraded to NetworkError, which would defeat the token-failure and rate-limit backoff logic that consumes UsageFetchOutcome. Also extracts status-code classification into a pure, unit-testable classify_status() function.
Every cship process (one per Claude Code session, re-run on each render) fetched /api/oauth/usage independently and cached per transcript. With several sessions open this produced redundant calls, 429s, and empty first renders, with no shared backoff. Follow-up to stephenleo#95, which made the interval configurable but left each process fetching on its own. Fetches now go through usage_limits_state::resolve_shared_usage, which keeps one state file per Claude config directory (atomic write, schema-versioned, owner-only on Unix): - Cross-process exclusion uses an OS-level advisory lock (File::try_lock, stable since Rust 1.89, so Cargo.toml now declares rust-version = "1.89"). The OS releases it if the holder crashes, so there is no stale-lock bookkeeping. - Eligibility: not blamed for the current token's last 401, past any rate-limit cooldown, past next_allowed_at. 429 (honouring Retry-After) and 5xx back off exponentially; a 401 blames the token fingerprint so retries stop until the token changes, then exactly one probe is allowed. - Errors never make polling more frequent than ttl: every backoff is floored at the configured interval. - Window resets: like the old cache, which expired the moment a window reset, the next poll is scheduled for 5s after a known reset when that comes before the regular poll, or within 30s after it (a regular poll would only see the old window). Polls are never closer than min(30s, ttl), and a regular poll is never delayed by more than that. - When a fetch starts, the lock file records the token fingerprint and mtime of the attempt. This is an independent throttle: an unwritable or AV/sync-blocked state file cannot cause a fetch on every render. A fresh lock file or a different token does not count as an attempt, so the first render and an account switch are never throttled by it. - The old worker-thread + 2s bounded wait is kept (fetch_with_render_timeout, HTTP timeout stays at 1500ms), so a slow API cannot stall a render. Fetch failures are logged at warn. - Credentials are only read, and the API only called, when the input has a transcript_path, so non-session input never touches a real credential. ttl defaults to 60s and has no floor. The coordinator dedupes fetches across sessions, so ttl only sets overall polling frequency. ttl = 0 would disable throttling, so it is clamped to 1s (with a warning) instead of being a fatal config error. Docs updated: README, configuration, faq (new state file location, backoff behaviour), showcase. Two display bugs are fixed on the same path: - A window that just reset reports utilization 0 and resets_at null; this was indistinguishable from a plan with no such window (Enterprise). five_hour_available / seven_day_available now record presence, and lacks_standard_signal checks presence rather than value. - A stale percentage was paired with a newer reset time (or vice versa) when merging sources, yielding "resets in now". UsageWindowSnapshot / merge_window keep each pct and reset timestamp together, in all source arms. Removes the per-transcript usage cache from cache.rs and its tests, and points `cship explain`'s usage_limits hint at the shared state, checking the token fingerprint and with the hint logic unit-tested directly. Adds platform::claude_config_dir and cship_shared_cache_dir. Cache schema is v3 so older entries are not misread. The coordinator is tested through resolve_shared_usage_in, which takes the cache directory and fetch function as parameters (the public function is a thin wrapper), against a temp dir and a fake fetch. Tests were checked by mutating the function and confirming they fail. A test-only seam in the usage_limits module covers the stdin + API data assembly. Not addressed: usage_limits still reports OAuth quota in sessions that use another provider (stephenleo#199); the coordinator is per Claude config directory, not provider-aware. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Owner
|
@Romboter thanks for the PR! Here are the findings from a code review. #1–#3 are the most important. Correctness
Cleanup
🤖 Generated with Claude Code |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #95. With several Claude Code sessions open, each
cshipprocess polled/api/oauth/usageon its own per-transcript schedule, which gets the account rate limited (HTTP 429). A 429 was not remembered, so the next render tried again. This change makes everycshipprocess that uses the same Claude config directory share one fetch schedule.On each render, cship reads the shared state. If the data is fresh enough, it renders that. Otherwise it tries the lock. The process that gets the lock fetches on a worker thread (the request times out at 1.5 s, and the render waits at most 2 s) and writes the result back. Processes that do not get the lock render the last saved data.
Commit 1 changes how a fetch reports its result:
fetch_usage_limitsreturns aUsageFetchOutcome(success, 429 withRetry-After, 401, 5xx, network error, invalid response) instead of a bareResult.tracing::warn!.User-Agent: cship/<version>. I checked the endpoint withclaude-code/<version>,cship/<version>and no header at all, and it answered the same each time, so I used our own name.Commit 2 adds the shared schedule, in
src/usage_limits_state.rs:<CLAUDE_CONFIG_DIR or ~/.claude>/cship/cache/, readable only by the owner on Unix. An OS advisory lock lets one process fetch while the others render the last shared data.ttl. A 429 waits forRetry-After, up to 15 minutes. A 5xx waits 30 s, then doubles with each repeat up to 5 minutes. A network error waits 30 s, orttlif that is longer. A 401 stops fetching until the token changes.ttl, if that is shorter), and it never delays a regular poll by more than 30 s.ttldefaults to 60 s as before.ttl = 0still loads and is raised to 1 s with a warning.Cargo.tomlgetsrust-version = "1.89", becauseFile::try_lockneeds it.cship explainreads the shared state. The FAQ usage section and thettlrow indocs/configuration.mdare updated.No config fields are added or removed, the default
ttlis unchanged, and there are no new dependencies. Output is the same as before when nothing is failing.Two behaviours differ from main:
Not covered:
test_per_period_tokens_none_without_standard_signalalready fails intermittently onmainon a machine with a real Claude login, because it calls the live API. It passes reliably on the final commit, but can flake if you test commit 1 on its own.The coordinator takes its directory and its fetch function as arguments, so 13 tests call the real function with a temporary directory and a fake fetch. I checked that they fail when the function is broken by deleting lines from it. The scheduling, backoff and data-merge paths also have unit tests. To try it by hand, log out of Claude Code and back in. After a 401, fetching resumes once the token changes.
Verification
cargo fmt --check✅cargo clippy --all-targets -- -D warnings✅cargo test✅ (613 unit + 77 integration, run on Linux)cargo build --release✅🤖 Generated with Claude Code