-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(github): pass OAuth token to Python runner subprocesses (fixes #563) #698
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,31 @@ | ||
| import { getOAuthModeClearVars } from '../../../agent/env-utils'; | ||
| import { getAPIProfileEnv } from '../../../services/profile'; | ||
| import { getProfileEnv } from '../../../rate-limit-detector'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 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 🤖 Prompt for AI Agents |
||
|
|
||
| /** | ||
| * Get environment variables for Python runner subprocesses. | ||
| * | ||
| * Environment variable precedence (lowest to highest): | ||
| * 1. apiProfileEnv - Custom Anthropic-compatible API profile (ANTHROPIC_BASE_URL, ANTHROPIC_AUTH_TOKEN) | ||
| * 2. oauthModeClearVars - Clears stale ANTHROPIC_* vars when in OAuth mode | ||
| * 3. profileEnv - Claude OAuth token from profile manager (CLAUDE_CODE_OAUTH_TOKEN) | ||
| * 4. extraEnv - Caller-specific vars (e.g., USE_CLAUDE_MD) | ||
| * | ||
| * The profileEnv is critical for OAuth authentication (#563) - it retrieves the | ||
| * decrypted OAuth token from the profile manager's encrypted storage (macOS Keychain | ||
| * via Electron's safeStorage API). | ||
| */ | ||
| export async function getRunnerEnv( | ||
| extraEnv?: Record<string, string> | ||
| ): Promise<Record<string, string>> { | ||
| const apiProfileEnv = await getAPIProfileEnv(); | ||
| const oauthModeClearVars = getOAuthModeClearVars(apiProfileEnv); | ||
| const profileEnv = getProfileEnv(); | ||
|
|
||
| return { | ||
| ...apiProfileEnv, | ||
| ...oauthModeClearVars, | ||
| ...profileEnv, // OAuth token from profile manager (fixes #563) | ||
| ...extraEnv, | ||
| }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better code organization and maintainability, the
getProfileEnvfunction seems misplaced inrate-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
getProfileEnvto a more logical location, such as../../../services/profile, where the relatedgetAPIProfileEnvfunction is already defined. This would improve code structure, make the function easier to find, and avoid creating a dependency onrate-limit-detectorfor a non-rate-limiting concern.