fix(github): pass OAuth token to Python runner subprocesses (fixes #563) #698
Conversation
…dyMik90#563) The getRunnerEnv utility was missing the OAuth token from the Claude Profile Manager. It only included API profile env vars (ANTHROPIC_*) for custom endpoints, but not the CLAUDE_CODE_OAUTH_TOKEN needed for default Claude authentication. Root cause: The OAuth token is stored encrypted in Electron's profile storage (macOS Keychain via safeStorage), not as a system env var. The getProfileEnv() function retrieves and decrypts it. This fixes the 401 authentication errors in PR review, autofix, and triage handlers that all use getRunnerEnv().
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis change adds profile-based environment variables to the runner environment, specifically including the decrypted CLAUDE_CODE_OAUTH_TOKEN from secure storage. The profileEnv is merged with defined precedence to ensure the OAuth token is available in subprocess execution contexts. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
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 |
Summary of ChangesHello @MikeeBuilds, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request rectifies a critical authentication issue affecting GitHub integration features such as PR review and autofix. By ensuring the Claude OAuth token is correctly retrieved from the profile manager and passed to Python subprocesses, it eliminates Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves the authentication error by ensuring the Claude OAuth token is passed to Python subprocesses. The change is clear, well-documented with a helpful JSDoc comment explaining environment variable precedence, and directly addresses the root cause described in issue #563. I have one suggestion regarding code organization to improve long-term maintainability.
| @@ -1,15 +1,31 @@ | |||
| import { getOAuthModeClearVars } from '../../../agent/env-utils'; | |||
| import { getAPIProfileEnv } from '../../../services/profile'; | |||
| import { getProfileEnv } from '../../../rate-limit-detector'; | |||
There was a problem hiding this comment.
For better code organization and maintainability, the getProfileEnv function seems misplaced in rate-limit-detector.ts. Its purpose is to retrieve profile-specific environment variables, which is more aligned with profile management than rate limit detection.
Consider moving getProfileEnv to a more logical location, such as ../../../services/profile, where the related getAPIProfileEnv function is already defined. This would improve code structure, make the function easier to find, and avoid creating a dependency on rate-limit-detector for a non-rate-limiting concern.
![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
Fix all issues with AI Agents 🤖
In @apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts:
- Line 3: The import getProfileEnv from '../../../rate-limit-detector' indicates
the rate-limit-detector module now contains profile environment logic; extract
profile-related functions (e.g., getProfileEnv) into a dedicated module (suggest
name: profile-env or profile-utils), export them from that new module, update
runner-env.ts to import getProfileEnv from the new module, and update any other
files that currently import profile helpers from rate-limit-detector to the new
path; ensure exports/exports names remain consistent and run tests/typechecks to
confirm no breaking imports.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts
🧬 Code graph analysis (1)
apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts (1)
apps/frontend/src/main/agent/env-utils.ts (1)
getOAuthModeClearVars(22-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (3)
apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts (3)
5-17: Excellent documentation of environment precedence and security details.The expanded documentation clearly describes the environment variable precedence and the critical role of profileEnv in OAuth authentication. The reference to the encrypted storage mechanism (macOS Keychain via Electron's safeStorage API) is valuable context for maintainers.
25-30: Environment variable precedence is correctly implemented.The spread order correctly implements the documented precedence, ensuring:
profileEnvincludes the OAuth token afteroauthModeClearVars(so it's not cleared)extraEnvhas the highest precedence for caller-specific overrides- The fix for #563 is properly integrated
23-23: No action needed. ThegetProfileEnv()function inapps/frontend/src/main/rate-limit-detector.tsalready implements robust error handling:
- Returns an empty object
{}when no profile exists (line 266-268)- Safely handles token decryption failures with fallback logic (lines 274-278)
- Provides fallback returns for default profiles and legacy configs
- Always returns
Record<string, string>, nevernullorundefinedThe synchronous call on line 23 is safe and appropriate.
Likely an incorrect or invalid review comment.
| @@ -1,15 +1,31 @@ | |||
| import { getOAuthModeClearVars } from '../../../agent/env-utils'; | |||
| import { getAPIProfileEnv } from '../../../services/profile'; | |||
| import { getProfileEnv } from '../../../rate-limit-detector'; | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the rate-limit-detector module and getProfileEnv export
# Find the rate-limit-detector file
echo "=== Locating rate-limit-detector module ==="
fd -e ts -e tsx "rate-limit-detector" apps/frontend/src/main
# Navigate from the current file to check the relative path
echo -e "\n=== Checking relative path resolution ==="
# From apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts
# ../../../rate-limit-detector should resolve to apps/frontend/src/main/rate-limit-detector
# Check for files in the main directory
echo "Files in apps/frontend/src/main directory:"
ls -la apps/frontend/src/main/ | head -20Repository: AndyMik90/Auto-Claude
Length of output: 1540
🏁 Script executed:
#!/bin/bash
# Check for getProfileEnv export in the module
echo "=== Checking for getProfileEnv exports ==="
rg -n "export.*getProfileEnv" apps/frontend/src/main
echo -e "\n=== Checking rate-limit-detector module content ==="
# Check all files named rate-limit-detector or containing the module
fd "rate-limit-detector" apps/frontend/src/main --type f -x cat -n {}Repository: AndyMik90/Auto-Claude
Length of output: 37620
🏁 Script executed:
#!/bin/bash
# Examine the full content of the runner-env.ts file to understand context
echo "=== Full content of runner-env.ts ==="
cat -n apps/frontend/src/main/ipc-handlers/github/utils/runner-env.tsRepository: AndyMik90/Auto-Claude
Length of output: 1591
Import path is correct; consider refactoring module organization.
The import path '../../../rate-limit-detector' resolves correctly. However, the module's scope has expanded beyond rate limiting to include profile environment management. If this module continues to grow, consider extracting profile-related functions into a dedicated module to improve clarity and maintainability.
🤖 Prompt for AI Agents
In @apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts around line
3, The import getProfileEnv from '../../../rate-limit-detector' indicates the
rate-limit-detector module now contains profile environment logic; extract
profile-related functions (e.g., getProfileEnv) into a dedicated module (suggest
name: profile-env or profile-utils), export them from that new module, update
runner-env.ts to import getProfileEnv from the new module, and update any other
files that currently import profile helpers from rate-limit-detector to the new
path; ensure exports/exports names remain consistent and run tests/typechecks to
confirm no breaking imports.
MikeeBuilds
added
bug
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
✅ Auto Claude Review - APPROVED
Status: Ready to Merge
Summary: ### Merge Verdict: ✅ READY TO MERGE
No blocking issues. 1 non-blocking suggestion(s) to consider
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | Low | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
Findings Summary
- Low: 1 issue(s)
Generated by Auto Claude PR Review
💡 Suggestions (1)
These are non-blocking suggestions for consideration:
🔵 [LOW] Missing test coverage for new getProfileEnv integration
📁 apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts:0
The PR adds getProfileEnv() to the environment variable chain, but the existing test file (runner-env.test.ts) doesn't mock getProfileEnv and therefore doesn't test the new OAuth token functionality. While the implementation is correct, adding test coverage would improve confidence in the fix.
Suggested fix:
Add test case that mocks getProfileEnv to verify CLAUDE_CODE_OAUTH_TOKEN is properly included in the returned environment variables.
This automated review found no blocking issues. The PR can be safely merged.
Generated by Auto Claude
Merged commits: - fix: WIndows not finding the gith bash path (AndyMik90#724) - fix(profiles): support API profiles in auth check and model resolution (AndyMik90#608) - Fix Window Size on Hi-DPI Displays (AndyMik90#696) - fix: centralize Claude CLI invocation (AndyMik90#680) - fix(github): pass OAuth token to Python runner subprocesses (AndyMik90#698) - chore: Update Linux app icon to use multiple resolution sizes (AndyMik90#672) - fix(a11y): Add missing ARIA attributes for screen reader accessibility (AndyMik90#634) Resolved conflicts in: - apps/backend/core/auth.py (kept both Azure Foundry support + Windows git-bash detection) - apps/frontend/src/main/agent/agent-queue.ts (kept Azure Foundry model resolution) - apps/frontend/src/main/terminal/claude-integration-handler.ts (merged CLI invocation + Azure Foundry support) - apps/frontend/src/renderer/components/ProjectTabBar.tsx (kept overflow menu + added aria-label) - apps/frontend/src/shared/i18n/locales/en/common.json (merged all translation keys) - apps/frontend/src/shared/i18n/locales/fr/common.json (merged all translation keys)
…dyMik90#563) (AndyMik90#698) The getRunnerEnv utility was missing the OAuth token from the Claude Profile Manager. It only included API profile env vars (ANTHROPIC_*) for custom endpoints, but not the CLAUDE_CODE_OAUTH_TOKEN needed for default Claude authentication. Root cause: The OAuth token is stored encrypted in Electron's profile storage (macOS Keychain via safeStorage), not as a system env var. The getProfileEnv() function retrieves and decrypts it. This fixes the 401 authentication errors in PR review, autofix, and triage handlers that all use getRunnerEnv(). Co-authored-by: Andy <[email protected]>
…dyMik90#563) (AndyMik90#698) The getRunnerEnv utility was missing the OAuth token from the Claude Profile Manager. It only included API profile env vars (ANTHROPIC_*) for custom endpoints, but not the CLAUDE_CODE_OAUTH_TOKEN needed for default Claude authentication. Root cause: The OAuth token is stored encrypted in Electron's profile storage (macOS Keychain via safeStorage), not as a system env var. The getProfileEnv() function retrieves and decrypts it. This fixes the 401 authentication errors in PR review, autofix, and triage handlers that all use getRunnerEnv(). Co-authored-by: Andy <[email protected]>
2 participants
Summary
Fixes #563 | Linear: ACS-108 - The PR review (and other GitHub features) were failing with
401 authentication_error: Invalid bearer token.Root Cause
The
getRunnerEnv()utility was missing the OAuth token from the Claude Profile Manager. It only included API profile env vars (ANTHROPIC_*) for custom endpoints, but not theCLAUDE_CODE_OAUTH_TOKENneeded for default Claude OAuth authentication.The OAuth token is stored encrypted in Electron's profile storage (macOS Keychain via safeStorage API), not as a system environment variable. The
getProfileEnv()function retrieves and decrypts it.Changes
Added
getProfileEnv()call torunner-env.tsto include the decrypted OAuth token in the environment passed to Python subprocesses.Environment Variable Precedence (lowest to highest):
apiProfileEnv- Custom Anthropic-compatible API profileoauthModeClearVars- Clears stale ANTHROPIC_* vars when in OAuth modeprofileEnv- Claude OAuth token from profile manager (NEW)extraEnv- Caller-specific vars (e.g., USE_CLAUDE_MD)Impact
This fixes authentication for all GitHub features that use
getRunnerEnv():Testing
Summary by CodeRabbit
Release Notes
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.