diff --git a/apps/frontend/src/main/ipc-handlers/github/__tests__/oauth-handlers.spec.ts b/apps/frontend/src/main/ipc-handlers/github/__tests__/oauth-handlers.spec.ts index 4c3c942f..b0b5f073 100644 --- a/apps/frontend/src/main/ipc-handlers/github/__tests__/oauth-handlers.spec.ts +++ b/apps/frontend/src/main/ipc-handlers/github/__tests__/oauth-handlers.spec.ts @@ -9,6 +9,7 @@ import { EventEmitter } from 'events'; const mockSpawn = vi.fn(); const mockExecSync = vi.fn(); const mockExecFileSync = vi.fn(); +const mockExecFile = vi.fn(); vi.mock('child_process', async (importOriginal) => { const actual = await importOriginal(); @@ -16,7 +17,8 @@ vi.mock('child_process', async (importOriginal) => { ...actual, spawn: (...args: unknown[]) => mockSpawn(...args), execSync: (...args: unknown[]) => mockExecSync(...args), - execFileSync: (...args: unknown[]) => mockExecFileSync(...args) + execFileSync: (...args: unknown[]) => mockExecFileSync(...args), + execFile: (...args: unknown[]) => mockExecFile(...args) }; }); @@ -110,6 +112,10 @@ function createMockProcess(): EventEmitter & { return proc; } +// Helper to wait for async setup (getCurrentGitHubUsername) to complete +// This is needed because the handler now awaits async operations before spawning +const waitForAsyncSetup = () => new Promise(resolve => setTimeout(resolve, 20)); + describe('GitHub OAuth Handlers', () => { let ipcMain: EventEmitter & { handlers: Map; @@ -125,6 +131,24 @@ describe('GitHub OAuth Handlers', () => { mockGetAugmentedEnv.mockReturnValue(process.env as Record); mockFindExecutable.mockReturnValue(null); // Default: executable not found + // Set up default execFile mock for getCurrentGitHubUsername (async) + // This returns null by default (not authenticated) + mockExecFile.mockImplementation( + ( + _cmd: string, + _args: string[], + _options: unknown, + callback?: (error: Error | null, stdout: string, stderr: string) => void + ) => { + // If callback provided, call it with error to simulate not authenticated + if (callback) { + callback(new Error('not authenticated'), '', ''); + } + // Return a mock ChildProcess-like object + return { on: vi.fn(), stdout: null, stderr: null }; + } + ); + // Get mocked ipcMain const electron = await import('electron'); ipcMain = electron.ipcMain as unknown as typeof ipcMain; @@ -146,6 +170,9 @@ describe('GitHub OAuth Handlers', () => { // Start the handler const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup (getCurrentGitHubUsername) to complete + await waitForAsyncSetup(); + // Simulate gh CLI output with device code mockProcess.stderr?.emit('data', '! First copy your one-time code: ABCD-1234\n'); mockProcess.stderr?.emit('data', '- Press Enter to open github.com in your browser...\n'); @@ -171,6 +198,9 @@ describe('GitHub OAuth Handlers', () => { const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup + await waitForAsyncSetup(); + // Alternate format: "code: XXXX-XXXX" without "one-time" mockProcess.stderr?.emit('data', 'Enter the code: EFGH-5678\n'); mockProcess.emit('close', 0); @@ -192,6 +222,9 @@ describe('GitHub OAuth Handlers', () => { const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup + await waitForAsyncSetup(); + // Device code in stdout instead of stderr mockProcess.stdout?.emit('data', '! First copy your one-time code: IJKL-9012\n'); mockProcess.emit('close', 0); @@ -212,6 +245,9 @@ describe('GitHub OAuth Handlers', () => { const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup + await waitForAsyncSetup(); + // Output without device code mockProcess.stderr?.emit('data', 'Some other message\n'); mockProcess.emit('close', 0); @@ -233,6 +269,9 @@ describe('GitHub OAuth Handlers', () => { const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup + await waitForAsyncSetup(); + mockProcess.stderr?.emit('data', '! First copy your one-time code: MNOP-3456\n'); mockProcess.stderr?.emit('data', 'Then visit https://github.com/login/device to authenticate\n'); mockProcess.emit('close', 0); @@ -256,9 +295,12 @@ describe('GitHub OAuth Handlers', () => { const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup + await waitForAsyncSetup(); + mockProcess.stderr?.emit('data', '! First copy your one-time code: QRST-7890\n'); - // Wait for next tick to allow async browser opening + // Wait for async browser opening await new Promise(resolve => setTimeout(resolve, 10)); mockProcess.emit('close', 0); @@ -277,6 +319,9 @@ describe('GitHub OAuth Handlers', () => { const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup + await waitForAsyncSetup(); + mockProcess.stderr?.emit('data', '! First copy your one-time code: UVWX-1234\n'); // Wait for async browser opening @@ -300,6 +345,9 @@ describe('GitHub OAuth Handlers', () => { const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup + await waitForAsyncSetup(); + mockProcess.stderr?.emit('data', '! First copy your one-time code: YZAB-5678\n'); // Wait for async browser opening to fail @@ -323,6 +371,9 @@ describe('GitHub OAuth Handlers', () => { const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup + await waitForAsyncSetup(); + mockProcess.stderr?.emit('data', '! First copy your one-time code: CDEF-9012\n'); // Wait for async browser opening to fail @@ -346,6 +397,9 @@ describe('GitHub OAuth Handlers', () => { const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup + await waitForAsyncSetup(); + mockProcess.stderr?.emit('data', '! First copy your one-time code: GHIJ-3456\n'); // Wait for async browser opening @@ -370,6 +424,9 @@ describe('GitHub OAuth Handlers', () => { const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup + await waitForAsyncSetup(); + // Emit error event mockProcess.emit('error', new Error('spawn gh ENOENT')); @@ -390,6 +447,9 @@ describe('GitHub OAuth Handlers', () => { const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup + await waitForAsyncSetup(); + mockProcess.stderr?.emit('data', 'error: some authentication error\n'); mockProcess.emit('close', 1); @@ -410,6 +470,9 @@ describe('GitHub OAuth Handlers', () => { const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup + await waitForAsyncSetup(); + // Device code output followed by failure mockProcess.stderr?.emit('data', '! First copy your one-time code: KLMN-7890\n'); @@ -436,6 +499,9 @@ describe('GitHub OAuth Handlers', () => { const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + // Wait for async setup + await waitForAsyncSetup(); + mockProcess.emit('error', new Error('spawn gh ENOENT')); const result = await resultPromise; @@ -532,7 +598,11 @@ describe('GitHub OAuth Handlers', () => { const { registerStartGhAuth } = await import('../oauth-handlers'); registerStartGhAuth(); - ipcMain.invokeHandler('github:startAuth', {}); + // Start the handler (this is async due to getCurrentGitHubUsername) + const resultPromise = ipcMain.invokeHandler('github:startAuth', {}); + + // Wait for the async getCurrentGitHubUsername to complete and spawn to be called + await new Promise(resolve => setTimeout(resolve, 10)); expect(mockSpawn).toHaveBeenCalledWith( 'gh', @@ -541,6 +611,10 @@ describe('GitHub OAuth Handlers', () => { stdio: ['pipe', 'pipe', 'pipe'] }) ); + + // Complete the process to avoid hanging promise + mockProcess.emit('close', 0); + await resultPromise; }); }); diff --git a/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts index 81d8cd81..aa1a7aeb 100644 --- a/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts @@ -4,12 +4,15 @@ */ import { ipcMain, shell, BrowserWindow } from 'electron'; -import { execSync, execFileSync, spawn } from 'child_process'; +import { execSync, execFileSync, execFile, spawn } from 'child_process'; +import { promisify } from 'util'; import { IPC_CHANNELS } from '../../../shared/constants'; import type { IPCResult } from '../../../shared/types'; import { getAugmentedEnv, findExecutable } from '../../env-utils'; import { getToolPath } from '../../cli-tool-manager'; +const execFileAsync = promisify(execFile); + /** * Send device code info to all renderer windows immediately when extracted * This allows the UI to display the code while the auth process is still running @@ -26,6 +29,48 @@ function sendDeviceCodeToRenderer(deviceCode: string, authUrl: string, browserOp } } +/** + * Payload for GitHub auth change event + */ +interface GitHubAuthChangedPayload { + oldUsername: string | null; + newUsername: string; +} + +/** + * Send auth change notification to all renderer windows + * This notifies the UI that GitHub authentication has changed (e.g., account swap) + */ +function sendAuthChangedToRenderer(oldUsername: string | null, newUsername: string): void { + debugLog('Sending auth changed event to renderer windows', { oldUsername, newUsername }); + const windows = BrowserWindow.getAllWindows(); + const payload: GitHubAuthChangedPayload = { + oldUsername, + newUsername + }; + for (const win of windows) { + win.webContents.send(IPC_CHANNELS.GITHUB_AUTH_CHANGED, payload); + } +} + +/** + * Get current GitHub username from gh CLI (async to avoid blocking main thread) + * Returns null if not authenticated or on error + */ +async function getCurrentGitHubUsername(): Promise { + try { + const { stdout } = await execFileAsync(getToolPath('gh'), ['api', 'user', '--jq', '.login'], { + encoding: 'utf-8', + env: getAugmentedEnv() + }); + const username = stdout.trim(); + return username || null; + } catch { + // Not authenticated or gh CLI error + return null; + } +} + // Debug logging helper const DEBUG = process.env.DEBUG === 'true' || process.env.NODE_ENV === 'development'; @@ -230,12 +275,20 @@ interface GitHubAuthStartResult { * Start GitHub OAuth flow using gh CLI * This will extract the device code from gh CLI output and open the browser * using Electron's shell.openExternal (bypasses macOS child process restrictions) + * + * Detects account changes and emits GITHUB_AUTH_CHANGED event when the authenticated + * account differs from the previous one (or when going from unauthenticated to authenticated). */ export function registerStartGhAuth(): void { ipcMain.handle( IPC_CHANNELS.GITHUB_START_AUTH, async (): Promise> => { debugLog('startGitHubAuth handler called'); + + // Capture current username before auth to detect account changes (async to avoid blocking main thread) + const usernameBeforeAuth = await getCurrentGitHubUsername(); + debugLog('Username before auth:', usernameBeforeAuth || '(not authenticated)'); + return new Promise((resolve) => { try { // Use gh auth login with web flow and repo scope @@ -313,12 +366,29 @@ export function registerStartGhAuth(): void { void tryExtractAndOpenBrowser(); }); - ghProcess.on('close', (code) => { + ghProcess.on('close', async (code) => { debugLog('gh process exited with code:', code); debugLog('Full stdout:', output); debugLog('Full stderr:', errorOutput); if (code === 0) { + // Check for auth change after successful authentication (async to avoid blocking main thread) + const usernameAfterAuth = await getCurrentGitHubUsername(); + debugLog('Username after auth:', usernameAfterAuth || '(unknown)'); + + // Emit auth changed event if account changed (or went from unauthenticated to authenticated) + if (usernameAfterAuth && usernameAfterAuth !== usernameBeforeAuth) { + debugLog('GitHub account changed detected', { + from: usernameBeforeAuth || '(none)', + to: usernameAfterAuth + }); + sendAuthChangedToRenderer(usernameBeforeAuth, usernameAfterAuth); + } else if (!usernameAfterAuth) { + // Auth succeeded (exit code 0) but username fetch failed - log warning + // This edge case means we can't detect account changes, but auth is still valid + debugLog('WARNING: Auth succeeded but could not fetch username to detect account change'); + } + // Success case - include fallbackUrl if browser failed to open // so the user can manually navigate if needed resolve({ diff --git a/apps/frontend/src/main/ipc-handlers/github/utils.ts b/apps/frontend/src/main/ipc-handlers/github/utils.ts index 3b0799b4..114059e3 100644 --- a/apps/frontend/src/main/ipc-handlers/github/utils.ts +++ b/apps/frontend/src/main/ipc-handlers/github/utils.ts @@ -3,7 +3,8 @@ */ import { existsSync, readFileSync } from 'fs'; -import { execFileSync } from 'child_process'; +import { execFileSync, execFile } from 'child_process'; +import { promisify } from 'util'; import path from 'path'; import type { Project } from '../../../shared/types'; import { parseEnvFile } from '../utils'; @@ -11,11 +12,30 @@ import type { GitHubConfig } from './types'; import { getAugmentedEnv } from '../../env-utils'; import { getToolPath } from '../../cli-tool-manager'; +const execFileAsync = promisify(execFile); + /** - * Get GitHub token from gh CLI if available + * Get GitHub token from gh CLI if available (async to avoid blocking main thread) * Uses augmented PATH to find gh CLI in common locations (e.g., Homebrew on macOS) */ -function getTokenFromGhCli(): string | null { +async function getTokenFromGhCliAsync(): Promise { + try { + const { stdout } = await execFileAsync(getToolPath('gh'), ['auth', 'token'], { + encoding: 'utf-8', + env: getAugmentedEnv() + }); + const token = stdout.trim(); + return token || null; + } catch { + return null; + } +} + +/** + * Get GitHub token from gh CLI if available (sync version for getGitHubConfig) + * Uses augmented PATH to find gh CLI in common locations (e.g., Homebrew on macOS) + */ +function getTokenFromGhCliSync(): string | null { try { const token = execFileSync(getToolPath('gh'), ['auth', 'token'], { encoding: 'utf-8', @@ -28,6 +48,15 @@ function getTokenFromGhCli(): string | null { } } +/** + * Get a fresh GitHub token for subprocess use (async to avoid blocking main thread) + * Always fetches fresh from gh CLI - no caching to ensure account changes are reflected + * @returns The current GitHub token or null if not authenticated + */ +export async function getGitHubTokenForSubprocess(): Promise { + return getTokenFromGhCliAsync(); +} + /** * Get GitHub configuration from project environment file * Falls back to gh CLI token if GITHUB_TOKEN not in .env @@ -43,9 +72,9 @@ export function getGitHubConfig(project: Project): GitHubConfig | null { let token: string | undefined = vars['GITHUB_TOKEN']; const repo = vars['GITHUB_REPO']; - // If no token in .env, try to get it from gh CLI + // If no token in .env, try to get it from gh CLI (sync version for sync function) if (!token) { - const ghToken = getTokenFromGhCli(); + const ghToken = getTokenFromGhCliSync(); if (ghToken) { token = ghToken; } diff --git a/apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts b/apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts index ace24490..dd05c336 100644 --- a/apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts +++ b/apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts @@ -2,6 +2,7 @@ import { getOAuthModeClearVars } from '../../../agent/env-utils'; import { getAPIProfileEnv } from '../../../services/profile'; import { getProfileEnv } from '../../../rate-limit-detector'; import { pythonEnvManager } from '../../../python-env-manager'; +import { getGitHubTokenForSubprocess } from '../utils'; /** * Get environment variables for Python runner subprocesses. @@ -11,7 +12,11 @@ import { pythonEnvManager } from '../../../python-env-manager'; * 2. apiProfileEnv - Custom Anthropic-compatible API profile (ANTHROPIC_BASE_URL, ANTHROPIC_AUTH_TOKEN) * 3. oauthModeClearVars - Clears stale ANTHROPIC_* vars when in OAuth mode * 4. profileEnv - Claude OAuth token from profile manager (CLAUDE_CODE_OAUTH_TOKEN) - * 5. extraEnv - Caller-specific vars (e.g., USE_CLAUDE_MD) + * 5. githubEnv - Fresh GitHub token from gh CLI (GITHUB_TOKEN) - fetched on each call to reflect account changes + * 6. extraEnv - Caller-specific vars (e.g., USE_CLAUDE_MD) + * + * NOTE: extraEnv can intentionally override any of the above, including GITHUB_TOKEN. + * This allows callers to provide their own token for testing or special cases. * * The pythonEnv is critical for packaged apps (#139) - without PYTHONPATH, Python * cannot find bundled dependencies like dotenv, claude_agent_sdk, etc. @@ -19,6 +24,9 @@ import { pythonEnvManager } from '../../../python-env-manager'; * 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). + * + * The githubEnv is critical for GitHub operations (#151) - it fetches a fresh token + * from the gh CLI on each call to ensure account changes are reflected immediately. */ export async function getRunnerEnv( extraEnv?: Record @@ -28,11 +36,16 @@ export async function getRunnerEnv( const oauthModeClearVars = getOAuthModeClearVars(apiProfileEnv); const profileEnv = getProfileEnv(); + // Fetch fresh GitHub token from gh CLI (no caching to reflect account changes) + const githubToken = await getGitHubTokenForSubprocess(); + const githubEnv: Record = githubToken ? { GITHUB_TOKEN: githubToken } : {}; + return { ...pythonEnv, // Python environment including PYTHONPATH (fixes #139) ...apiProfileEnv, ...oauthModeClearVars, ...profileEnv, // OAuth token from profile manager (fixes #563) + ...githubEnv, // Fresh GitHub token from gh CLI (fixes #151) ...extraEnv, }; } diff --git a/apps/frontend/src/preload/api/modules/github-api.ts b/apps/frontend/src/preload/api/modules/github-api.ts index 6e409c7e..a814fdae 100644 --- a/apps/frontend/src/preload/api/modules/github-api.ts +++ b/apps/frontend/src/preload/api/modules/github-api.ts @@ -189,6 +189,11 @@ export interface GitHubAPI { callback: (data: { deviceCode: string; authUrl: string; browserOpened: boolean }) => void ) => IpcListenerCleanup; + // OAuth event listener - notifies when GitHub account changes (via gh auth login) + onGitHubAuthChanged: ( + callback: (data: { oldUsername: string | null; newUsername: string }) => void + ) => IpcListenerCleanup; + // Repository detection and management detectGitHubRepo: (projectPath: string) => Promise>; getGitHubBranches: (repo: string, token: string) => Promise>; @@ -532,6 +537,12 @@ export const createGitHubAPI = (): GitHubAPI => ({ ): IpcListenerCleanup => createIpcListener(IPC_CHANNELS.GITHUB_AUTH_DEVICE_CODE, callback), + // OAuth event listener - notifies when GitHub account changes (via gh auth login) + onGitHubAuthChanged: ( + callback: (data: { oldUsername: string | null; newUsername: string }) => void + ): IpcListenerCleanup => + createIpcListener(IPC_CHANNELS.GITHUB_AUTH_CHANGED, callback), + // Repository detection and management detectGitHubRepo: (projectPath: string): Promise> => invokeIpc(IPC_CHANNELS.GITHUB_DETECT_REPO, projectPath), diff --git a/apps/frontend/src/renderer/lib/browser-mock.ts b/apps/frontend/src/renderer/lib/browser-mock.ts index bef70985..83824e8f 100644 --- a/apps/frontend/src/renderer/lib/browser-mock.ts +++ b/apps/frontend/src/renderer/lib/browser-mock.ts @@ -189,6 +189,7 @@ const browserMockAPI: ElectronAPI = { addGitRemote: async () => ({ success: true, data: { remoteUrl: '' } }), listGitHubOrgs: async () => ({ success: true, data: { orgs: [] } }), onGitHubAuthDeviceCode: () => () => {}, + onGitHubAuthChanged: () => () => {}, onGitHubInvestigationProgress: () => () => {}, onGitHubInvestigationComplete: () => () => {}, onGitHubInvestigationError: () => () => {}, diff --git a/apps/frontend/src/renderer/stores/github/pr-review-store.ts b/apps/frontend/src/renderer/stores/github/pr-review-store.ts index a592c9df..91a8bf16 100644 --- a/apps/frontend/src/renderer/stores/github/pr-review-store.ts +++ b/apps/frontend/src/renderer/stores/github/pr-review-store.ts @@ -252,6 +252,18 @@ export function initializePRReviewListeners(): void { } ); + // Listen for GitHub auth changes - clear all PR review state when account changes + window.electronAPI.github.onGitHubAuthChanged( + (data: { oldUsername: string | null; newUsername: string }) => { + console.warn( + `[PRReviewStore] GitHub auth changed from "${data.oldUsername ?? 'none'}" to "${data.newUsername}". ` + + `Clearing all PR review state.` + ); + // Clear all PR review state since the token has changed + usePRReviewStore.setState({ prReviews: {} }); + } + ); + prReviewListenersInitialized = true; } diff --git a/apps/frontend/src/shared/constants/ipc.ts b/apps/frontend/src/shared/constants/ipc.ts index 98d9131c..d7d4f368 100644 --- a/apps/frontend/src/shared/constants/ipc.ts +++ b/apps/frontend/src/shared/constants/ipc.ts @@ -254,6 +254,7 @@ export const IPC_CHANNELS = { // GitHub OAuth events (main -> renderer) - for streaming device code during auth GITHUB_AUTH_DEVICE_CODE: 'github:authDeviceCode', + GITHUB_AUTH_CHANGED: 'github:authChanged', // Event: GitHub auth state changed (account swap) // GitHub events (main -> renderer) GITHUB_INVESTIGATION_PROGRESS: 'github:investigationProgress',