fix(gh cli): migrate legacy gh cli auth token into keychain

src/main/ipc/githubIpc.ts

@@ -1,6 +1,6 @@
 import { ipcMain, app } from 'electron';
 import { log } from '../lib/logger';
-import { GitHubService } from '../services/GitHubService';
+import { githubService } from '../services/GitHubService';
 import { worktreeService } from '../services/WorktreeService';
 import { githubCLIInstaller } from '../services/GitHubCLIInstaller';
 import { databaseService } from '../services/DatabaseService';
@@ -14,7 +14,6 @@ import { quoteShellArg } from '../utils/shellEscape';
 import { getAppSettings } from '../settings';
 
 const execAsync = promisify(exec);
-const githubService = new GitHubService();
 
 const slugify = (name: string) =>
   name

src/main/services/GitHubService.ts

@@ -97,13 +97,22 @@ export interface DeviceCodeResult {
 export class GitHubService {
   private readonly SERVICE_NAME = 'emdash-github';
   private readonly ACCOUNT_NAME = 'github-token';
+  private readonly MIGRATION_BLOCK_ACCOUNT = 'github-migration-blocked';
 
   // Polling state management
   private isPolling = false;
   private pollingInterval: NodeJS.Timeout | null = null;
   private currentDeviceCode: string | null = null;
   private currentInterval = 5;
 
+  // One-shot migration guard: try reading from `gh auth token` at most once
+  // per process when the Emdash keychain is empty.
+  private migrationAttempted = false;
+  private migrationInFlight: Promise<string | null> | null = null;
+
+  // Serializes auth state changes (logout + legacy token migration persistence).
+  private authStateLock: Promise<void> = Promise.resolve();
+
   /**
    * Authenticate with GitHub using Device Flow
    * Returns device code info for the UI to display to the user
@@ -1258,44 +1267,136 @@ export class GitHubService {
     }
   }
 
+  private async withAuthStateLock<T>(operation: () => Promise<T>): Promise<T> {
+    const previousLock = this.authStateLock;
+    let releaseLock!: () => void;
+
+    this.authStateLock = new Promise<void>((resolve) => {
+      releaseLock = resolve;
+    });
+
+    await previousLock;
+
+    try {
+      return await operation();
+    } finally {
+      releaseLock();
+    }
+  }
+
   /**
    * Logout and clear stored token
    */
   async logout(): Promise<void> {
     this.stopPolling();
-    try {
-      const keytar = await import('keytar');
-      await keytar.deletePassword(this.SERVICE_NAME, this.ACCOUNT_NAME);
-    } catch (error) {
-      console.error('Failed to clear keychain token:', error);
-      throw new Error('Failed to clear keychain token');
-    }
+    this.migrationAttempted = true;
+
+    await this.withAuthStateLock(async () => {
+      try {
+        const keytar = await import('keytar');
+        await keytar.deletePassword(this.SERVICE_NAME, this.ACCOUNT_NAME);
+        await keytar.setPassword(this.SERVICE_NAME, this.MIGRATION_BLOCK_ACCOUNT, '1');
+      } catch (error) {
+        console.error('Failed to clear keychain token:', error);
+        throw new Error('Failed to clear keychain token');
+      }
+    });
   }
 
   /**
    * Store authentication token securely
    */
-  private async storeToken(token: string): Promise<void> {
+  private async storeToken(token: string, source: 'user' | 'migration' = 'user'): Promise<void> {
     try {
       const keytar = await import('keytar');
       await keytar.setPassword(this.SERVICE_NAME, this.ACCOUNT_NAME, token);
+      if (source === 'user') {
+        await keytar.deletePassword(this.SERVICE_NAME, this.MIGRATION_BLOCK_ACCOUNT);
+      }
     } catch (error) {
       console.error('Failed to store token:', error);
       throw error;
     }
   }
 
   /**
-   * Retrieve stored authentication token
+   * Retrieve stored authentication token.
+   *
+   * Migration for users authenticated before the gh-CLI decouple: their token
+   * only lives in the global gh CLI state. If the Emdash keychain is empty we
+   * try `gh auth token` once and persist the result, so PR lists and other
+   * gh-backed features keep working without asking the user to re-auth.
    */
   async getStoredToken(): Promise<string | null> {
+    if (this.migrationInFlight) {
+      return this.migrationInFlight;
+    }
+
     try {
       const keytar = await import('keytar');
-      return await keytar.getPassword(this.SERVICE_NAME, this.ACCOUNT_NAME);
+      const stored = await keytar.getPassword(this.SERVICE_NAME, this.ACCOUNT_NAME);
+      if (stored) return stored;
+
+      const migrationBlocked =
+        (await keytar.getPassword(this.SERVICE_NAME, this.MIGRATION_BLOCK_ACCOUNT)) === '1';
+      if (migrationBlocked) return null;
+
+      if (this.migrationInFlight) {
+        return this.migrationInFlight;
+      }
+
+      if (this.migrationAttempted) return null;
     } catch (error) {
       console.error('Failed to retrieve token:', error);
       return null;
     }
+
+    const inFlight = this.migrateTokenFromGHCLI();
+    this.migrationInFlight = inFlight;
+
+    try {
+      return await inFlight;
+    } finally {
+      if (this.migrationInFlight === inFlight) {
+        this.migrationInFlight = null;
+      }
+    }
+  }
+
+  private async migrateTokenFromGHCLI(): Promise<string | null> {
+    return this.withAuthStateLock(async () => {
+      if (this.migrationAttempted) return null;
+      this.migrationAttempted = true;
+
+      try {
+        const keytar = await import('keytar');
+
+        const migrationBlockedBeforeRead =
+          (await keytar.getPassword(this.SERVICE_NAME, this.MIGRATION_BLOCK_ACCOUNT)) === '1';
+        if (migrationBlockedBeforeRead) return null;
+
+        const { stdout } = await execAsync('gh auth token', { encoding: 'utf8' });
+        const token = String(stdout).trim();
+        if (!token) return null;
+
+        // Re-check auth state while still serialized with logout().
+        const stored = await keytar.getPassword(this.SERVICE_NAME, this.ACCOUNT_NAME);
+        if (stored) return stored;
+
+        const migrationBlockedAfterRead =
+          (await keytar.getPassword(this.SERVICE_NAME, this.MIGRATION_BLOCK_ACCOUNT)) === '1';
+        if (migrationBlockedAfterRead) return null;
+
+        try {
+          await this.storeToken(token, 'migration');
+        } catch (error) {
+          console.warn('Failed to persist migrated gh CLI token to keychain:', error);
+        }
+        return token;
+      } catch {
+        return null;
+      }
+    });
   }
   // -----------------------------------------------------------------------
   // Repo events API — efficient polling with ETag caching

src/test/main/GitHubService.test.ts

@@ -7,6 +7,9 @@ let issueSearchStdout = '[]';
 let prListStdout = '[]';
 let repoViewStdout = 'generalaction/emdash';
 let prCountStdout = '0';
+let blockTokenStore = false;
+let releaseTokenStore: (() => void) | null = null;
+let notifyTokenStoreStarted: (() => void) | null = null;
 
 vi.mock('child_process', () => {
   const execImpl = (command: string, options?: any, callback?: any) => {
@@ -70,9 +73,30 @@ vi.mock('child_process', () => {
   };
 });
 
-const setPasswordMock = vi.fn().mockResolvedValue(undefined);
-const getPasswordMock = vi.fn().mockResolvedValue(null);
-const deletePasswordMock = vi.fn().mockResolvedValue(undefined);
+const keychain = new Map<string, string>();
+
+const keyFor = (serviceName: string, accountName: string) => `${serviceName}:${accountName}`;
+
+const setPasswordMock = vi.fn(
+  async (serviceName: string, accountName: string, password: string) => {
+    if (accountName === 'github-token') {
+      notifyTokenStoreStarted?.();
+      if (blockTokenStore) {
+        await new Promise<void>((resolve) => {
+          releaseTokenStore = resolve;
+        });
+      }
+    }
+
+    keychain.set(keyFor(serviceName, accountName), password);
+  }
+);
+const getPasswordMock = vi.fn(async (serviceName: string, accountName: string) => {
+  return keychain.get(keyFor(serviceName, accountName)) ?? null;
+});
+const deletePasswordMock = vi.fn(async (serviceName: string, accountName: string) => {
+  keychain.delete(keyFor(serviceName, accountName));
+});
 const fetchMock = vi.fn();
 
 vi.mock('keytar', () => {
@@ -98,10 +122,33 @@ describe('GitHubService.isAuthenticated', () => {
     prListStdout = '[]';
     repoViewStdout = 'generalaction/emdash';
     prCountStdout = '0';
+    blockTokenStore = false;
+    releaseTokenStore = null;
+    notifyTokenStoreStarted = null;
+    keychain.clear();
     setPasswordMock.mockClear();
     getPasswordMock.mockClear();
     deletePasswordMock.mockClear();
-    getPasswordMock.mockResolvedValue(null);
+    setPasswordMock.mockImplementation(
+      async (serviceName: string, accountName: string, password: string) => {
+        if (accountName === 'github-token') {
+          notifyTokenStoreStarted?.();
+          if (blockTokenStore) {
+            await new Promise<void>((resolve) => {
+              releaseTokenStore = resolve;
+            });
+          }
+        }
+
+        keychain.set(keyFor(serviceName, accountName), password);
+      }
+    );
+    getPasswordMock.mockImplementation(async (serviceName: string, accountName: string) => {
+      return keychain.get(keyFor(serviceName, accountName)) ?? null;
+    });
+    deletePasswordMock.mockImplementation(async (serviceName: string, accountName: string) => {
+      keychain.delete(keyFor(serviceName, accountName));
+    });
     fetchMock.mockReset();
     fetchMock.mockResolvedValue({
       ok: true,
@@ -121,14 +168,65 @@ describe('GitHubService.isAuthenticated', () => {
     vi.unstubAllGlobals();
   });
 
-  it('does not treat a global gh login as authenticated without an Emdash token', async () => {
+  it('migrates a token from the gh CLI into the keychain when Emdash has none', async () => {
     const service = new GitHubService();
 
     const result = await service.isAuthenticated();
 
+    expect(result).toBe(true);
+    expect(execCalls.find((cmd) => cmd.startsWith('gh auth token'))).toBeDefined();
+    expect(setPasswordMock).toHaveBeenCalledWith('emdash-github', 'github-token', 'gho_mocktoken');
+    expect(fetchMock).toHaveBeenCalledWith(
+      'https://api.github.com/user',
+      expect.objectContaining({
+        headers: expect.objectContaining({
+          Authorization: 'Bearer gho_mocktoken',
+        }),
+      })
+    );
+  });
+
+  it('does not persist a migrated token after logout races with migration', async () => {
+    const service = new GitHubService();
+
+    blockTokenStore = true;
+    const tokenStoreStarted = new Promise<void>((resolve) => {
+      notifyTokenStoreStarted = () => {
+        notifyTokenStoreStarted = null;
+        resolve();
+      };
+    });
+
+    const migration = service.getStoredToken();
+    await tokenStoreStarted;
+
+    const logout = service.logout();
+
+    expect(deletePasswordMock).not.toHaveBeenCalled();
+    expect(keychain.get('emdash-github:github-migration-blocked')).toBeUndefined();
+
+    releaseTokenStore?.();
+
+    expect(await migration).toBe('gho_mocktoken');
+    await logout;
+
+    expect(keychain.get('emdash-github:github-token') ?? null).toBeNull();
+    expect(keychain.get('emdash-github:github-migration-blocked')).toBe('1');
+  });
+
+  it('does not auto-migrate from the gh CLI after the user has logged out', async () => {
+    const service = new GitHubService();
+    await service.logout();
+
+    execCalls.length = 0;
+    setPasswordMock.mockClear();
+
+    // New service instance simulates an app restart.
+    const serviceAfterRestart = new GitHubService();
+    const result = await serviceAfterRestart.isAuthenticated();
+
     expect(result).toBe(false);
-    expect(fetchMock).not.toHaveBeenCalled();
-    expect(execCalls.find((cmd) => cmd.startsWith('gh auth status'))).toBeUndefined();
+    expect(execCalls.find((cmd) => cmd.startsWith('gh auth token'))).toBeUndefined();
     expect(setPasswordMock).not.toHaveBeenCalled();
   });
 
@@ -155,6 +253,7 @@ describe('GitHubService.isAuthenticated', () => {
     await service.logout();
 
     expect(deletePasswordMock).toHaveBeenCalledWith('emdash-github', 'github-token');
+    expect(setPasswordMock).toHaveBeenCalledWith('emdash-github', 'github-migration-blocked', '1');
     expect(execCalls).toEqual([]);
   });