Fix Claude and Gemini provider authentication compatibility#147
Fix Claude and Gemini provider authentication compatibility#147Daltonganger wants to merge 3 commits into
Conversation
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
![opgg-ai-devops[bot]](https://avatars.githubusercontent.com/in/2680015?s=60&v=4){kind=small}
There was a problem hiding this comment.
The direction is sensible: Gemini project precedence is centralized, the existing override order is preserved, and the new tests cover the pure helper behavior. CI is green across Lint, Build and Release, CI, and Test.
I’m requesting changes before merge because the new fallback paths are not contract-tested:
project: "default"is now sent to the quota API for accounts with missing project metadata, but the PR does not establish that this sentinel is accepted or maps to the correct account scope.- Any Gemini token-refresh failure now triggers a second OAuth-client attempt, even for accounts whose refresh token is not tied to the Gemini CLI client.
- The shell version parser can pass malformed
claude --versionoutput straight into the User-Agent, and Swift and shell now carry separate version-discovery policies.
The review runner could not execute macOS XCTest because xcodebuild is unavailable in this Linux environment. The existing GitHub Test check is green, but request-level tests for the changed network and shell behaviors are still needed. Also, the Antigravity “no account with project ID” message is now stale after the new fallback and should be updated while fixing the fallback contract.
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
|
Addressed all requested review changes in 22c83ce:
Validation: 236 Xcode tests total (220 passed, 16 credential-dependent skips, 0 failures), SwiftLint clean, shell contract tests passed, workflow validation passed, and live Gemini/Antigravity fetches succeeded. All four inline threads have been replied to and resolved. |
There was a problem hiding this comment.
The direction is solid: typed Gemini OAuth failures, centralized quota requests, and the Claude shell contract test are all useful improvements. CI is green, and bash scripts/tests/query-claude-version-test.sh passes.
The concrete benefit is that provider auth fallbacks now preserve error reasons and can use the installed Claude version instead of always sending the stale default.
I’m requesting changes because a few new fallback paths still fail for valid configurations:
- A 400 OAuth client-mismatch response skips the intended Gemini CLI-client retry.
- Antigravity stops at an unusable preferred account instead of trying the next usable one.
- The new
defaultproject policy is not applied consistently to standalone OAuth credentials.
The macOS XCTest/build path could not run in this Linux review environment. GitHub’s Test, Lint, Build and Release, and CI checks are green. The existing docs still describe the old Claude version behavior, so they should be updated with the implementation.
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
|
Second review round completed in bec84c4. This covers stale Antigravity fallback accounts, HTTP 400 OAuth client mismatches, ASCII-only Claude version parsing, standalone oauth_creds default-project handling, and the shared quota timeout. The Antigravity regression test is now active in the Xcode test target. Local validation: 247 tests total, 231 passed, 16 credential-dependent skips, 0 failures; SwiftLint, shell contracts, action validation, project validation, and a clean Debug build all passed. |
There was a problem hiding this comment.
The direction is solid: typed Gemini OAuth failures, the centralized quota request, and the Claude version contract tests are all useful. CI is green, and the shell contract test passes locally too.
The concrete benefit is real: Claude requests can identify the installed client version, and Gemini/Antigravity accounts without project metadata can reach the quota endpoint instead of being rejected up front.
I’m requesting changes because the new process/network fallback paths still add avoidable production risk:
- Claude launches
claude --versionon every provider refresh, with a three-second timeout and no cache. A missing or hung executable can therefore add that delay to every periodic refresh. - Gemini quota calls have a 10-second timeout, but the OAuth token request does not have an explicit timeout, so the refresh can still hang before the quota timeout is reached.
- The Swift and shell implementations now duplicate the Claude version/discovery policy, but their executable search paths differ (
CLAUDE_CODE_PATH/fallback paths in Swift versus onlycommand -vin the script). They can report different versions for the same machine. - The Swift process runner accepts valid-looking stdout without checking the exit status, so a failing
claude --versioncommand can still become the User-Agent.
GitHub’s macOS Build, Test, SwiftLint, and Actions Lint checks are green. Local macOS XCTest/SwiftLint could not run here because this review environment is Linux.
|
|
||
| do { | ||
| let (data, response) = try await URLSession.shared.data(for: request) | ||
| let (data, response) = try await session.data(for: request) |
There was a problem hiding this comment.
GeminiQuotaAPI has a 10-second timeout, but this token request inherits the session/default timeout. A stalled OAuth endpoint can therefore block the provider before the quota request starts. Set an explicit token-request timeout and cover it in the request contract test.
| @@ -191,11 +321,12 @@ final class ClaudeProvider: ProviderProtocol { | |||
| throw ProviderError.authenticationFailed("Anthropic access token not available") | |||
There was a problem hiding this comment.
The process termination handler returns stdout without checking the exit status. A broken claude executable can print a valid-looking version and exit nonzero, and that output will still become the User-Agent. Require a zero termination status before accepting the output, and add a regression test.
| throw ProviderError.authenticationFailed("Anthropic access token not available") | ||
| } | ||
|
|
||
| let installedVersion = await ClaudeOAuthRequestPolicy.installedClaudeCodeVersion() |
There was a problem hiding this comment.
This runs before every account fetch, so a missing or hung claude executable can add up to three seconds to every periodic refresh even though the version is unchanged. Cache the discovered version for the provider/process lifetime, or resolve it only when needed and invalidate it explicitly.
| fi | ||
|
|
||
| printf '%s\n' "$CLAUDE_DEFAULT_CODE_VERSION" | ||
| } |
There was a problem hiding this comment.
The shell diagnostic only calls command -v claude, while the Swift path also honors CLAUDE_CODE_PATH, every supplied PATH entry, and fallback locations. An installation found by the app can therefore fall back to 2.1.80 in query-claude.sh, contradicting the documented “same version policy.” Make discovery order identical across both consumers or drive both from shared contract fixtures.
1 participant
Summary
claude --versionand use the validated version in Anthropic OAuth request headers.defaultproject fallback.Root cause
Claude usage requests advertised
claude-code/2.1.80while the installed Claude Code client was newer. Anthropic returned HTTP 429 for the stale client identity, while the same credentials succeeded with the installed client version.Current Gemini and Antigravity credential files can omit
projectId. The quota API acceptsproject: "default", but OpenCode Bar rejected or skipped these accounts before making the request. This caused both providers to show an error even when a valid Antigravity refresh token was available.Review hardening
--versionoutput as authoritative and accept only ASCIIX.Y.ZorX.Y.Z (Claude Code).invalid_clientandunauthorized_clientretry on HTTP 400 or 401;invalid_grant, transport failures, Antigravity, and standalone Gemini CLI credentials do not retry.{"project":"default"}payload, and successful fixture decoding.oauth_creds.jsoncredentials use the same project precedence as the other sources: explicit project, quota project, environment project, thendefault.Validation
swiftlint lint --strict --no-cache --config .swiftlint.yml: 0 violations.bash scripts/tests/query-claude-version-test.sh: all strict version-parser cases passed, including Unicode-digit rejection.make lint-actions: all workflow validations passed.git diff --checkpassed.