Skip to content

feat(backend): add Linux secret-service support for OAuth token storage (ACS-293)#4

Closed
StillKnotKnown wants to merge 1 commit intodevelopfrom
stillknotknown/acs-293-linux-profile-oauth-token-overrides-env-token-causing-authentication-failures
Closed

feat(backend): add Linux secret-service support for OAuth token storage (ACS-293)#4
StillKnotKnown wants to merge 1 commit intodevelopfrom
stillknotknown/acs-293-linux-profile-oauth-token-overrides-env-token-causing-authentication-failures

Conversation

@StillKnotKnown
Copy link
Owner

Base Branch

  • This PR targets the develop branch (required for all feature/fix PRs)
  • This PR targets main (hotfix only - maintainers)

Description

This PR implements Linux Secret Service API support for OAuth token storage, bringing Linux to feature parity with macOS (Keychain) and Windows (Credential Files).

The Problem (ACS-293): On Linux, the Electron app's profile system stores encrypted OAuth tokens that override the .env file's CLAUDE_CODE_OAUTH_TOKEN, causing authentication failures. Since the Python backend had no Linux keychain support (returned None), users had to manually remove tokens from profile JSON files.

The Fix: Added _get_token_from_linux_secret_service() function that:

  • Uses the secretstorage library to communicate with the Freedesktop.org Secret Service API via DBus
  • Searches for Claude Code credentials stored by the claude setup-token CLI
  • Validates token format and provides graceful fallback to .env when secret-service is unavailable
  • Works with gnome-keyring, kwallet, and other Secret Service providers

Impact: Linux users can now use claude setup-token to store OAuth tokens securely in their system keychain, just like macOS and Windows users. The profile token override issue is resolved because the backend can now retrieve valid tokens from the keychain.

Related Issue

Closes #ACS-293

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 📚 Documentation
  • ♻️ Refactor
  • 🧪 Test

Area

  • Frontend
  • Backend
  • Fullstack

Commit Message Format

Follow conventional commits: <type>: <subject>

Types: feat, fix, docs, style, refactor, test, chore

Example: feat: add user authentication system

Checklist

  • I've synced with develop branch
  • I've tested my changes locally
  • I've followed the code principles (SOLID, DRY, KISS)
  • My PR is small and focused (< 400 lines ideally)

CI/Testing Requirements

  • All CI checks pass
  • All existing tests pass
  • New features include test coverage
  • Bug fixes include regression tests

Test Coverage: Added comprehensive test suite in tests/test_auth.py with 38 tests covering:

  • Environment variable token resolution
  • macOS keychain token retrieval
  • Windows credential file token retrieval
  • Linux secret-service token retrieval (new)
  • Token source detection
  • Error handling and edge cases

All tests passing (38/38).

Screenshots

Before After

Feature Toggle

  • Behind localStorage flag: use_feature_name
  • Behind settings toggle
  • Behind environment variable/config
  • N/A - Feature is complete and ready for all users

Breaking Changes

Breaking: No

Details:

  • The change is fully backward compatible
  • If secretstorage is not installed or DBus is unavailable, the code gracefully falls back to requiring .env file token
  • The dependency is platform-specific (sys_platform == "linux") so it only affects Linux users
  • No changes to existing behavior on macOS or Windows

…ge (ACS-293)

This commit implements Linux keychain support using the secretstorage library,
bringing Linux to parity with macOS (Keychain) and Windows (Credential Files).

Changes:
- Add _get_token_from_linux_secret_service() function in auth.py
  - Uses secretstorage library for DBus communication
  - Searches for Claude Code credentials by application attribute
  - Validates token format (sk-ant-oat01- prefix)
  - Graceful fallback to .env when secret-service unavailable
- Update get_token_from_keychain() to call Linux implementation
- Update get_auth_token_source() to return "Linux Secret Service"
- Update require_auth_token() error message with Linux instructions
- Add secretstorage>=3.3.3 to requirements.txt (Linux-only)

Testing:
- Add comprehensive test suite in tests/test_auth.py (38 tests)
  - Environment variable token resolution
  - macOS keychain token retrieval
  - Windows credential file token retrieval
  - Linux secret-service token retrieval (new)
  - Token source detection
  - Error handling and edge cases

Fixes ACS-293
@StillKnotKnown
Copy link
Owner Author

Recreating on upstream repository

StillKnotKnown pushed a commit that referenced this pull request Jan 18, 2026
…ik90#1259)

* fix(windows): prevent zombie process accumulation on app close

- Use taskkill /f /t on Windows to properly kill process trees
  (SIGTERM/SIGKILL are ignored on Windows)
- Make killAllProcesses() wait for process exit events with timeout
- Kill PTY daemon process on shutdown
- Clear periodic update check interval on app quit

Fixes process accumulation in Task Manager after closing Auto-Claude.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(windows): extract killProcessGracefully utility with timer cleanup

- Extract shared killProcessGracefully() to platform module
- Fix Issue #1: Move taskkill outside try-catch scope
- Fix Issue #2: Track exit state to skip unnecessary taskkill
- Fix Issue #3: Add GRACEFUL_KILL_TIMEOUT_MS constant
- Fix Issue #4: Use consistent 5000ms timeout everywhere
- Fix Issue #5: Add debug logging for catch blocks
- Fix Issue AndyMik90#6: Log warning when process.once unavailable
- Fix Issue AndyMik90#7: Eliminate code duplication across 3 files
- Fix timer leak: Clear timeout on process exit/error, unref timer

Add comprehensive tests (19 test cases) covering:
- Windows taskkill fallback behavior
- Unix SIGTERM/SIGKILL sequence
- Timer cleanup and memory leak prevention
- Edge cases and error handling

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
StillKnotKnown pushed a commit that referenced this pull request Jan 26, 2026
…ndyMik90#1456)

* auto-claude: subtask-1-1 - Add GITHUB_AUTH_CHANGED IPC channel constant

* auto-claude: subtask-1-2 - Add async getGitHubTokenForSubprocess() helper to utils.ts

Add a new exported async function getGitHubTokenForSubprocess() that calls
getTokenFromGhCli() to retrieve fresh GitHub tokens for subprocess use.
This provides a clean interface for runner-env.ts to get tokens without
caching, ensuring account changes are reflected immediately.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-1-3 - Update getRunnerEnv() to include GITHUB_TOKEN

* auto-claude: subtask-2-1 - Add auth change detection and event emission to oauth-handlers.ts

- Add GitHubAuthChangedPayload interface for auth change events
- Add sendAuthChangedToRenderer() to broadcast auth changes to all windows
- Add getCurrentGitHubUsername() helper to get current GitHub user
- Modify registerStartGhAuth() to:
  - Capture username before auth starts
  - Get username after successful auth
  - Emit GITHUB_AUTH_CHANGED event if account changed

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-3-1 - Add onGitHubAuthChanged listener to GitHubAPI interface

- Add onGitHubAuthChanged to GitHubAPI interface in github-api.ts
- Add implementation using createIpcListener with IPC_CHANNELS.GITHUB_AUTH_CHANGED
- Add mock implementation in browser-mock.ts for testing

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-4-1 - Add GitHub auth change listener to pr-review-store

* fix: Address PR review findings for GitHub auth handlers

- Convert getCurrentGitHubUsername() to async using promisified execFile
  to avoid blocking Electron main thread during auth flow (finding #1)
- Make getGitHubTokenForSubprocess() truly async by introducing async
  getTokenFromGhCliAsync() - the sync version is preserved for
  getGitHubConfig (finding #2)
- Add warning log when username fetch fails after successful auth,
  handling the edge case where auth succeeds but account change
  detection fails (finding #3)
- Remove unused timestamp field from GitHubAuthChangedPayload interface
  since the renderer callback only uses oldUsername/newUsername (finding #4)
- Document the intentional extraEnv override behavior in getRunnerEnv()
  JSDoc comment (finding #5)

All 5 findings from PR review were real issues. These fixes improve code
quality by avoiding main thread blocking and clarifying edge cases.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* test: Fix oauth-handlers tests for async getCurrentGitHubUsername

Update test mocks and add waitForAsyncSetup helper to handle the async
changes in getCurrentGitHubUsername(). The function now uses promisified
execFile instead of execFileSync to avoid blocking the main thread.

Key changes:
- Add mockExecFile mock for the promisified execFile function
- Add waitForAsyncSetup helper to wait for async setup before emitting
  process events
- Update all affected tests to use waitForAsyncSetup before emitting
  mock process events

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
StillKnotKnown pushed a commit that referenced this pull request Feb 6, 2026
…ndyMik90#1747)

* auto-claude: subtask-1-1 - Add errorCode propagation in reactiveTokenRefresh()

* auto-claude: subtask-1-2 - Return null instead of revoked token for permanent errors in ensureValidToken()

* auto-claude: subtask-1-3 - Clear credential cache on invalid_grant error in refreshOAuthToken()

* auto-claude: subtask-1-4 - Clear revoked credentials on persistence failure (Bug #5)

When token refresh succeeds but persistence to keychain fails, the old
credentials in the keychain are now revoked server-side. This commit adds
defensive cache clearing in both ensureValidToken() and reactiveTokenRefresh()
to prevent serving revoked tokens from cache.

On app restart, Bugs #3 and #4 fixes will handle the revoked credentials
properly by returning null and clearing the cache, forcing re-authentication.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* auto-claude: subtask-2-1 - Move authFailedProfiles marking before early return

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
StillKnotKnown pushed a commit that referenced this pull request Feb 9, 2026
…ndyMik90#1456)

* auto-claude: subtask-1-1 - Add GITHUB_AUTH_CHANGED IPC channel constant

* auto-claude: subtask-1-2 - Add async getGitHubTokenForSubprocess() helper to utils.ts

Add a new exported async function getGitHubTokenForSubprocess() that calls
getTokenFromGhCli() to retrieve fresh GitHub tokens for subprocess use.
This provides a clean interface for runner-env.ts to get tokens without
caching, ensuring account changes are reflected immediately.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-1-3 - Update getRunnerEnv() to include GITHUB_TOKEN

* auto-claude: subtask-2-1 - Add auth change detection and event emission to oauth-handlers.ts

- Add GitHubAuthChangedPayload interface for auth change events
- Add sendAuthChangedToRenderer() to broadcast auth changes to all windows
- Add getCurrentGitHubUsername() helper to get current GitHub user
- Modify registerStartGhAuth() to:
  - Capture username before auth starts
  - Get username after successful auth
  - Emit GITHUB_AUTH_CHANGED event if account changed

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-3-1 - Add onGitHubAuthChanged listener to GitHubAPI interface

- Add onGitHubAuthChanged to GitHubAPI interface in github-api.ts
- Add implementation using createIpcListener with IPC_CHANNELS.GITHUB_AUTH_CHANGED
- Add mock implementation in browser-mock.ts for testing

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-4-1 - Add GitHub auth change listener to pr-review-store

* fix: Address PR review findings for GitHub auth handlers

- Convert getCurrentGitHubUsername() to async using promisified execFile
  to avoid blocking Electron main thread during auth flow (finding #1)
- Make getGitHubTokenForSubprocess() truly async by introducing async
  getTokenFromGhCliAsync() - the sync version is preserved for
  getGitHubConfig (finding #2)
- Add warning log when username fetch fails after successful auth,
  handling the edge case where auth succeeds but account change
  detection fails (finding #3)
- Remove unused timestamp field from GitHubAuthChangedPayload interface
  since the renderer callback only uses oldUsername/newUsername (finding #4)
- Document the intentional extraEnv override behavior in getRunnerEnv()
  JSDoc comment (finding #5)

All 5 findings from PR review were real issues. These fixes improve code
quality by avoiding main thread blocking and clarifying edge cases.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* test: Fix oauth-handlers tests for async getCurrentGitHubUsername

Update test mocks and add waitForAsyncSetup helper to handle the async
changes in getCurrentGitHubUsername(). The function now uses promisified
execFile instead of execFileSync to avoid blocking the main thread.

Key changes:
- Add mockExecFile mock for the promisified execFile function
- Add waitForAsyncSetup helper to wait for async setup before emitting
  process events
- Update all affected tests to use waitForAsyncSetup before emitting
  mock process events

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
StillKnotKnown pushed a commit that referenced this pull request Feb 9, 2026
…ndyMik90#1747)

* auto-claude: subtask-1-1 - Add errorCode propagation in reactiveTokenRefresh()

* auto-claude: subtask-1-2 - Return null instead of revoked token for permanent errors in ensureValidToken()

* auto-claude: subtask-1-3 - Clear credential cache on invalid_grant error in refreshOAuthToken()

* auto-claude: subtask-1-4 - Clear revoked credentials on persistence failure (Bug #5)

When token refresh succeeds but persistence to keychain fails, the old
credentials in the keychain are now revoked server-side. This commit adds
defensive cache clearing in both ensureValidToken() and reactiveTokenRefresh()
to prevent serving revoked tokens from cache.

On app restart, Bugs #3 and #4 fixes will handle the revoked credentials
properly by returning null and clearing the cache, forcing re-authentication.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* auto-claude: subtask-2-1 - Move authFailedProfiles marking before early return

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
StillKnotKnown pushed a commit that referenced this pull request Feb 9, 2026
…ndyMik90#1456)

* auto-claude: subtask-1-1 - Add GITHUB_AUTH_CHANGED IPC channel constant

* auto-claude: subtask-1-2 - Add async getGitHubTokenForSubprocess() helper to utils.ts

Add a new exported async function getGitHubTokenForSubprocess() that calls
getTokenFromGhCli() to retrieve fresh GitHub tokens for subprocess use.
This provides a clean interface for runner-env.ts to get tokens without
caching, ensuring account changes are reflected immediately.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-1-3 - Update getRunnerEnv() to include GITHUB_TOKEN

* auto-claude: subtask-2-1 - Add auth change detection and event emission to oauth-handlers.ts

- Add GitHubAuthChangedPayload interface for auth change events
- Add sendAuthChangedToRenderer() to broadcast auth changes to all windows
- Add getCurrentGitHubUsername() helper to get current GitHub user
- Modify registerStartGhAuth() to:
  - Capture username before auth starts
  - Get username after successful auth
  - Emit GITHUB_AUTH_CHANGED event if account changed

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-3-1 - Add onGitHubAuthChanged listener to GitHubAPI interface

- Add onGitHubAuthChanged to GitHubAPI interface in github-api.ts
- Add implementation using createIpcListener with IPC_CHANNELS.GITHUB_AUTH_CHANGED
- Add mock implementation in browser-mock.ts for testing

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-4-1 - Add GitHub auth change listener to pr-review-store

* fix: Address PR review findings for GitHub auth handlers

- Convert getCurrentGitHubUsername() to async using promisified execFile
  to avoid blocking Electron main thread during auth flow (finding #1)
- Make getGitHubTokenForSubprocess() truly async by introducing async
  getTokenFromGhCliAsync() - the sync version is preserved for
  getGitHubConfig (finding #2)
- Add warning log when username fetch fails after successful auth,
  handling the edge case where auth succeeds but account change
  detection fails (finding #3)
- Remove unused timestamp field from GitHubAuthChangedPayload interface
  since the renderer callback only uses oldUsername/newUsername (finding #4)
- Document the intentional extraEnv override behavior in getRunnerEnv()
  JSDoc comment (finding #5)

All 5 findings from PR review were real issues. These fixes improve code
quality by avoiding main thread blocking and clarifying edge cases.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* test: Fix oauth-handlers tests for async getCurrentGitHubUsername

Update test mocks and add waitForAsyncSetup helper to handle the async
changes in getCurrentGitHubUsername(). The function now uses promisified
execFile instead of execFileSync to avoid blocking the main thread.

Key changes:
- Add mockExecFile mock for the promisified execFile function
- Add waitForAsyncSetup helper to wait for async setup before emitting
  process events
- Update all affected tests to use waitForAsyncSetup before emitting
  mock process events

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
StillKnotKnown pushed a commit that referenced this pull request Feb 9, 2026
…ndyMik90#1747)

* auto-claude: subtask-1-1 - Add errorCode propagation in reactiveTokenRefresh()

* auto-claude: subtask-1-2 - Return null instead of revoked token for permanent errors in ensureValidToken()

* auto-claude: subtask-1-3 - Clear credential cache on invalid_grant error in refreshOAuthToken()

* auto-claude: subtask-1-4 - Clear revoked credentials on persistence failure (Bug #5)

When token refresh succeeds but persistence to keychain fails, the old
credentials in the keychain are now revoked server-side. This commit adds
defensive cache clearing in both ensureValidToken() and reactiveTokenRefresh()
to prevent serving revoked tokens from cache.

On app restart, Bugs #3 and #4 fixes will handle the revoked credentials
properly by returning null and clearing the cache, forcing re-authentication.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* auto-claude: subtask-2-1 - Move authFailedProfiles marking before early return

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
StillKnotKnown pushed a commit that referenced this pull request Feb 9, 2026
…ndyMik90#1456)

* auto-claude: subtask-1-1 - Add GITHUB_AUTH_CHANGED IPC channel constant

* auto-claude: subtask-1-2 - Add async getGitHubTokenForSubprocess() helper to utils.ts

Add a new exported async function getGitHubTokenForSubprocess() that calls
getTokenFromGhCli() to retrieve fresh GitHub tokens for subprocess use.
This provides a clean interface for runner-env.ts to get tokens without
caching, ensuring account changes are reflected immediately.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-1-3 - Update getRunnerEnv() to include GITHUB_TOKEN

* auto-claude: subtask-2-1 - Add auth change detection and event emission to oauth-handlers.ts

- Add GitHubAuthChangedPayload interface for auth change events
- Add sendAuthChangedToRenderer() to broadcast auth changes to all windows
- Add getCurrentGitHubUsername() helper to get current GitHub user
- Modify registerStartGhAuth() to:
  - Capture username before auth starts
  - Get username after successful auth
  - Emit GITHUB_AUTH_CHANGED event if account changed

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-3-1 - Add onGitHubAuthChanged listener to GitHubAPI interface

- Add onGitHubAuthChanged to GitHubAPI interface in github-api.ts
- Add implementation using createIpcListener with IPC_CHANNELS.GITHUB_AUTH_CHANGED
- Add mock implementation in browser-mock.ts for testing

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-4-1 - Add GitHub auth change listener to pr-review-store

* fix: Address PR review findings for GitHub auth handlers

- Convert getCurrentGitHubUsername() to async using promisified execFile
  to avoid blocking Electron main thread during auth flow (finding #1)
- Make getGitHubTokenForSubprocess() truly async by introducing async
  getTokenFromGhCliAsync() - the sync version is preserved for
  getGitHubConfig (finding #2)
- Add warning log when username fetch fails after successful auth,
  handling the edge case where auth succeeds but account change
  detection fails (finding #3)
- Remove unused timestamp field from GitHubAuthChangedPayload interface
  since the renderer callback only uses oldUsername/newUsername (finding #4)
- Document the intentional extraEnv override behavior in getRunnerEnv()
  JSDoc comment (finding #5)

All 5 findings from PR review were real issues. These fixes improve code
quality by avoiding main thread blocking and clarifying edge cases.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* test: Fix oauth-handlers tests for async getCurrentGitHubUsername

Update test mocks and add waitForAsyncSetup helper to handle the async
changes in getCurrentGitHubUsername(). The function now uses promisified
execFile instead of execFileSync to avoid blocking the main thread.

Key changes:
- Add mockExecFile mock for the promisified execFile function
- Add waitForAsyncSetup helper to wait for async setup before emitting
  process events
- Update all affected tests to use waitForAsyncSetup before emitting
  mock process events

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
StillKnotKnown pushed a commit that referenced this pull request Feb 9, 2026
…ndyMik90#1747)

* auto-claude: subtask-1-1 - Add errorCode propagation in reactiveTokenRefresh()

* auto-claude: subtask-1-2 - Return null instead of revoked token for permanent errors in ensureValidToken()

* auto-claude: subtask-1-3 - Clear credential cache on invalid_grant error in refreshOAuthToken()

* auto-claude: subtask-1-4 - Clear revoked credentials on persistence failure (Bug #5)

When token refresh succeeds but persistence to keychain fails, the old
credentials in the keychain are now revoked server-side. This commit adds
defensive cache clearing in both ensureValidToken() and reactiveTokenRefresh()
to prevent serving revoked tokens from cache.

On app restart, Bugs #3 and #4 fixes will handle the revoked credentials
properly by returning null and clearing the cache, forcing re-authentication.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* auto-claude: subtask-2-1 - Move authFailedProfiles marking before early return

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
StillKnotKnown pushed a commit that referenced this pull request Feb 10, 2026
…ndyMik90#1456)

* auto-claude: subtask-1-1 - Add GITHUB_AUTH_CHANGED IPC channel constant

* auto-claude: subtask-1-2 - Add async getGitHubTokenForSubprocess() helper to utils.ts

Add a new exported async function getGitHubTokenForSubprocess() that calls
getTokenFromGhCli() to retrieve fresh GitHub tokens for subprocess use.
This provides a clean interface for runner-env.ts to get tokens without
caching, ensuring account changes are reflected immediately.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-1-3 - Update getRunnerEnv() to include GITHUB_TOKEN

* auto-claude: subtask-2-1 - Add auth change detection and event emission to oauth-handlers.ts

- Add GitHubAuthChangedPayload interface for auth change events
- Add sendAuthChangedToRenderer() to broadcast auth changes to all windows
- Add getCurrentGitHubUsername() helper to get current GitHub user
- Modify registerStartGhAuth() to:
  - Capture username before auth starts
  - Get username after successful auth
  - Emit GITHUB_AUTH_CHANGED event if account changed

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-3-1 - Add onGitHubAuthChanged listener to GitHubAPI interface

- Add onGitHubAuthChanged to GitHubAPI interface in github-api.ts
- Add implementation using createIpcListener with IPC_CHANNELS.GITHUB_AUTH_CHANGED
- Add mock implementation in browser-mock.ts for testing

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-4-1 - Add GitHub auth change listener to pr-review-store

* fix: Address PR review findings for GitHub auth handlers

- Convert getCurrentGitHubUsername() to async using promisified execFile
  to avoid blocking Electron main thread during auth flow (finding #1)
- Make getGitHubTokenForSubprocess() truly async by introducing async
  getTokenFromGhCliAsync() - the sync version is preserved for
  getGitHubConfig (finding #2)
- Add warning log when username fetch fails after successful auth,
  handling the edge case where auth succeeds but account change
  detection fails (finding #3)
- Remove unused timestamp field from GitHubAuthChangedPayload interface
  since the renderer callback only uses oldUsername/newUsername (finding #4)
- Document the intentional extraEnv override behavior in getRunnerEnv()
  JSDoc comment (finding #5)

All 5 findings from PR review were real issues. These fixes improve code
quality by avoiding main thread blocking and clarifying edge cases.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* test: Fix oauth-handlers tests for async getCurrentGitHubUsername

Update test mocks and add waitForAsyncSetup helper to handle the async
changes in getCurrentGitHubUsername(). The function now uses promisified
execFile instead of execFileSync to avoid blocking the main thread.

Key changes:
- Add mockExecFile mock for the promisified execFile function
- Add waitForAsyncSetup helper to wait for async setup before emitting
  process events
- Update all affected tests to use waitForAsyncSetup before emitting
  mock process events

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
StillKnotKnown pushed a commit that referenced this pull request Feb 10, 2026
…ndyMik90#1747)

* auto-claude: subtask-1-1 - Add errorCode propagation in reactiveTokenRefresh()

* auto-claude: subtask-1-2 - Return null instead of revoked token for permanent errors in ensureValidToken()

* auto-claude: subtask-1-3 - Clear credential cache on invalid_grant error in refreshOAuthToken()

* auto-claude: subtask-1-4 - Clear revoked credentials on persistence failure (Bug #5)

When token refresh succeeds but persistence to keychain fails, the old
credentials in the keychain are now revoked server-side. This commit adds
defensive cache clearing in both ensureValidToken() and reactiveTokenRefresh()
to prevent serving revoked tokens from cache.

On app restart, Bugs #3 and #4 fixes will handle the revoked credentials
properly by returning null and clearing the cache, forcing re-authentication.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* auto-claude: subtask-2-1 - Move authFailedProfiles marking before early return

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant