feat: support claude setup-token for quota monitoring#129
Merged
hanrw merged 1 commit intotddworks:mainfrom Feb 25, 2026
Merged
feat: support claude setup-token for quota monitoring#129hanrw merged 1 commit intotddworks:mainfrom
claude setup-token for quota monitoring#129hanrw merged 1 commit intotddworks:mainfrom
Conversation
Add support for users who authenticate via `claude setup-token`, which produces a long-lived inference-only OAuth token (CLAUDE_CODE_OAUTH_TOKEN). The setup-token only has `user:inference` scope and lacks the `user:profile` scope required by both the API endpoint and CLI `/usage` command. This means neither probe can fetch quota data using the setup-token alone — stored credentials from `claude login` are needed. Changes: - ClaudeCredentialLoader: Add `.environment` credential source to load tokens from CLAUDE_CODE_OAUTH_TOKEN env var. Reorder credential priority to file → keychain → env so full-scope stored credentials are preferred over the inference-only setup-token. - ClaudeAPIUsageProbe: Skip token refresh when refreshToken is nil (setup-tokens have no refresh token). On 401/403 retry, re-throw immediately if no refresh token available instead of attempting refresh. - ClaudeUsageProbe (CLI): Strip CLAUDE_CODE_OAUTH_TOKEN from the subprocess environment when running `claude /usage`, so the CLI falls back to stored credentials with full scope. - InteractiveRunner: Add environmentExclusions to Options, allowing callers to remove specific env vars from subprocess environment. - DefaultCLIExecutor: Accept environmentExclusions and forward them to InteractiveRunner. Includes 16 new tests covering all changes with zero regressions.
Contributor
📝 WalkthroughWalkthroughThis PR adds support for setup tokens (environment-based credentials) in Claude API credential handling and introduces environment variable exclusion capabilities for CLI subprocess execution. Changes include conditional token refresh logic that only attempts refresh when a refresh_token exists, new environment variable loading in ClaudeCredentialLoader, and propagation of environment exclusion settings through the CLI execution layer. Comprehensive test coverage validates setup token behavior and environment exclusion functionality. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
Sources/Infrastructure/Claude/ClaudeCredentialLoader.swift (2)
149-162: Add at least a debug log inloadFromEnvironment()for consistency and diagnostics.
loadFromFile()andloadFromKeychain()both emitAppLog.credentials.error(...)on failure paths.loadFromEnvironment()has zero logging — no debug on success, no path tracing on miss. As per coding guidelines,AppLog.debug()is appropriate for development diagnostics.♻️ Proposed addition
private func loadFromEnvironment() -> ClaudeCredentialResult? { guard let token = environment["CLAUDE_CODE_OAUTH_TOKEN"], !token.isEmpty else { + AppLog.credentials.debug("No CLAUDE_CODE_OAUTH_TOKEN env var found") return nil } let oauth = ClaudeOAuthCredentials( accessToken: token, refreshToken: nil, expiresAt: nil ) + AppLog.credentials.debug("Loaded setup-token credentials from environment (inference-only scope)") return ClaudeCredentialResult(oauth: oauth, source: .environment, fullData: [:]) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Infrastructure/Claude/ClaudeCredentialLoader.swift` around lines 149 - 162, In ClaudeCredentialLoader.loadFromEnvironment(), add AppLog.debug calls to trace both miss and success paths: when environment["CLAUDE_CODE_OAUTH_TOKEN"] is nil or empty emit an AppLog.debug indicating the environment source was checked and no token found; when creating ClaudeOAuthCredentials and returning ClaudeCredentialResult emit an AppLog.debug indicating credentials were loaded from environment (do not log the raw token value). Use the existing symbols ClaudeCredentialLoader.loadFromEnvironment(), ClaudeOAuthCredentials and ClaudeCredentialResult and mirror the logging style used by loadFromFile()/loadFromKeychain().
113-144: Remove the redundant.environmentcase from thesaveCredentialsswitch.The early return at Line 116-118 already handles the environment case before the switch, making Line 138-139 unreachable. The compiler won't warn because the switch must be exhaustive, but the comment "Already handled above, but satisfy exhaustive switch" signals the design tension. A cleaner approach is to remove the guard and rely solely on the exhaustive switch:
♻️ Proposed refactor
public func saveCredentials(_ result: ClaudeCredentialResult) { - // Environment credentials are read-only (set via env var, not persisted by us) - if result.source == .environment { - return - } - var updatedData = result.fullData // Update the OAuth section var oauthDict: [String: Any] = [ "accessToken": result.oauth.accessToken ] ... updatedData["claudeAiOauth"] = oauthDict switch result.source { case .environment: - return // Already handled above, but satisfy exhaustive switch + return // Environment credentials are read-only (set via env var, not persisted by us) case .file: saveToFile(updatedData) case .keychain: saveToKeychain(updatedData) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Infrastructure/Claude/ClaudeCredentialLoader.swift` around lines 113 - 144, In saveCredentials(ClaudeCredentialResult) remove the early environment guard (the if result.source == .environment return block) and instead handle the read-only environment case inside the switch by keeping a case .environment that returns there; this makes the switch the single exhaustive control-flow point, so delete the redundant comment and the early-return lines and leave the switch with case .environment { return } plus case .file { saveToFile(updatedData) } and case .keychain { saveToKeychain(updatedData) } to fix control flow and maintain exhaustiveness.Tests/InfrastructureTests/Claude/ClaudeAPIUsageProbeTests.swift (1)
492-497: Three near-identicalmakeTemporaryDirectory()helpers across test suites — consider extracting to a shared utility.
ClaudeAPIUsageProbeTests,ClaudeAPIUsageProbeTokenRefreshTests, andClaudeAPIUsageProbeSetupTokenTestseach define their ownmakeTemporaryDirectory()with only the directory-name prefix differing. Extracting to a free function or a sharedTestHelpersstruct would remove the duplication.Also applies to: 624-629
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/InfrastructureTests/Claude/ClaudeAPIUsageProbeTests.swift` around lines 492 - 497, Three test files duplicate makeTemporaryDirectory() helpers; extract a shared helper to remove duplication. Create a single free function or a TestHelpers struct (e.g., TestHelpers.makeTemporaryDirectory(prefix: String) or makeTemporaryDirectory(prefix:)) and replace the per-test methods in ClaudeAPIUsageProbeTests, ClaudeAPIUsageProbeTokenRefreshTests, and ClaudeAPIUsageProbeSetupTokenTests to call the shared helper, passing the differing prefix strings; ensure the new helper uses FileManager.default.temporaryDirectory and createDirectory(at:withIntermediateDirectories:) and preserves the same throwing behavior as the original makeTemporaryDirectory().Tests/InfrastructureTests/Claude/ClaudeUsageProbeTests.swift (1)
208-222: Two tests cover the same assertion; consider consolidating or strengthening coverage.Both
envExclusions includes CLAUDE_CODE_OAUTH_TOKENanddefault init creates executor that excludes setup token env varassertClaudeUsageProbe.envExclusions == ["CLAUDE_CODE_OAUTH_TOKEN"]. Neither verifies that the exclusion is actually forwarded to the underlyingDefaultCLIExecutor. The second test's own comment acknowledges this is indirect. Consider either merging them into one, or replacing the second with an integration-style test that verifies subprocess exclusion behavior (similar to theInteractiveRunnerTestsapproach).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/InfrastructureTests/Claude/ClaudeUsageProbeTests.swift` around lines 208 - 222, Two tests duplicate the same assertion about ClaudeUsageProbe.envExclusions; consolidate and strengthen by keeping a single unit test that asserts ClaudeUsageProbe.envExclusions == ["CLAUDE_CODE_OAUTH_TOKEN"] and replacing the second test with an integration-style test that constructs a ClaudeUsageProbe (using its default init) and verifies the exclusion is passed to the executor by either injecting or observing the DefaultCLIExecutor behavior (e.g., create a spy/mocked CLI executor or run a subprocess like InteractiveRunnerTests do) to confirm CLAUDE_CODE_OAUTH_TOKEN is omitted from the child process env; update or remove the redundant test and add the integration test referencing ClaudeUsageProbe, envExclusions, and DefaultCLIExecutor (or a CLI executor test double) so the exclusion is actually exercised.Sources/Infrastructure/Shared/InteractiveRunner.swift (1)
393-406: Excluding hardcoded-default keys silently has no effect.Excluded keys are removed from
envon Lines 396-398, but the defaults block that follows unconditionally re-introducesPATH(Line 399, always overwritten) and re-appliesHOME,TERM,COLORTERM,LANG, andCIvia??(Lines 400-404). Because the key was just deleted,env["HOME"]isnil, so?? NSHomeDirectory()fires and the key is restored. Any key in that set can never actually be excluded, and the omission is silent.For the current use case (
CLAUDE_CODE_OAUTH_TOKEN), this is harmless because that key is not in the defaults list. But it could surprise a future maintainer who tries to exclude, say,CI.Consider applying exclusions after the defaults block, or at least adding a comment warning that hardcoded default keys cannot be excluded:
♻️ Proposed fix — apply exclusions last
private static func terminalEnvironment(excluding: [String] = []) -> [String: String] { var env = ProcessInfo.processInfo.environment - // Remove excluded keys before setting defaults - for key in excluding { - env.removeValue(forKey: key) - } env["PATH"] = ensureCommonPathsIncluded(BinaryLocator.shellPath()) env["HOME"] = env["HOME"] ?? NSHomeDirectory() env["TERM"] = env["TERM"] ?? "xterm-256color" env["COLORTERM"] = env["COLORTERM"] ?? "truecolor" env["LANG"] = env["LANG"] ?? "en_US.UTF-8" env["CI"] = env["CI"] ?? "0" + // Remove excluded keys after defaults so callers can strip arbitrary inherited vars. + // Note: PATH is always overwritten above and cannot be excluded. + for key in excluding { + env.removeValue(forKey: key) + } return env }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Infrastructure/Shared/InteractiveRunner.swift` around lines 393 - 406, The exclusion list is applied before defaults, so keys like "PATH", "HOME", "TERM", "COLORTERM", "LANG", and "CI" always get re-added; update terminalEnvironment(excluding:) to apply the defaults first (using BinaryLocator.shellPath() for PATH and NSHomeDirectory()/fallbacks for the others) and then remove any keys present in excluding, or alternatively guard each default assignment with a check that the key is not in excluding; ensure you reference terminalEnvironment(excluding:), BinaryLocator.shellPath(), and the default keys ("PATH","HOME","TERM","COLORTERM","LANG","CI") when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/Infrastructure/Claude/ClaudeAPIUsageProbe.swift`:
- Around line 85-89: The log in the else branch of ClaudeAPIUsageProbe (where
needsRefresh returned true but we found no refreshToken) can be misleading;
update the message to reflect two cases: if credential.expiresAt is nil log that
there is no expiry info and we're proceeding, otherwise log that the token is
expiring soon (include credential.expiresAt) but there's no refreshToken so
we'll proceed and expect a 401. Modify the AppLog.probes.info call in
ClaudeAPIUsageProbe.swift (the branch handling "no refresh mechanism") to
inspect credential.expiresAt and emit the appropriate, precise message.
In `@Tests/InfrastructureTests/Claude/ClaudeAPIUsageProbeTests.swift`:
- Around line 695-701: Replace the broad assertion that any ProbeError is thrown
with an assertion that specifically expects ProbeError.authenticationRequired:
in the test that constructs ClaudeAPIUsageProbe (symbols: ClaudeAPIUsageProbe,
probe.probe(), loader, mockNetwork), change the await `#expect`(throws:
ProbeError.self) call to assert the concrete case
ProbeError.authenticationRequired so the test verifies the exact error variant
(authenticationRequired) is produced when no refresh token is available.
- Around line 619-720: The ClaudeAPIUsageProbeSetupTokenTests `@Suite` is
accidentally nested inside ClaudeAPIUsageProbeTokenRefreshTests; to fix, close
the ClaudeAPIUsageProbeTokenRefreshTests scope before declaring
ClaudeAPIUsageProbeSetupTokenTests (ensure the closing brace for
ClaudeAPIUsageProbeTokenRefreshTests is placed immediately before the start of
the ClaudeAPIUsageProbeSetupTokenTests struct), verify both structs
(ClaudeAPIUsageProbeTokenRefreshTests and ClaudeAPIUsageProbeSetupTokenTests)
have their own matching opening/closing braces and re-run tests to confirm the
suites are independent.
In `@Tests/InfrastructureTests/Claude/ClaudeCredentialLoaderTests.swift`:
- Around line 342-362: Rename the test function to reflect the actual assertion
(it expects needsRefresh == true): change the test function declaration from
`func \`needsRefresh returns false for environment source credentials with no
expiresAt\`() throws` to `func \`needsRefresh returns true for environment
source credentials with no expiresAt\`() throws`, keeping the body that
constructs ClaudeOAuthCredentials(accessToken: "setup-token", refreshToken: nil,
expiresAt: nil) and the ClaudeCredentialLoader usage and the
`#expect`(loader.needsRefresh(oauth) == true) assertion; ensure any related
comment text remains consistent with the new name and references to
ClaudeOAuthCredentials, ClaudeCredentialLoader, and needsRefresh are correct.
In `@Tests/InfrastructureTests/Shared/InteractiveRunnerTests.swift`:
- Around line 44-77: The two tests `run with environmentExclusions strips env
vars from subprocess` and `run without environmentExclusions preserves env vars
in subprocess` mutate process-global environment via setenv/unsetenv and should
be run serially; update each test declaration (or the containing test class) to
use serialized execution (e.g., annotate the tests with `@Test`(.serialized) or
move them into a serialized test suite) so the calls in these test methods and
interactions with InteractiveRunner do not race with other concurrent tests.
---
Nitpick comments:
In `@Sources/Infrastructure/Claude/ClaudeCredentialLoader.swift`:
- Around line 149-162: In ClaudeCredentialLoader.loadFromEnvironment(), add
AppLog.debug calls to trace both miss and success paths: when
environment["CLAUDE_CODE_OAUTH_TOKEN"] is nil or empty emit an AppLog.debug
indicating the environment source was checked and no token found; when creating
ClaudeOAuthCredentials and returning ClaudeCredentialResult emit an AppLog.debug
indicating credentials were loaded from environment (do not log the raw token
value). Use the existing symbols ClaudeCredentialLoader.loadFromEnvironment(),
ClaudeOAuthCredentials and ClaudeCredentialResult and mirror the logging style
used by loadFromFile()/loadFromKeychain().
- Around line 113-144: In saveCredentials(ClaudeCredentialResult) remove the
early environment guard (the if result.source == .environment return block) and
instead handle the read-only environment case inside the switch by keeping a
case .environment that returns there; this makes the switch the single
exhaustive control-flow point, so delete the redundant comment and the
early-return lines and leave the switch with case .environment { return } plus
case .file { saveToFile(updatedData) } and case .keychain {
saveToKeychain(updatedData) } to fix control flow and maintain exhaustiveness.
In `@Sources/Infrastructure/Shared/InteractiveRunner.swift`:
- Around line 393-406: The exclusion list is applied before defaults, so keys
like "PATH", "HOME", "TERM", "COLORTERM", "LANG", and "CI" always get re-added;
update terminalEnvironment(excluding:) to apply the defaults first (using
BinaryLocator.shellPath() for PATH and NSHomeDirectory()/fallbacks for the
others) and then remove any keys present in excluding, or alternatively guard
each default assignment with a check that the key is not in excluding; ensure
you reference terminalEnvironment(excluding:), BinaryLocator.shellPath(), and
the default keys ("PATH","HOME","TERM","COLORTERM","LANG","CI") when making the
change.
In `@Tests/InfrastructureTests/Claude/ClaudeAPIUsageProbeTests.swift`:
- Around line 492-497: Three test files duplicate makeTemporaryDirectory()
helpers; extract a shared helper to remove duplication. Create a single free
function or a TestHelpers struct (e.g.,
TestHelpers.makeTemporaryDirectory(prefix: String) or
makeTemporaryDirectory(prefix:)) and replace the per-test methods in
ClaudeAPIUsageProbeTests, ClaudeAPIUsageProbeTokenRefreshTests, and
ClaudeAPIUsageProbeSetupTokenTests to call the shared helper, passing the
differing prefix strings; ensure the new helper uses
FileManager.default.temporaryDirectory and
createDirectory(at:withIntermediateDirectories:) and preserves the same throwing
behavior as the original makeTemporaryDirectory().
In `@Tests/InfrastructureTests/Claude/ClaudeUsageProbeTests.swift`:
- Around line 208-222: Two tests duplicate the same assertion about
ClaudeUsageProbe.envExclusions; consolidate and strengthen by keeping a single
unit test that asserts ClaudeUsageProbe.envExclusions ==
["CLAUDE_CODE_OAUTH_TOKEN"] and replacing the second test with an
integration-style test that constructs a ClaudeUsageProbe (using its default
init) and verifies the exclusion is passed to the executor by either injecting
or observing the DefaultCLIExecutor behavior (e.g., create a spy/mocked CLI
executor or run a subprocess like InteractiveRunnerTests do) to confirm
CLAUDE_CODE_OAUTH_TOKEN is omitted from the child process env; update or remove
the redundant test and add the integration test referencing ClaudeUsageProbe,
envExclusions, and DefaultCLIExecutor (or a CLI executor test double) so the
exclusion is actually exercised.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Sources/Infrastructure/Claude/ClaudeAPIUsageProbe.swiftSources/Infrastructure/Claude/ClaudeCredentialLoader.swiftSources/Infrastructure/Claude/ClaudeUsageProbe.swiftSources/Infrastructure/Shared/DefaultCLIExecutor.swiftSources/Infrastructure/Shared/InteractiveRunner.swiftTests/InfrastructureTests/Claude/ClaudeAPIUsageProbeTests.swiftTests/InfrastructureTests/Claude/ClaudeCredentialLoaderTests.swiftTests/InfrastructureTests/Claude/ClaudeUsageProbeTests.swiftTests/InfrastructureTests/Shared/InteractiveRunnerTests.swift
Comment on lines
+85
to
89
| } else { | ||
| // Long-lived token (e.g. from `claude setup-token`) — no refresh mechanism. | ||
| // Proceed directly with the token; the API call will fail with 401 if it's actually expired. | ||
| AppLog.probes.info("Claude API: Token has no expiry info and no refresh token (setup-token), proceeding...") | ||
| } |
Contributor
There was a problem hiding this comment.
Log message is slightly inaccurate when token has expiry info but is expiring soon.
needsRefresh returns true in two cases: (a) expiresAt is nil, or (b) token is within the 5-minute refresh buffer. If a credential somehow reaches this branch via case (b) (has an expiresAt but no refreshToken), the message "no expiry info" is misleading.
💬 Proposed fix
- AppLog.probes.info("Claude API: Token has no expiry info and no refresh token (setup-token), proceeding...")
+ AppLog.probes.info("Claude API: Token needs refresh but has no refresh token (setup-token), proceeding with current token...")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Infrastructure/Claude/ClaudeAPIUsageProbe.swift` around lines 85 -
89, The log in the else branch of ClaudeAPIUsageProbe (where needsRefresh
returned true but we found no refreshToken) can be misleading; update the
message to reflect two cases: if credential.expiresAt is nil log that there is
no expiry info and we're proceeding, otherwise log that the token is expiring
soon (include credential.expiresAt) but there's no refreshToken so we'll proceed
and expect a 401. Modify the AppLog.probes.info call in
ClaudeAPIUsageProbe.swift (the branch handling "no refresh mechanism") to
inspect credential.expiresAt and emit the appropriate, precise message.
Comment on lines
+619
to
720
| // MARK: - Setup-Token (Environment) Tests | ||
|
|
||
| @Suite("ClaudeAPIUsageProbe Setup-Token Tests") | ||
| struct ClaudeAPIUsageProbeSetupTokenTests { | ||
|
|
||
| private func makeTemporaryDirectory() throws -> URL { | ||
| let tempDir = FileManager.default.temporaryDirectory | ||
| .appendingPathComponent("claude-api-probe-setup-token-tests-\(UUID().uuidString)", isDirectory: true) | ||
| try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) | ||
| return tempDir | ||
| } | ||
|
|
||
| @Test | ||
| func `probe skips refresh when no refresh token and fetches successfully`() async throws { | ||
| let tempDir = try makeTemporaryDirectory() | ||
| defer { try? FileManager.default.removeItem(at: tempDir) } | ||
|
|
||
| // Simulate setup-token: loaded from env var, no refresh token, no expiresAt | ||
| let loader = ClaudeCredentialLoader( | ||
| homeDirectory: tempDir.path, | ||
| useKeychain: false, | ||
| environment: ["CLAUDE_CODE_OAUTH_TOKEN": "setup-token-abc123"] | ||
| ) | ||
|
|
||
| let mockNetwork = MockNetworkClient() | ||
| let usageResponse = """ | ||
| { | ||
| "five_hour": { "utilization": 20.0, "resets_at": "2025-01-15T10:00:00Z" }, | ||
| "seven_day": { "utilization": 40.0, "resets_at": "2025-01-20T00:00:00Z" } | ||
| } | ||
| """.data(using: .utf8)! | ||
|
|
||
| let usageHTTP = HTTPURLResponse( | ||
| url: URL(string: "https://api.anthropic.com")!, | ||
| statusCode: 200, | ||
| httpVersion: nil, | ||
| headerFields: nil | ||
| )! | ||
|
|
||
| // Only the usage call should be made — NO refresh call | ||
| given(mockNetwork).request(.any).willReturn((usageResponse, usageHTTP)) | ||
|
|
||
| let probe = ClaudeAPIUsageProbe(credentialLoader: loader, networkClient: mockNetwork) | ||
|
|
||
| let snapshot = try await probe.probe() | ||
|
|
||
| #expect(snapshot.providerId == "claude") | ||
| #expect(snapshot.quotas.count == 2) | ||
|
|
||
| let sessionQuota = snapshot.quotas.first { $0.quotaType == .session } | ||
| #expect(sessionQuota?.percentRemaining == 80.0) // 100 - 20 | ||
|
|
||
| let weeklyQuota = snapshot.quotas.first { $0.quotaType == .weekly } | ||
| #expect(weeklyQuota?.percentRemaining == 60.0) // 100 - 40 | ||
| } | ||
|
|
||
| @Test | ||
| func `probe with setup-token throws authenticationRequired on 401 without attempting refresh`() async throws { | ||
| let tempDir = try makeTemporaryDirectory() | ||
| defer { try? FileManager.default.removeItem(at: tempDir) } | ||
|
|
||
| let loader = ClaudeCredentialLoader( | ||
| homeDirectory: tempDir.path, | ||
| useKeychain: false, | ||
| environment: ["CLAUDE_CODE_OAUTH_TOKEN": "expired-setup-token"] | ||
| ) | ||
|
|
||
| let mockNetwork = MockNetworkClient() | ||
| let unauthorizedHTTP = HTTPURLResponse( | ||
| url: URL(string: "https://api.anthropic.com")!, | ||
| statusCode: 401, | ||
| httpVersion: nil, | ||
| headerFields: nil | ||
| )! | ||
|
|
||
| given(mockNetwork).request(.any).willReturn((Data(), unauthorizedHTTP)) | ||
|
|
||
| let probe = ClaudeAPIUsageProbe(credentialLoader: loader, networkClient: mockNetwork) | ||
|
|
||
| // Should throw without attempting refresh (no refresh token available) | ||
| await #expect(throws: ProbeError.self) { | ||
| try await probe.probe() | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| func `isAvailable returns true when env var token is set`() async throws { | ||
| let tempDir = try makeTemporaryDirectory() | ||
| defer { try? FileManager.default.removeItem(at: tempDir) } | ||
|
|
||
| let loader = ClaudeCredentialLoader( | ||
| homeDirectory: tempDir.path, | ||
| useKeychain: false, | ||
| environment: ["CLAUDE_CODE_OAUTH_TOKEN": "my-setup-token"] | ||
| ) | ||
|
|
||
| let probe = ClaudeAPIUsageProbe(credentialLoader: loader) | ||
|
|
||
| #expect(await probe.isAvailable() == true) | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
ClaudeAPIUsageProbeSetupTokenTests is accidentally nested inside ClaudeAPIUsageProbeTokenRefreshTests.
The new @Suite struct is declared at Line 621 but the closing } for ClaudeAPIUsageProbeTokenRefreshTests is at Line 720, after the new struct's closing } at Line 719. This makes the setup-token suite a child of the refresh-tests suite in the Swift Testing hierarchy, which is unintended — these are semantically independent suites.
Move the new @Suite struct outside and after the } on Line 720:
📐 Proposed structural fix
// ... last test in ClaudeAPIUsageProbeTokenRefreshTests ...
}
} // closes ClaudeAPIUsageProbeTokenRefreshTests
+
+// MARK: - Setup-Token (Environment) Tests
+
+@Suite("ClaudeAPIUsageProbe Setup-Token Tests")
+struct ClaudeAPIUsageProbeSetupTokenTests {
+ // ... setup-token tests ...
+}
-// MARK: - Setup-Token (Environment) Tests
-
-@Suite("ClaudeAPIUsageProbe Setup-Token Tests")
-struct ClaudeAPIUsageProbeSetupTokenTests {
- // ... setup-token tests ...
-}
-} // was closing ClaudeAPIUsageProbeTokenRefreshTests🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Tests/InfrastructureTests/Claude/ClaudeAPIUsageProbeTests.swift` around lines
619 - 720, The ClaudeAPIUsageProbeSetupTokenTests `@Suite` is accidentally nested
inside ClaudeAPIUsageProbeTokenRefreshTests; to fix, close the
ClaudeAPIUsageProbeTokenRefreshTests scope before declaring
ClaudeAPIUsageProbeSetupTokenTests (ensure the closing brace for
ClaudeAPIUsageProbeTokenRefreshTests is placed immediately before the start of
the ClaudeAPIUsageProbeSetupTokenTests struct), verify both structs
(ClaudeAPIUsageProbeTokenRefreshTests and ClaudeAPIUsageProbeSetupTokenTests)
have their own matching opening/closing braces and re-run tests to confirm the
suites are independent.
Comment on lines
+695
to
+701
|
|
||
| let probe = ClaudeAPIUsageProbe(credentialLoader: loader, networkClient: mockNetwork) | ||
|
|
||
| // Should throw without attempting refresh (no refresh token available) | ||
| await #expect(throws: ProbeError.self) { | ||
| try await probe.probe() | ||
| } |
Contributor
There was a problem hiding this comment.
Strengthen the error-type assertion in the 401 setup-token test.
await #expect(throws: ProbeError.self) accepts any ProbeError, while the test intent (per comment: "Should throw without attempting refresh") is specifically ProbeError.authenticationRequired. Using the concrete case makes the test a meaningful regression guard.
✅ Proposed fix
- // Should throw without attempting refresh (no refresh token available)
- await `#expect`(throws: ProbeError.self) {
+ // Should throw authenticationRequired without attempting refresh (no refresh token available)
+ await `#expect`(throws: ProbeError.authenticationRequired) {
try await probe.probe()
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Tests/InfrastructureTests/Claude/ClaudeAPIUsageProbeTests.swift` around lines
695 - 701, Replace the broad assertion that any ProbeError is thrown with an
assertion that specifically expects ProbeError.authenticationRequired: in the
test that constructs ClaudeAPIUsageProbe (symbols: ClaudeAPIUsageProbe,
probe.probe(), loader, mockNetwork), change the await `#expect`(throws:
ProbeError.self) call to assert the concrete case
ProbeError.authenticationRequired so the test verifies the exact error variant
(authenticationRequired) is produced when no refresh token is available.
Comment on lines
+342
to
+362
| @Test | ||
| func `needsRefresh returns false for environment source credentials with no expiresAt`() throws { | ||
| // Setup-token credentials have no expiresAt, but should NOT be treated as "needs refresh" | ||
| // because they are long-lived tokens with no refresh mechanism | ||
| let oauth = ClaudeOAuthCredentials( | ||
| accessToken: "setup-token", | ||
| refreshToken: nil, | ||
| expiresAt: nil | ||
| ) | ||
|
|
||
| let loader = ClaudeCredentialLoader( | ||
| homeDirectory: NSTemporaryDirectory(), | ||
| useKeychain: false | ||
| ) | ||
|
|
||
| // Currently needsRefresh returns true when expiresAt is nil. | ||
| // For setup-token (no refresh token), the probe should handle this | ||
| // by skipping refresh when refreshToken is nil. | ||
| // So needsRefresh itself stays true, but the probe won't attempt refresh. | ||
| #expect(loader.needsRefresh(oauth) == true) | ||
| } |
Contributor
There was a problem hiding this comment.
Test name contradicts the assertion — fix the name.
The test is named needsRefresh returns false for environment source credentials with no expiresAt but the assertion on Line 361 is #expect(loader.needsRefresh(oauth) == true). The name and the assertion say opposite things.
The comment block (Lines 357-360) correctly documents the intent: needsRefresh currently returns true for nil expiresAt, and the probe is responsible for not acting on it when refreshToken is nil. Rename the test to match the actual assertion:
✅ Proposed fix
- func `needsRefresh returns false for environment source credentials with no expiresAt`() throws {
+ func `needsRefresh returns true for setup-token credentials with no expiresAt (probe skips refresh when refreshToken is nil)`() throws {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Tests/InfrastructureTests/Claude/ClaudeCredentialLoaderTests.swift` around
lines 342 - 362, Rename the test function to reflect the actual assertion (it
expects needsRefresh == true): change the test function declaration from `func
\`needsRefresh returns false for environment source credentials with no
expiresAt\`() throws` to `func \`needsRefresh returns true for environment
source credentials with no expiresAt\`() throws`, keeping the body that
constructs ClaudeOAuthCredentials(accessToken: "setup-token", refreshToken: nil,
expiresAt: nil) and the ClaudeCredentialLoader usage and the
`#expect`(loader.needsRefresh(oauth) == true) assertion; ensure any related
comment text remains consistent with the new name and references to
ClaudeOAuthCredentials, ClaudeCredentialLoader, and needsRefresh are correct.
Comment on lines
+44
to
+77
| @Test | ||
| func `run with environmentExclusions strips env vars from subprocess`() throws { | ||
| let runner = InteractiveRunner() | ||
| // Set a test env var that we'll verify is excluded | ||
| let testKey = "CLAUDEBAR_TEST_EXCLUSION_VAR" | ||
| setenv(testKey, "should_be_stripped", 1) | ||
| defer { unsetenv(testKey) } | ||
|
|
||
| // Run env command with the exclusion — the var should NOT appear in output | ||
| let result = try runner.run( | ||
| binary: "/usr/bin/env", | ||
| input: "", | ||
| options: .init(environmentExclusions: [testKey]) | ||
| ) | ||
|
|
||
| #expect(!result.output.contains("CLAUDEBAR_TEST_EXCLUSION_VAR=should_be_stripped")) | ||
| } | ||
|
|
||
| @Test | ||
| func `run without environmentExclusions preserves env vars in subprocess`() throws { | ||
| let runner = InteractiveRunner() | ||
| let testKey = "CLAUDEBAR_TEST_PRESERVE_VAR" | ||
| setenv(testKey, "should_be_present", 1) | ||
| defer { unsetenv(testKey) } | ||
|
|
||
| // Run env command without exclusion — the var SHOULD appear in output | ||
| let result = try runner.run( | ||
| binary: "/usr/bin/env", | ||
| input: "", | ||
| options: .init() | ||
| ) | ||
|
|
||
| #expect(result.output.contains("CLAUDEBAR_TEST_PRESERVE_VAR=should_be_present")) | ||
| } |
Contributor
There was a problem hiding this comment.
setenv/unsetenv in concurrent tests may cause rare races.
Swift Testing runs tests concurrently by default. setenv/unsetenv mutate the process-global environment, which is not thread-safe when other tests concurrently read ProcessInfo.processInfo.environment or enumerate it. The unique variable names (CLAUDEBAR_TEST_EXCLUSION_VAR, CLAUDEBAR_TEST_PRESERVE_VAR) minimize the likelihood of a false result, but a concurrent reader could observe a partially-set environment.
Consider annotating these two tests with @Test(.serialized) or isolating them in a dedicated serial suite to be safe.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Tests/InfrastructureTests/Shared/InteractiveRunnerTests.swift` around lines
44 - 77, The two tests `run with environmentExclusions strips env vars from
subprocess` and `run without environmentExclusions preserves env vars in
subprocess` mutate process-global environment via setenv/unsetenv and should be
run serially; update each test declaration (or the containing test class) to use
serialized execution (e.g., annotate the tests with `@Test`(.serialized) or move
them into a serialized test suite) so the calls in these test methods and
interactions with InteractiveRunner do not race with other concurrent tests.
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (77.97%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #129 +/- ##
==========================================
- Coverage 78.96% 77.97% -0.99%
==========================================
Files 61 78 +17
Lines 4715 6048 +1333
==========================================
+ Hits 3723 4716 +993
- Misses 992 1332 +340
🚀 New features to boost your workflow:
|
Collaborator
Author
|
Reopening this so I can add a couple fixed to here |
5 tasks
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
2 participants
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.
Summary
Adds support for users who authenticate via
claude setup-token, which produces a long-lived, inference-only OAuth token exported asCLAUDE_CODE_OAUTH_TOKEN. This is a common setup for CI/automated environments and developers who prefer token-based auth.Problem
The
claude setup-tokencommand produces a token with only theuser:inferencescope. However, both ClaudeBar's API probe (/api/oauth/usage) and CLI probe (claude /usage) require theuser:profilescope to fetch quota data. WhenCLAUDE_CODE_OAUTH_TOKENis set in the environment:permission_error(missinguser:profilescope), then enters a retry/refresh loop that never succeeds because setup-tokens have no refresh token.claude /usagesubprocess inheritsCLAUDE_CODE_OAUTH_TOKENfrom the parent process, causing the Claude CLI to use the inference-only token instead of stored credentials fromclaude login.Net effect: ClaudeBar fails to display quota data for any user with
CLAUDE_CODE_OAUTH_TOKENexported, even if they also have full-scope credentials fromclaude login.Solution
Three complementary fixes that work together:
1. Credential priority reordering (ClaudeCredentialLoader)
.environmentcredential source to load tokens fromCLAUDE_CODE_OAUTH_TOKENclaude loginare now preferred over the inference-only setup-tokensaveCredentials(env tokens are read-only)2. Graceful handling of missing refresh token (ClaudeAPIUsageProbe)
needsRefresh()returns true butrefreshTokenis nil, skip refresh and proceed with the existing token3. CLI subprocess env var stripping (ClaudeUsageProbe + InteractiveRunner)
ClaudeUsageProbenow creates itsDefaultCLIExecutorwithenvironmentExclusions: ["CLAUDE_CODE_OAUTH_TOKEN"]InteractiveRunner.Optionsgained anenvironmentExclusions: [String]field (defaults to[], backward-compatible)InteractiveRunner.terminalEnvironment(excluding:)removes specified keys before building the subprocess environmentclaude /usagefalls back to stored credentials with the fulluser:profilescopeFiles Changed
ClaudeCredentialLoader.swift.environmentsource, env loading, priority reorderClaudeAPIUsageProbe.swiftClaudeUsageProbe.swiftDefaultCLIExecutor.swiftenvironmentExclusionsInteractiveRunner.swiftenvironmentExclusionsin Options, strip interminalEnvironment()Tests
16 new tests added across 4 test files, all passing with zero regressions:
User Impact
Users who have
CLAUDE_CODE_OAUTH_TOKENset (fromclaude setup-token) and also have stored credentials (fromclaude login) will now see their quota data in ClaudeBar. The setup-token continues to handle inference in the terminal, while stored credentials handle quota monitoring — no workflow changes required.Note
Users still need to run
claude loginonce to create full-scope credentials. The setup-token alone cannot fetch quota data due to Anthropic's scope restrictions (user:inferenceonly).Summary by CodeRabbit
Release Notes
New Features
Improvements