diff --git a/packages/core/src/types/session.ts b/packages/core/src/types/session.ts index a93902b1..9cdaea7a 100644 --- a/packages/core/src/types/session.ts +++ b/packages/core/src/types/session.ts @@ -35,6 +35,10 @@ export interface Session { archived?: boolean; claudeSessionId?: string; executionMode?: 'plan' | 'execute'; + currentBranch?: string; + ownerRepo?: string; // Main repo (upstream in fork, origin otherwise) + isFork?: boolean; + originOwnerRepo?: string; // Origin repo (only for fork workflow) } export interface SessionUpdate { @@ -50,6 +54,10 @@ export interface SessionUpdate { skipContinueNext?: boolean; toolType?: 'claude' | 'codex' | 'gemini' | 'none'; executionMode?: 'plan' | 'execute'; + currentBranch?: string; + ownerRepo?: string; + isFork?: boolean; + originOwnerRepo?: string; } export interface GitStatus { diff --git a/packages/desktop/src/infrastructure/database/database.ts b/packages/desktop/src/infrastructure/database/database.ts index 1bd5ed50..b3bf3a8c 100644 --- a/packages/desktop/src/infrastructure/database/database.ts +++ b/packages/desktop/src/infrastructure/database/database.ts @@ -125,6 +125,7 @@ export class DatabaseService { const migrations: Migration[] = [ { version: 1, name: 'add_execution_mode', run: () => this.migrate_001_add_execution_mode() }, + { version: 2, name: 'add_repo_info_cache', run: () => this.migrate_002_add_repo_info_cache() }, // Future migrations go here ]; @@ -187,6 +188,35 @@ export class DatabaseService { this.db.prepare("ALTER TABLE sessions ADD COLUMN execution_mode TEXT DEFAULT 'execute'").run(); } } + + // Migration 002: Add repo info cache columns to sessions table + private migrate_002_add_repo_info_cache(): void { + interface SqliteTableInfo { + cid: number; + name: string; + type: string; + notnull: number; + dflt_value: unknown; + pk: number; + } + + const tableInfo = this.db.prepare("PRAGMA table_info(sessions)").all() as SqliteTableInfo[]; + const existingColumns = new Set(tableInfo.map((col: SqliteTableInfo) => col.name)); + + if (!existingColumns.has('current_branch')) { + this.db.prepare("ALTER TABLE sessions ADD COLUMN current_branch TEXT").run(); + } + if (!existingColumns.has('owner_repo')) { + this.db.prepare("ALTER TABLE sessions ADD COLUMN owner_repo TEXT").run(); + } + if (!existingColumns.has('is_fork')) { + this.db.prepare("ALTER TABLE sessions ADD COLUMN is_fork BOOLEAN DEFAULT 0").run(); + } + if (!existingColumns.has('origin_owner_repo')) { + this.db.prepare("ALTER TABLE sessions ADD COLUMN origin_owner_repo TEXT").run(); + } + } + private ensureSessionsTableColumns(): void { interface SqliteTableInfo { cid: number; @@ -221,6 +251,22 @@ export class DatabaseService { if (!existing.has('execution_mode')) { addColumnBestEffort("ALTER TABLE sessions ADD COLUMN execution_mode TEXT DEFAULT 'execute'", 'execution_mode'); } + + if (!existing.has('current_branch')) { + addColumnBestEffort("ALTER TABLE sessions ADD COLUMN current_branch TEXT", 'current_branch'); + } + + if (!existing.has('owner_repo')) { + addColumnBestEffort("ALTER TABLE sessions ADD COLUMN owner_repo TEXT", 'owner_repo'); + } + + if (!existing.has('is_fork')) { + addColumnBestEffort("ALTER TABLE sessions ADD COLUMN is_fork BOOLEAN DEFAULT 0", 'is_fork'); + } + + if (!existing.has('origin_owner_repo')) { + addColumnBestEffort("ALTER TABLE sessions ADD COLUMN origin_owner_repo TEXT", 'origin_owner_repo'); + } } private migrateTimelineMaskedPrompts(): void { @@ -866,6 +912,22 @@ export class DatabaseService { updates.push('execution_mode = ?'); values.push(data.execution_mode); } + if (data.current_branch !== undefined) { + updates.push('current_branch = ?'); + values.push(data.current_branch); + } + if (data.owner_repo !== undefined) { + updates.push('owner_repo = ?'); + values.push(data.owner_repo); + } + if (data.is_fork !== undefined) { + updates.push('is_fork = ?'); + values.push(data.is_fork ? 1 : 0); + } + if (data.origin_owner_repo !== undefined) { + updates.push('origin_owner_repo = ?'); + values.push(data.origin_owner_repo); + } if (updates.length === 0) { return this.getSession(id); diff --git a/packages/desktop/src/infrastructure/database/migrations/add_repo_info_cache.sql b/packages/desktop/src/infrastructure/database/migrations/add_repo_info_cache.sql new file mode 100644 index 00000000..ba0ce835 --- /dev/null +++ b/packages/desktop/src/infrastructure/database/migrations/add_repo_info_cache.sql @@ -0,0 +1,5 @@ +-- Add repo info cache fields to sessions table +ALTER TABLE sessions ADD COLUMN current_branch TEXT; +ALTER TABLE sessions ADD COLUMN owner_repo TEXT; +ALTER TABLE sessions ADD COLUMN is_fork BOOLEAN DEFAULT 0; +ALTER TABLE sessions ADD COLUMN origin_owner_repo TEXT; diff --git a/packages/desktop/src/infrastructure/database/models.ts b/packages/desktop/src/infrastructure/database/models.ts index a8a6871c..ba8e4c5c 100644 --- a/packages/desktop/src/infrastructure/database/models.ts +++ b/packages/desktop/src/infrastructure/database/models.ts @@ -35,6 +35,10 @@ export interface Session { skip_continue_next?: boolean | null; archived?: boolean | null; execution_mode?: 'plan' | 'execute' | null; + current_branch?: string | null; + owner_repo?: string | null; + is_fork?: boolean | null; + origin_owner_repo?: string | null; } export interface Project { @@ -127,6 +131,10 @@ export interface UpdateSessionData { base_branch?: string | null; archived?: boolean; execution_mode?: 'plan' | 'execute'; + current_branch?: string | null; + owner_repo?: string | null; + is_fork?: boolean | null; + origin_owner_repo?: string | null; } export interface PromptMarker { diff --git a/packages/desktop/src/infrastructure/ipc/__tests__/gitCacheHandlers.test.ts b/packages/desktop/src/infrastructure/ipc/__tests__/gitCacheHandlers.test.ts new file mode 100644 index 00000000..629edbb3 --- /dev/null +++ b/packages/desktop/src/infrastructure/ipc/__tests__/gitCacheHandlers.test.ts @@ -0,0 +1,281 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import type { IpcMain } from 'electron'; +import { registerGitHandlers } from '../git'; +import type { AppServices } from '../types'; + +// Mock IpcMain +class MockIpcMain { + private handlers: Map unknown> = new Map(); + + handle(channel: string, listener: (event: unknown, ...args: unknown[]) => unknown) { + this.handlers.set(channel, listener); + } + + async invoke(channel: string, ...args: unknown[]) { + const handler = this.handlers.get(channel); + if (!handler) { + throw new Error(`No handler registered for channel: ${channel}`); + } + return handler({}, ...args); + } + + clear() { + this.handlers.clear(); + } +} + +// Mock GitExecutor run result +interface MockRunResult { + exitCode: number; + stdout: string; + stderr: string; +} + +describe('Git IPC Handlers - Repo Info Cache', () => { + let mockIpcMain: MockIpcMain; + let mockGitExecutor: { run: ReturnType }; + let mockSessionManager: { getSession: ReturnType; db: { getSession: ReturnType; updateSession: ReturnType } }; + let mockServices: AppServices; + + beforeEach(() => { + mockIpcMain = new MockIpcMain(); + + mockGitExecutor = { + run: vi.fn(), + }; + + mockSessionManager = { + getSession: vi.fn(), + db: { + getSession: vi.fn().mockReturnValue(null), // Return null by default (cache miss) + updateSession: vi.fn(), + }, + }; + + mockServices = { + gitExecutor: mockGitExecutor, + sessionManager: mockSessionManager, + gitStagingManager: { stageHunk: vi.fn(), restoreHunk: vi.fn() }, + gitStatusManager: { refreshSessionGitStatus: vi.fn() }, + gitDiffManager: { + getDiff: vi.fn(), + getCommitHistory: vi.fn(), + hasChanges: vi.fn(), + getWorkingDiffStatsQuick: vi.fn(), + }, + worktreeManager: {}, + configManager: {}, + } as unknown as AppServices; + + registerGitHandlers(mockIpcMain as unknown as IpcMain, mockServices); + }); + + describe('sessions:get-remote-pull-request with cache', () => { + const sessionId = 'test-session-123'; + const worktreePath = '/path/to/worktree'; + + beforeEach(() => { + mockSessionManager.getSession.mockReturnValue({ worktreePath }); + }); + + it('should use cached branch and repo info', async () => { + // Set up cache hit + mockSessionManager.db.getSession.mockReturnValue({ + current_branch: 'feature-branch', + owner_repo: 'owner/repo', + is_fork: false, + origin_owner_repo: 'owner/repo', + }); + + mockGitExecutor.run + // 1. git remote get-url upstream (will fail) + .mockResolvedValueOnce({ + exitCode: 1, + stdout: '', + stderr: 'fatal: No such remote', + } as MockRunResult) + // 2. git remote get-url origin + .mockResolvedValueOnce({ + exitCode: 0, + stdout: 'git@github.com:owner/repo.git\n', + stderr: '', + } as MockRunResult) + // 3. gh pr view + .mockResolvedValueOnce({ + exitCode: 0, + stdout: JSON.stringify({ number: 123, url: 'https://github.com/owner/repo/pull/123', state: 'OPEN', isDraft: false }), + stderr: '', + } as MockRunResult); + + const result = await mockIpcMain.invoke('sessions:get-remote-pull-request', sessionId); + + expect(result).toEqual({ + success: true, + data: { number: 123, url: 'https://github.com/owner/repo/pull/123', state: 'open' }, + }); + + // Verify cache was used - should NOT call git branch or fetch repo info + expect(mockGitExecutor.run).toHaveBeenCalledTimes(3); + // Verify updateSession was not called (cache hit) + expect(mockSessionManager.db.updateSession).not.toHaveBeenCalled(); + }); + + it('should cache repo info on first call', async () => { + // Cache miss - will call fetchAndCacheRepoInfo + mockGitExecutor.run + // 1. git branch --show-current + .mockResolvedValueOnce({ + exitCode: 0, + stdout: 'feature-branch\n', + stderr: '', + } as MockRunResult) + // 2. git remote get-url origin (from fetchAndCacheRepoInfo) + .mockResolvedValueOnce({ + exitCode: 0, + stdout: 'git@github.com:owner/repo.git\n', + stderr: '', + } as MockRunResult) + // 3. git remote get-url upstream (from fetchAndCacheRepoInfo) + .mockResolvedValueOnce({ + exitCode: 1, + stdout: '', + stderr: 'fatal: No such remote', + } as MockRunResult) + // 4. git remote get-url upstream (from PR search loop) + .mockResolvedValueOnce({ + exitCode: 1, + stdout: '', + stderr: 'fatal: No such remote', + } as MockRunResult) + // 5. git remote get-url origin (from PR search loop) + .mockResolvedValueOnce({ + exitCode: 0, + stdout: 'git@github.com:owner/repo.git\n', + stderr: '', + } as MockRunResult) + // 6. gh pr view + .mockResolvedValueOnce({ + exitCode: 0, + stdout: JSON.stringify({ number: 123, url: 'https://github.com/owner/repo/pull/123', state: 'OPEN', isDraft: false }), + stderr: '', + } as MockRunResult); + + const result = await mockIpcMain.invoke('sessions:get-remote-pull-request', sessionId); + + expect(result).toEqual({ + success: true, + data: { number: 123, url: 'https://github.com/owner/repo/pull/123', state: 'open' }, + }); + + // Verify updateSession was called to cache the data + expect(mockSessionManager.db.updateSession).toHaveBeenCalledWith(sessionId, { + current_branch: 'feature-branch', + owner_repo: 'owner/repo', + is_fork: false, + origin_owner_repo: 'owner/repo', + }); + }); + + it('should use cached data for fork workflow', async () => { + // Set up cache with fork info + mockSessionManager.db.getSession.mockReturnValue({ + current_branch: 'feature-branch', + owner_repo: 'upstream-owner/repo', + is_fork: true, + origin_owner_repo: 'fork-owner/repo', + }); + + mockGitExecutor.run + // 1. git remote get-url upstream (will succeed for fork) + .mockResolvedValueOnce({ + exitCode: 0, + stdout: 'git@github.com:upstream-owner/repo.git\n', + stderr: '', + } as MockRunResult) + // 2. gh pr view (with fork-owner:branch format) + .mockResolvedValueOnce({ + exitCode: 0, + stdout: JSON.stringify({ number: 456, url: 'https://github.com/upstream-owner/repo/pull/456', state: 'OPEN', isDraft: false }), + stderr: '', + } as MockRunResult); + + const result = await mockIpcMain.invoke('sessions:get-remote-pull-request', sessionId); + + expect(result).toEqual({ + success: true, + data: { number: 456, url: 'https://github.com/upstream-owner/repo/pull/456', state: 'open' }, + }); + + // Verify the gh command used the fork-owner:branch format + const ghCall = mockGitExecutor.run.mock.calls[1]; + expect(ghCall[0].argv).toContain('fork-owner:feature-branch'); + }); + }); + + describe('sessions:get-pr-remote-commits with cache', () => { + const sessionId = 'test-session-123'; + const worktreePath = '/path/to/worktree'; + + beforeEach(() => { + mockSessionManager.getSession.mockReturnValue({ worktreePath }); + mockSessionManager.db.getSession.mockReturnValue(null); + }); + + it('should use cached branch name', async () => { + // Set up cache hit + mockSessionManager.db.getSession.mockReturnValue({ + current_branch: 'feature-branch', + }); + + mockGitExecutor.run + // 1. git config branch.feature-branch.pushRemote + .mockResolvedValueOnce({ + exitCode: 1, + stdout: '', + stderr: '', + } as MockRunResult) + // 2. git config branch.feature-branch.remote + .mockResolvedValueOnce({ + exitCode: 0, + stdout: 'origin\n', + stderr: '', + } as MockRunResult) + // 3. git fetch origin feature-branch + .mockResolvedValueOnce({ + exitCode: 0, + stdout: '', + stderr: '', + } as MockRunResult) + // 4. git show-ref + .mockResolvedValueOnce({ + exitCode: 0, + stdout: '', + stderr: '', + } as MockRunResult) + // 5. git rev-list (local ahead) + .mockResolvedValueOnce({ + exitCode: 0, + stdout: '2\n', + stderr: '', + } as MockRunResult) + // 6. git rev-list (remote ahead) + .mockResolvedValueOnce({ + exitCode: 0, + stdout: '1\n', + stderr: '', + } as MockRunResult); + + const result = await mockIpcMain.invoke('sessions:get-pr-remote-commits', sessionId); + + expect(result).toEqual({ + success: true, + data: { ahead: 2, behind: 1, branch: 'feature-branch' }, + }); + + // Verify cache was used - should NOT call git branch + const firstCall = mockGitExecutor.run.mock.calls[0]; + expect(firstCall[0].argv).not.toContain('branch'); + expect(firstCall[0].argv).toContain('config'); + }); + }); +}); diff --git a/packages/desktop/src/infrastructure/ipc/__tests__/gitHandlers.test.ts b/packages/desktop/src/infrastructure/ipc/__tests__/gitHandlers.test.ts index 90ecb12d..2c35b4cf 100644 --- a/packages/desktop/src/infrastructure/ipc/__tests__/gitHandlers.test.ts +++ b/packages/desktop/src/infrastructure/ipc/__tests__/gitHandlers.test.ts @@ -34,7 +34,7 @@ interface MockRunResult { describe('Git IPC Handlers - Remote Pull Request', () => { let mockIpcMain: MockIpcMain; let mockGitExecutor: { run: ReturnType }; - let mockSessionManager: { getSession: ReturnType }; + let mockSessionManager: { getSession: ReturnType; db: { getSession: ReturnType; updateSession: ReturnType } }; let mockServices: AppServices; beforeEach(() => { @@ -46,6 +46,10 @@ describe('Git IPC Handlers - Remote Pull Request', () => { mockSessionManager = { getSession: vi.fn(), + db: { + getSession: vi.fn().mockReturnValue(null), // Return null by default (cache miss) + updateSession: vi.fn(), + }, }; mockServices = { @@ -72,6 +76,8 @@ describe('Git IPC Handlers - Remote Pull Request', () => { beforeEach(() => { mockSessionManager.getSession.mockReturnValue({ worktreePath }); + // Reset db.getSession to return null (cache miss) by default for each test + mockSessionManager.db.getSession.mockReturnValue(null); }); it('should return null when session has no worktreePath', async () => { @@ -91,32 +97,28 @@ describe('Git IPC Handlers - Remote Pull Request', () => { }); it('should parse SSH remote URL and fetch PR with --repo flag', async () => { + // Use cached data to avoid calling fetchAndCacheRepoInfo + mockSessionManager.db.getSession.mockReturnValue({ + current_branch: 'feature-branch', + owner_repo: 'BohuTANG/blog-hexo', + is_fork: false, + origin_owner_repo: 'BohuTANG/blog-hexo', + }); + mockGitExecutor.run - // 1. git branch --show-current - .mockResolvedValueOnce({ - exitCode: 0, - stdout: 'feature-branch\n', - stderr: '', - } as MockRunResult) - // 2. git remote get-url origin (to extract owner) - .mockResolvedValueOnce({ - exitCode: 0, - stdout: 'git@github.com:BohuTANG/blog-hexo.git\n', - stderr: '', - } as MockRunResult) - // 3. git remote get-url upstream (first in loop, will fail) + // 1. git remote get-url upstream (first in PR search loop, will fail) .mockResolvedValueOnce({ exitCode: 1, stdout: '', stderr: 'fatal: No such remote', } as MockRunResult) - // 4. git remote get-url origin (second in loop) + // 2. git remote get-url origin (second in PR search loop) .mockResolvedValueOnce({ exitCode: 0, stdout: 'git@github.com:BohuTANG/blog-hexo.git\n', stderr: '', } as MockRunResult) - // 5. gh pr view + // 3. gh pr view .mockResolvedValueOnce({ exitCode: 0, stdout: JSON.stringify({ number: 123, url: 'https://github.com/BohuTANG/blog-hexo/pull/123', state: 'OPEN', isDraft: false }), @@ -131,39 +133,31 @@ describe('Git IPC Handlers - Remote Pull Request', () => { }); // Verify gh pr view was called with --repo and branch - const ghPrViewCall = mockGitExecutor.run.mock.calls[4]; // Changed from index 2 to 4 + const ghPrViewCall = mockGitExecutor.run.mock.calls[2]; expect(ghPrViewCall[0].argv).toContain('--repo'); expect(ghPrViewCall[0].argv).toContain('BohuTANG/blog-hexo'); expect(ghPrViewCall[0].argv).toContain('feature-branch'); }); it('should parse HTTPS remote URL and fetch PR with --repo flag', async () => { + mockSessionManager.db.getSession.mockReturnValue({ + current_branch: 'main', + owner_repo: 'owner/repo', + is_fork: false, + origin_owner_repo: 'owner/repo', + }); + mockGitExecutor.run - // 1. git branch --show-current - .mockResolvedValueOnce({ - exitCode: 0, - stdout: 'main\n', - stderr: '', - } as MockRunResult) - // 2. git remote get-url origin (to extract owner) - .mockResolvedValueOnce({ - exitCode: 0, - stdout: 'https://github.com/owner/repo.git\n', - stderr: '', - } as MockRunResult) - // 3. git remote get-url upstream (first in loop, will fail) .mockResolvedValueOnce({ exitCode: 1, stdout: '', stderr: 'fatal: No such remote', } as MockRunResult) - // 4. git remote get-url origin (second in loop) .mockResolvedValueOnce({ exitCode: 0, stdout: 'https://github.com/owner/repo.git\n', stderr: '', } as MockRunResult) - // 5. gh pr view .mockResolvedValueOnce({ exitCode: 0, stdout: JSON.stringify({ number: 42, url: 'https://github.com/owner/repo/pull/42', state: 'MERGED', isDraft: false }), @@ -177,38 +171,29 @@ describe('Git IPC Handlers - Remote Pull Request', () => { data: { number: 42, url: 'https://github.com/owner/repo/pull/42', state: 'merged' }, }); - // Verify --repo contains owner/repo from HTTPS URL - const ghPrViewCall = mockGitExecutor.run.mock.calls[4]; // Changed from index 2 to 4 + const ghPrViewCall = mockGitExecutor.run.mock.calls[2]; expect(ghPrViewCall[0].argv).toContain('owner/repo'); }); it('should return null when no PR exists for the branch', async () => { + mockSessionManager.db.getSession.mockReturnValue({ + current_branch: 'new-branch', + owner_repo: 'owner/repo', + is_fork: false, + origin_owner_repo: 'owner/repo', + }); + mockGitExecutor.run - // 1. git branch --show-current - .mockResolvedValueOnce({ - exitCode: 0, - stdout: 'new-branch\n', - stderr: '', - } as MockRunResult) - // 2. git remote get-url origin (to extract owner) - .mockResolvedValueOnce({ - exitCode: 0, - stdout: 'git@github.com:owner/repo.git\n', - stderr: '', - } as MockRunResult) - // 3. git remote get-url upstream (first in loop, will fail) .mockResolvedValueOnce({ exitCode: 1, stdout: '', stderr: 'fatal: No such remote', } as MockRunResult) - // 4. git remote get-url origin (second in loop) .mockResolvedValueOnce({ exitCode: 0, stdout: 'git@github.com:owner/repo.git\n', stderr: '', } as MockRunResult) - // 5. gh pr view (returns exit code 1 when no PR found) .mockResolvedValueOnce({ exitCode: 1, stdout: '', @@ -221,31 +206,13 @@ describe('Git IPC Handlers - Remote Pull Request', () => { }); it('should return null when no remote is available', async () => { - mockGitExecutor.run - // 1. git branch --show-current - .mockResolvedValueOnce({ - exitCode: 0, - stdout: 'feature\n', - stderr: '', - } as MockRunResult) - // 2. git remote get-url origin (to extract owner, fails) - .mockResolvedValueOnce({ - exitCode: 1, - stdout: '', - stderr: 'fatal: No such remote', - } as MockRunResult) - // 3. git remote get-url upstream (first in loop, fails) - .mockResolvedValueOnce({ - exitCode: 1, - stdout: '', - stderr: 'fatal: No such remote', - } as MockRunResult) - // 4. git remote get-url origin (second in loop, fails) - .mockResolvedValueOnce({ - exitCode: 1, - stdout: '', - stderr: 'fatal: No such remote', - } as MockRunResult); + // Use cache with no owner_repo (simulating no remotes) + mockSessionManager.db.getSession.mockReturnValue({ + current_branch: 'feature', + owner_repo: null, + is_fork: false, + origin_owner_repo: null, + }); const result = await mockIpcMain.invoke('sessions:get-remote-pull-request', sessionId); @@ -256,26 +223,33 @@ describe('Git IPC Handlers - Remote Pull Request', () => { }); it('should handle non-GitHub remotes gracefully', async () => { + // Cache miss - need to provide full fetchAndCacheRepoInfo mocks mockGitExecutor.run - // 1. git branch --show-current + // 1. git branch --show-current (from fetchAndCacheRepoInfo) .mockResolvedValueOnce({ exitCode: 0, stdout: 'main\n', stderr: '', } as MockRunResult) - // 2. git remote get-url origin (to extract owner, GitLab URL won't match) + // 2. git remote get-url origin (from fetchAndCacheRepoInfo) .mockResolvedValueOnce({ exitCode: 0, stdout: 'git@gitlab.com:owner/repo.git\n', // GitLab, not GitHub stderr: '', } as MockRunResult) - // 3. git remote get-url upstream (first in loop, fails) + // 3. git remote get-url upstream (from fetchAndCacheRepoInfo) .mockResolvedValueOnce({ exitCode: 1, stdout: '', stderr: 'fatal: No such remote', } as MockRunResult) - // 4. git remote get-url origin (second in loop, GitLab URL won't match regex) + // 4. git remote get-url upstream (first in PR search loop, fails) + .mockResolvedValueOnce({ + exitCode: 1, + stdout: '', + stderr: 'fatal: No such remote', + } as MockRunResult) + // 5. git remote get-url origin (second in PR search loop, GitLab URL won't match regex) .mockResolvedValueOnce({ exitCode: 0, stdout: 'git@gitlab.com:owner/repo.git\n', @@ -330,31 +304,37 @@ describe('Git IPC Handlers - Remote Pull Request', () => { it('should handle malformed JSON response gracefully', async () => { mockGitExecutor.run - // 1. git branch --show-current + // 1. git branch --show-current (from fetchAndCacheRepoInfo) .mockResolvedValueOnce({ exitCode: 0, stdout: 'branch\n', stderr: '', } as MockRunResult) - // 2. git remote get-url origin (to extract owner) + // 2. git remote get-url origin (from fetchAndCacheRepoInfo) .mockResolvedValueOnce({ exitCode: 0, stdout: 'git@github.com:owner/repo.git\n', stderr: '', } as MockRunResult) - // 3. git remote get-url upstream (first in loop, fails) + // 3. git remote get-url upstream (from fetchAndCacheRepoInfo) .mockResolvedValueOnce({ exitCode: 1, stdout: '', stderr: 'fatal: No such remote', } as MockRunResult) - // 4. git remote get-url origin (second in loop) + // 4. git remote get-url upstream (first in PR search loop, fails) + .mockResolvedValueOnce({ + exitCode: 1, + stdout: '', + stderr: 'fatal: No such remote', + } as MockRunResult) + // 5. git remote get-url origin (second in PR search loop) .mockResolvedValueOnce({ exitCode: 0, stdout: 'git@github.com:owner/repo.git\n', stderr: '', } as MockRunResult) - // 5. gh pr view (returns malformed JSON) + // 6. gh pr view (returns malformed JSON) .mockResolvedValueOnce({ exitCode: 0, stdout: 'not valid json', @@ -431,7 +411,7 @@ describe('Git IPC Handlers - Remote Pull Request', () => { describe('Git IPC Handlers - Commit URL', () => { let mockIpcMain: MockIpcMain; let mockGitExecutor: { run: ReturnType }; - let mockSessionManager: { getSession: ReturnType }; + let mockSessionManager: { getSession: ReturnType; db: { getSession: ReturnType; updateSession: ReturnType } }; let mockServices: AppServices; beforeEach(() => { @@ -443,6 +423,10 @@ describe('Git IPC Handlers - Commit URL', () => { mockSessionManager = { getSession: vi.fn(), + db: { + getSession: vi.fn().mockReturnValue(null), + updateSession: vi.fn(), + }, }; mockServices = { @@ -497,7 +481,7 @@ describe('Git IPC Handlers - Commit URL', () => { describe('Git IPC Handlers - Branch Sync Status', () => { let mockIpcMain: MockIpcMain; let mockGitExecutor: { run: ReturnType }; - let mockSessionManager: { getSession: ReturnType }; + let mockSessionManager: { getSession: ReturnType; db: { getSession: ReturnType; updateSession: ReturnType } }; let mockServices: AppServices; beforeEach(() => { @@ -509,6 +493,10 @@ describe('Git IPC Handlers - Branch Sync Status', () => { mockSessionManager = { getSession: vi.fn(), + db: { + getSession: vi.fn().mockReturnValue(null), + updateSession: vi.fn(), + }, }; mockServices = { @@ -862,6 +850,7 @@ describe('Git IPC Handlers - Branch Sync Status', () => { beforeEach(() => { mockSessionManager.getSession.mockReturnValue({ worktreePath }); + mockSessionManager.db.getSession.mockReturnValue(null); }); it('should return error when session has no worktreePath', async () => { @@ -881,44 +870,43 @@ describe('Git IPC Handlers - Branch Sync Status', () => { }); it('should return ahead and behind counts', async () => { + // Use cached branch data + mockSessionManager.db.getSession.mockReturnValue({ + current_branch: 'feature-branch', + }); + mockGitExecutor.run - // 1. git branch --show-current - .mockResolvedValueOnce({ - exitCode: 0, - stdout: 'feature-branch\n', - stderr: '', - } as MockRunResult) - // 2. git config branch.feature-branch.pushRemote (fails) + // 1. git config branch.feature-branch.pushRemote (fails) .mockResolvedValueOnce({ exitCode: 1, stdout: '', stderr: '', } as MockRunResult) - // 3. git config branch.feature-branch.remote (returns origin) + // 2. git config branch.feature-branch.remote (returns origin) .mockResolvedValueOnce({ exitCode: 0, stdout: 'origin\n', stderr: '', } as MockRunResult) - // 4. git fetch origin feature-branch + // 3. git fetch origin feature-branch .mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '', } as MockRunResult) - // 5. git show-ref --verify --quiet refs/remotes/origin/feature-branch + // 4. git show-ref --verify --quiet refs/remotes/origin/feature-branch .mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '', } as MockRunResult) - // 6. git rev-list origin/feature-branch..HEAD --count (local ahead) + // 5. git rev-list origin/feature-branch..HEAD --count (local ahead) .mockResolvedValueOnce({ exitCode: 0, stdout: '3\n', stderr: '', } as MockRunResult) - // 7. git rev-list HEAD..origin/feature-branch --count (remote ahead) + // 6. git rev-list HEAD..origin/feature-branch --count (remote ahead) .mockResolvedValueOnce({ exitCode: 0, stdout: '2\n', @@ -934,6 +922,11 @@ describe('Git IPC Handlers - Branch Sync Status', () => { }); it('should return zeros when branch is synced', async () => { + // Use cached branch data + mockSessionManager.db.getSession.mockReturnValue({ + current_branch: 'main', + }); + mockGitExecutor.run // 1. git branch --show-current .mockResolvedValueOnce({ @@ -987,32 +980,31 @@ describe('Git IPC Handlers - Branch Sync Status', () => { }); it('should return zeros when remote branch does not exist', async () => { + // Use cached branch name + mockSessionManager.db.getSession.mockReturnValue({ + current_branch: 'new-branch', + }); + mockGitExecutor.run - // 1. git branch --show-current - .mockResolvedValueOnce({ - exitCode: 0, - stdout: 'new-branch\n', - stderr: '', - } as MockRunResult) - // 2. git config branch.new-branch.pushRemote (fails) + // 1. git config branch.new-branch.pushRemote (fails) .mockResolvedValueOnce({ exitCode: 1, stdout: '', stderr: '', } as MockRunResult) - // 3. git config branch.new-branch.remote (fails, not set up yet) + // 2. git config branch.new-branch.remote (fails, not set up yet) .mockResolvedValueOnce({ exitCode: 1, stdout: '', stderr: '', } as MockRunResult) - // 4. git fetch origin new-branch (may fail for non-existent branch) + // 3. git fetch origin new-branch (may fail for non-existent branch) .mockResolvedValueOnce({ exitCode: 1, stdout: '', stderr: 'fatal: could not fetch', } as MockRunResult) - // 5. git show-ref --verify --quiet refs/remotes/origin/new-branch (fails) + // 4. git show-ref --verify --quiet refs/remotes/origin/new-branch (fails) .mockResolvedValueOnce({ exitCode: 1, stdout: '', @@ -1093,11 +1085,25 @@ describe('Git IPC Handlers - Branch Sync Status', () => { }); it('should return null branch when in detached HEAD state', async () => { + // Cache miss - will call fetchAndCacheRepoInfo mockGitExecutor.run + // 1. git branch --show-current (from fetchAndCacheRepoInfo) .mockResolvedValueOnce({ exitCode: 0, stdout: '', // Empty for detached HEAD stderr: '', + } as MockRunResult) + // 2. git remote get-url origin (from fetchAndCacheRepoInfo) + .mockResolvedValueOnce({ + exitCode: 0, + stdout: 'git@github.com:owner/repo.git\n', + stderr: '', + } as MockRunResult) + // 3. git remote get-url upstream (from fetchAndCacheRepoInfo) + .mockResolvedValueOnce({ + exitCode: 1, + stdout: '', + stderr: 'fatal: No such remote', } as MockRunResult); const result = await mockIpcMain.invoke('sessions:get-pr-remote-commits', sessionId); @@ -1109,44 +1115,43 @@ describe('Git IPC Handlers - Branch Sync Status', () => { }); it('should handle only local commits ahead', async () => { + // Use cached branch name + mockSessionManager.db.getSession.mockReturnValue({ + current_branch: 'feature', + }); + mockGitExecutor.run - // 1. git branch --show-current - .mockResolvedValueOnce({ - exitCode: 0, - stdout: 'feature\n', - stderr: '', - } as MockRunResult) - // 2. git config branch.feature.pushRemote (fails) + // 1. git config branch.feature.pushRemote (fails) .mockResolvedValueOnce({ exitCode: 1, stdout: '', stderr: '', } as MockRunResult) - // 3. git config branch.feature.remote (returns origin) + // 2. git config branch.feature.remote (returns origin) .mockResolvedValueOnce({ exitCode: 0, stdout: 'origin\n', stderr: '', } as MockRunResult) - // 4. git fetch origin feature + // 3. git fetch origin feature .mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '', } as MockRunResult) - // 5. git show-ref --verify --quiet refs/remotes/origin/feature + // 4. git show-ref --verify --quiet refs/remotes/origin/feature .mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '', } as MockRunResult) - // 6. git rev-list --count origin/feature..HEAD (ahead) + // 5. git rev-list --count origin/feature..HEAD (ahead) .mockResolvedValueOnce({ exitCode: 0, stdout: '5\n', // 5 local commits ahead stderr: '', } as MockRunResult) - // 7. git rev-list --count HEAD..origin/feature (behind) + // 6. git rev-list --count HEAD..origin/feature (behind) .mockResolvedValueOnce({ exitCode: 0, stdout: '0\n', @@ -1162,44 +1167,43 @@ describe('Git IPC Handlers - Branch Sync Status', () => { }); it('should handle only remote commits ahead', async () => { + // Use cached branch name + mockSessionManager.db.getSession.mockReturnValue({ + current_branch: 'feature', + }); + mockGitExecutor.run - // 1. git branch --show-current - .mockResolvedValueOnce({ - exitCode: 0, - stdout: 'feature\n', - stderr: '', - } as MockRunResult) - // 2. git config branch.feature.pushRemote (fails) + // 1. git config branch.feature.pushRemote (fails) .mockResolvedValueOnce({ exitCode: 1, stdout: '', stderr: '', } as MockRunResult) - // 3. git config branch.feature.remote (returns origin) + // 2. git config branch.feature.remote (returns origin) .mockResolvedValueOnce({ exitCode: 0, stdout: 'origin\n', stderr: '', } as MockRunResult) - // 4. git fetch origin feature + // 3. git fetch origin feature .mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '', } as MockRunResult) - // 5. git show-ref --verify --quiet refs/remotes/origin/feature + // 4. git show-ref --verify --quiet refs/remotes/origin/feature .mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '', } as MockRunResult) - // 6. git rev-list --count origin/feature..HEAD (ahead) + // 5. git rev-list --count origin/feature..HEAD (ahead) .mockResolvedValueOnce({ exitCode: 0, stdout: '0\n', // 0 local commits ahead stderr: '', } as MockRunResult) - // 7. git rev-list --count HEAD..origin/feature (behind) + // 6. git rev-list --count HEAD..origin/feature (behind) .mockResolvedValueOnce({ exitCode: 0, stdout: '4\n', // 4 remote commits ahead @@ -1219,7 +1223,7 @@ describe('Git IPC Handlers - Branch Sync Status', () => { describe('Git IPC Handlers - CI Status', () => { let mockIpcMain: MockIpcMain; let mockGitExecutor: { run: ReturnType }; - let mockSessionManager: { getSession: ReturnType }; + let mockSessionManager: { getSession: ReturnType; db: { getSession: ReturnType; updateSession: ReturnType } }; let mockServices: AppServices; beforeEach(() => { @@ -1231,6 +1235,10 @@ describe('Git IPC Handlers - CI Status', () => { mockSessionManager = { getSession: vi.fn(), + db: { + getSession: vi.fn().mockReturnValue(null), + updateSession: vi.fn(), + }, }; mockServices = { diff --git a/packages/desktop/src/infrastructure/ipc/git.ts b/packages/desktop/src/infrastructure/ipc/git.ts index bcdf1970..b1106305 100644 --- a/packages/desktop/src/infrastructure/ipc/git.ts +++ b/packages/desktop/src/infrastructure/ipc/git.ts @@ -63,6 +63,96 @@ function isForkOfUpstream(originUrl: string, upstreamUrl: string): boolean { return origin.repo === upstream.repo && origin.owner !== upstream.owner; } +/** + * Fetch and cache repo information (branch, owner/repo, isFork, originOwnerRepo) for a session. + * This should be called once per session to populate the cache. + */ +async function fetchAndCacheRepoInfo( + sessionId: string, + worktreePath: string, + sessionManager: AppServices['sessionManager'], + gitExecutor: AppServices['gitExecutor'] +): Promise<{ currentBranch: string; ownerRepo: string | null; isFork: boolean; originOwnerRepo: string | null } | null> { + try { + // Get current branch + const branchRes = await gitExecutor.run({ + sessionId, + cwd: worktreePath, + argv: ['git', 'branch', '--show-current'], + op: 'read', + recordTimeline: false, + throwOnError: false, + timeoutMs: 3_000, + meta: { source: 'ipc.git', operation: 'cache-branch' }, + }); + + const currentBranch = branchRes.stdout?.trim() || ''; + + // Get origin and upstream URLs + const [originRes, upstreamRes] = await Promise.all([ + gitExecutor.run({ + sessionId, + cwd: worktreePath, + argv: ['git', 'remote', 'get-url', 'origin'], + op: 'read', + recordTimeline: false, + throwOnError: false, + timeoutMs: 3_000, + meta: { source: 'ipc.git', operation: 'cache-origin-url' }, + }), + gitExecutor.run({ + sessionId, + cwd: worktreePath, + argv: ['git', 'remote', 'get-url', 'upstream'], + op: 'read', + recordTimeline: false, + throwOnError: false, + timeoutMs: 3_000, + meta: { source: 'ipc.git', operation: 'cache-upstream-url' }, + }), + ]); + + const originUrl = originRes.exitCode === 0 ? originRes.stdout?.trim() : null; + const upstreamUrl = upstreamRes.exitCode === 0 ? upstreamRes.stdout?.trim() : null; + + // Determine owner/repo and fork status + let ownerRepo: string | null = null; + let originOwnerRepo: string | null = null; + let isFork = false; + + if (originUrl) { + originOwnerRepo = parseGitRemoteToOwnerRepo(originUrl); + } + + if (originUrl && upstreamUrl) { + // Check if this is a fork workflow + isFork = isForkOfUpstream(originUrl, upstreamUrl); + // Prefer upstream repo if it's a fork + ownerRepo = isFork ? parseGitRemoteToOwnerRepo(upstreamUrl) : originOwnerRepo; + } else if (originUrl) { + ownerRepo = originOwnerRepo; + } else if (upstreamUrl) { + ownerRepo = parseGitRemoteToOwnerRepo(upstreamUrl); + } + + // Cache the results in the session + const session = sessionManager.getSession(sessionId); + if (session) { + sessionManager.db.updateSession(sessionId, { + current_branch: currentBranch, + owner_repo: ownerRepo, + is_fork: isFork, + origin_owner_repo: originOwnerRepo, + }); + } + + return { currentBranch, ownerRepo, isFork, originOwnerRepo }; + } catch (error) { + console.error('[git.ts] Failed to fetch and cache repo info:', error); + return null; + } +} + /** * Parse git remote -v output and extract owner/repo for the origin remote. */ @@ -313,29 +403,48 @@ export function registerGitHandlers(ipcMain: IpcMain, services: AppServices): vo const session = sessionManager.getSession(sessionId); if (!session?.worktreePath) return { success: false, error: 'Session worktree not found' }; - // Run branch and tracking remote queries in parallel - const [branchRes, trackingRes] = await Promise.all([ - gitExecutor.run({ - sessionId, - cwd: session.worktreePath, - argv: ['git', 'branch', '--show-current'], - op: 'read', - meta: { source: 'ipc.git', operation: 'current-branch' }, - }), - gitExecutor.run({ + // Try to use cached data first + const dbSession = sessionManager.db.getSession(sessionId); + if (dbSession?.current_branch) { + // Determine remote name from tracking branch if available + let remoteName: string | null = null; + const trackingRes = await gitExecutor.run({ sessionId, cwd: session.worktreePath, argv: ['git', 'rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{upstream}'], op: 'read', throwOnError: false, meta: { source: 'ipc.git', operation: 'tracking-remote' }, - }), - ]); + }); - const currentBranch = branchRes.stdout.trim(); + if (trackingRes.exitCode === 0 && trackingRes.stdout.trim()) { + const trackingBranch = trackingRes.stdout.trim(); + const slashIndex = trackingBranch.indexOf('/'); + if (slashIndex > 0) { + remoteName = trackingBranch.substring(0, slashIndex); + } + } + + return { success: true, data: { currentBranch: dbSession.current_branch, remoteName } }; + } + + // Cache miss or first time - fetch and cache + const repoInfo = await fetchAndCacheRepoInfo(sessionId, session.worktreePath, sessionManager, gitExecutor); + if (!repoInfo) { + return { success: false, error: 'Failed to fetch repo info' }; + } - // Parse remote name from tracking branch (e.g., "origin/main" -> "origin") + // Get remote name from tracking branch let remoteName: string | null = null; + const trackingRes = await gitExecutor.run({ + sessionId, + cwd: session.worktreePath, + argv: ['git', 'rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{upstream}'], + op: 'read', + throwOnError: false, + meta: { source: 'ipc.git', operation: 'tracking-remote' }, + }); + if (trackingRes.exitCode === 0 && trackingRes.stdout.trim()) { const trackingBranch = trackingRes.stdout.trim(); const slashIndex = trackingBranch.indexOf('/'); @@ -344,7 +453,7 @@ export function registerGitHandlers(ipcMain: IpcMain, services: AppServices): vo } } - return { success: true, data: { currentBranch, remoteName } }; + return { success: true, data: { currentBranch: repoInfo.currentBranch, remoteName } }; } catch (error) { return { success: false, error: error instanceof Error ? error.message : 'Failed to load git commands' }; } @@ -355,42 +464,38 @@ export function registerGitHandlers(ipcMain: IpcMain, services: AppServices): vo const session = sessionManager.getSession(sessionId); if (!session?.worktreePath) return { success: false, error: 'Session worktree not found' }; - // Get current branch name - const branchRes = await gitExecutor.run({ - sessionId, - cwd: session.worktreePath, - argv: ['git', 'branch', '--show-current'], - op: 'read', - recordTimeline: false, - throwOnError: false, - timeoutMs: 3_000, - meta: { source: 'ipc.git', operation: 'get-branch' }, - }); - const branch = branchRes.stdout?.trim(); - if (!branch) return { success: true, data: null }; + // Try to use cached data first + const dbSession = sessionManager.db.getSession(sessionId); + let branch = dbSession?.current_branch; + let ownerRepo = dbSession?.owner_repo; + let isFork = dbSession?.is_fork || false; + let originOwnerRepo = dbSession?.origin_owner_repo; + + // If cache miss, fetch and cache + if (!branch || !ownerRepo) { + const repoInfo = await fetchAndCacheRepoInfo(sessionId, session.worktreePath, sessionManager, gitExecutor); + if (!repoInfo) { + return { success: true, data: null }; + } + branch = repoInfo.currentBranch; + ownerRepo = repoInfo.ownerRepo; + isFork = repoInfo.isFork; + originOwnerRepo = repoInfo.originOwnerRepo; + } - // Get origin owner for fork workflow + // Check if we have the necessary info + if (!branch || !ownerRepo) { + return { success: true, data: null }; + } + + // Get origin owner for fork workflow (derive from originOwnerRepo) let originOwner: string | null = null; - const originRes = await gitExecutor.run({ - sessionId, - cwd: session.worktreePath, - argv: ['git', 'remote', 'get-url', 'origin'], - op: 'read', - recordTimeline: false, - throwOnError: false, - timeoutMs: 3_000, - meta: { source: 'ipc.git', operation: 'get-origin-url' }, - }); - if (originRes.exitCode === 0 && originRes.stdout?.trim()) { - const originUrl = originRes.stdout.trim(); - const originMatch = originUrl.match(/github\.com[:/]([^/]+)\//); - if (originMatch) { - originOwner = originMatch[1]; - } + if (isFork && originOwnerRepo) { + originOwner = originOwnerRepo.split('/')[0]; } // Try to find PR in multiple remotes (upstream first for fork workflow, then origin) - const remoteNames = ['upstream', 'origin']; + const remoteNames = isFork ? ['upstream', 'origin'] : ['origin', 'upstream']; for (const remoteName of remoteNames) { const remoteRes = await gitExecutor.run({ @@ -937,19 +1042,19 @@ export function registerGitHandlers(ipcMain: IpcMain, services: AppServices): vo const cwd = session.worktreePath; - // Get current branch name - const branchRes = await gitExecutor.run({ - sessionId, - cwd, - argv: ['git', 'branch', '--show-current'], - op: 'read', - recordTimeline: false, - throwOnError: false, - timeoutMs: 3_000, - meta: { source: 'ipc.git', operation: 'get-branch' }, - }); + // Try to use cached branch name + const dbSession = sessionManager.db.getSession(sessionId); + let branch = dbSession?.current_branch; + + // If cache miss, fetch and cache + if (!branch) { + const repoInfo = await fetchAndCacheRepoInfo(sessionId, cwd, sessionManager, gitExecutor); + if (!repoInfo) { + return { success: true, data: { ahead: 0, behind: 0, branch: null } }; + } + branch = repoInfo.currentBranch; + } - const branch = branchRes.stdout?.trim(); if (!branch) { return { success: true, data: { ahead: 0, behind: 0, branch: null } }; }