fix: resolve Claude auth status via claude auth status instead of a credentials file - #1035
fix: resolve Claude auth status via claude auth status instead of a credentials file#1035zscgeek wants to merge 1 commit into
claude auth status instead of a credentials file#1035Conversation
… file claude-auth.provider.ts only checked a plaintext ~/.claude/.credentials.json for OAuth login state. On macOS, Claude Code actually stores OAuth credentials in the system Keychain, under a service name that includes a hash derived from CLAUDE_CONFIG_DIR when it's set (a different Keychain entry per profile) - so the file check found nothing and the Account tab always reported "not authenticated" for any OAuth-based login, on any profile, on any host where Keychain (or an equivalent OS credential store) is actually used. Reimplementing that Keychain hashing scheme isn't a stable contract to build against, so this shells out to `claude auth status --json` instead (inheriting the process env, including CLAUDE_CONFIG_DIR when set) and lets the CLI itself resolve however it actually stores credentials on the current platform/profile. The plaintext-file check is kept as a fast-path fallback for platforms/versions that do use a file. Verified directly against the compiled provider for three cases: a default-profile OAuth login, a CLAUDE_CONFIG_DIR-scoped OAuth login to a different account, and an unauthenticated CLAUDE_CONFIG_DIR - all three now report the correct account (or correctly report unauthenticated).
📝 WalkthroughWalkthroughClaude credential detection now checks local OAuth credentials first, then falls back to ChangesClaude authentication resolution
Sequence Diagram(s)sequenceDiagram
participant checkCredentials
participant readCredentialsFile
participant checkCliAuthStatus
checkCredentials->>readCredentialsFile: read local OAuth credentials
readCredentialsFile-->>checkCredentials: return credential result or null
checkCredentials->>checkCliAuthStatus: run claude auth status --json when null
checkCliAuthStatus-->>checkCredentials: return mapped auth status
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/modules/providers/list/claude/claude-auth.provider.ts`:
- Around line 173-192: Replace the synchronous spawn.sync call in the Claude
authentication status flow with an asynchronous execFile-based implementation,
using the existing async function context and preserving the 10-second timeout,
JSON output, environment, and argument handling. Ensure spawn failures,
timeouts, and non-zero exits are caught and mapped to the existing
authentication error response rather than falling through to
missingCredentialsError; update result.stdout handling to match the asynchronous
API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 30761b17-9b7f-4f42-9be8-e57bce10f637
📒 Files selected for processing (1)
server/modules/providers/list/claude/claude-auth.provider.ts
| let result; | ||
| try { | ||
| result = spawn.sync(cliPath, ['auth', 'status', '--json'], { | ||
| timeout: 10000, | ||
| encoding: 'utf8', | ||
| env: process.env, | ||
| }); | ||
| } catch { | ||
| return { | ||
| authenticated: false, | ||
| email: null, | ||
| method: null, | ||
| error: 'Unable to check Claude authentication status. Run claude /login again.', | ||
| }; | ||
| } | ||
|
|
||
| const stdout = typeof result.stdout === 'string' ? result.stdout.trim() : ''; | ||
| if (!stdout) { | ||
| return { authenticated: false, email: null, method: null, error: missingCredentialsError }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Avoid blocking the event loop and handle execution errors correctly.
Using synchronous spawn.sync inside an async function blocks the Node.js main thread for up to 10 seconds (the configured timeout). In a server environment, this stalls all concurrent requests and degrades availability.
Additionally, spawn.sync does not throw an exception when it fails to start the process (e.g., missing executable) or when it times out; it returns an error in result.error. The current try/catch block will not catch these failures, causing the code to swallow the error and fall through to reporting a misleading "missing credentials" error due to an empty stdout.
Consider replacing spawn.sync with an asynchronous alternative (such as child_process.execFile wrapped in util.promisify), which correctly throws on spawn failures, timeouts, and non-zero exit codes without blocking the event loop.
⚡ Proposed fix using promisified `execFile`
Assuming you have access to a promisified execFile (e.g., const execFileAsync = util.promisify(require('child_process').execFile)):
- let result;
+ let stdout = '';
try {
- result = spawn.sync(cliPath, ['auth', 'status', '--json'], {
+ const result = await execFileAsync(cliPath, ['auth', 'status', '--json'], {
timeout: 10000,
encoding: 'utf8',
env: process.env,
});
+ stdout = result.stdout.trim();
} catch {
return {
authenticated: false,
email: null,
method: null,
error: 'Unable to check Claude authentication status. Run claude /login again.',
};
}
- const stdout = typeof result.stdout === 'string' ? result.stdout.trim() : '';
if (!stdout) {
return { authenticated: false, email: null, method: null, error: missingCredentialsError };
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/modules/providers/list/claude/claude-auth.provider.ts` around lines
173 - 192, Replace the synchronous spawn.sync call in the Claude authentication
status flow with an asynchronous execFile-based implementation, using the
existing async function context and preserving the 10-second timeout, JSON
output, environment, and argument handling. Ensure spawn failures, timeouts, and
non-zero exits are caught and mapped to the existing authentication error
response rather than falling through to missingCredentialsError; update
result.stdout handling to match the asynchronous API.
1 participant
Summary
The Settings → Agents → Claude → Account tab always reports "Disconnected" / "Claude CLI is not authenticated" for OAuth-based logins (
claude /login), even whenclaudeitself is genuinely logged in and working fine in a real shell.Root cause
claude-auth.provider.ts'scheckCredentials()only ever checks a plaintext~/.claude/.credentials.jsonfile for OAuth login state. On macOS, Claude Code actually stores OAuth credentials in the system Keychain, under a service name that includes a hash derived fromCLAUDE_CONFIG_DIRwhen it's set (verified: a distinct Keychain entry —Claude Code-credentialsfor the default profile,Claude Code-credentials-<hash>for aCLAUDE_CONFIG_DIR-scoped profile). So the file the provider checks for simply never exists for any OAuth login on this platform, and it always falls through to "not authenticated" — regardless of whether the CLI is actually logged in, and regardless of whichCLAUDE_CONFIG_DIR/profile is active.Fix
Reimplementing that Keychain service-name hashing isn't a stable contract to build against (it's an internal detail of the CLI, not a documented interface). Instead, this shells out to
claude auth status --json, inheriting the current process environment (soCLAUDE_CONFIG_DIR, if set, is naturally respected) and lets the CLI itself resolve however it actually stores credentials — file, Keychain, or anything else it might use in the future. The plaintext-file check is kept as a fast path first, for platforms/older versions that do use a file.Testing
npm run typecheck— cleannpm run lint— no new errorsnpm run build— cleanClaudeProviderAuthdirectly (isolated from any other change, based straight off currentmain) for three cases:authenticated: trueCLAUDE_CONFIG_DIR-scoped OAuth login to a different account → correctly reports that different account's emailCLAUDE_CONFIG_DIR→ correctly reportsauthenticated: falsewith the expected error messageNote
This is independent of #1034 (which fixes
CLAUDE_CONFIG_DIRhandling for projects/sessions/settings elsewhere in the app) — either can merge first, they don't depend on each other.Summary by CodeRabbit