diff --git a/src/__tests__/main/ipc/handlers/symphony.test.ts b/src/__tests__/main/ipc/handlers/symphony.test.ts index 623b84a78..4f07ce826 100644 --- a/src/__tests__/main/ipc/handlers/symphony.test.ts +++ b/src/__tests__/main/ipc/handlers/symphony.test.ts @@ -42,6 +42,11 @@ vi.mock('../../../../main/utils/execFile', () => ({ execFileNoThrow: vi.fn(), })); +// Mock symphony-fork +vi.mock('../../../../main/utils/symphony-fork', () => ({ + ensureForkSetup: vi.fn(), +})); + // Mock the logger vi.mock('../../../../main/utils/logger', () => ({ logger: { @@ -58,6 +63,7 @@ global.fetch = mockFetch; // Import mocked functions import { execFileNoThrow } from '../../../../main/utils/execFile'; +import { ensureForkSetup } from '../../../../main/utils/symphony-fork'; describe('Symphony IPC handlers', () => { let handlers: Map; @@ -106,6 +112,9 @@ describe('Symphony IPC handlers', () => { vi.mocked(fs.mkdir).mockResolvedValue(undefined); vi.mocked(fs.writeFile).mockResolvedValue(undefined); + // Default: no fork needed (user has push access) + vi.mocked(ensureForkSetup).mockResolvedValue({ isFork: false }); + // Register handlers registerSymphonyHandlers(mockDeps); }); @@ -2117,6 +2126,145 @@ describe('Symphony IPC handlers', () => { expect(result.draftPrNumber).toBe(123); }); }); + + describe('fork setup', () => { + it('should call ensureForkSetup after branch creation', async () => { + vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT')); + vi.mocked(ensureForkSetup).mockResolvedValue({ isFork: false }); + vi.mocked(execFileNoThrow).mockImplementation(async (cmd, args) => { + if (cmd === 'gh' && args?.[0] === 'auth') + return { stdout: 'Logged in', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'clone') + return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'symbolic-ref') + return { stdout: 'refs/remotes/origin/main', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'checkout') + return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'rev-parse') + return { stdout: 'symphony/issue-42-abc', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'push') return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'gh' && args?.[0] === 'pr') + return { stdout: 'https://github.com/owner/repo/pull/1', stderr: '', exitCode: 0 }; + return { stdout: '', stderr: '', exitCode: 0 }; + }); + + const handler = getStartHandler(); + await handler!({} as any, validStartParams); + + expect(ensureForkSetup).toHaveBeenCalledWith(expect.stringContaining('repo'), 'owner/repo'); + + // Verify fork setup runs after branch creation (checkout -b) + const checkoutIdx = vi + .mocked(execFileNoThrow) + .mock.calls.findIndex( + (call) => call[0] === 'git' && (call[1] as string[])?.[0] === 'checkout' + ); + const checkoutCallOrder = vi.mocked(execFileNoThrow).mock.invocationCallOrder[checkoutIdx]; + const forkSetupCallOrder = vi.mocked(ensureForkSetup).mock.invocationCallOrder[0]; + expect(checkoutCallOrder).toBeDefined(); + expect(forkSetupCallOrder).toBeDefined(); + expect(checkoutCallOrder).toBeLessThan(forkSetupCallOrder!); + }); + + it('should return error when fork setup fails', async () => { + vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT')); + vi.mocked(ensureForkSetup).mockResolvedValue({ isFork: false, error: 'permission denied' }); + vi.mocked(execFileNoThrow).mockImplementation(async (cmd, args) => { + if (cmd === 'gh' && args?.[0] === 'auth') + return { stdout: 'Logged in', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'clone') + return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'symbolic-ref') + return { stdout: 'refs/remotes/origin/main', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'checkout') + return { stdout: '', stderr: '', exitCode: 0 }; + return { stdout: '', stderr: '', exitCode: 0 }; + }); + + const handler = getStartHandler(); + const result = await handler!({} as any, validStartParams); + + expect(result.error).toContain('Fork setup failed'); + }); + + it('should persist fork info in contribution when fork is needed', async () => { + vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT')); + vi.mocked(ensureForkSetup).mockResolvedValue({ isFork: true, forkSlug: 'chris/repo' }); + vi.mocked(execFileNoThrow).mockImplementation(async (cmd, args) => { + if (cmd === 'gh' && args?.[0] === 'auth') + return { stdout: 'Logged in', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'clone') + return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'symbolic-ref') + return { stdout: 'refs/remotes/origin/main', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'checkout') + return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'rev-parse') + return { stdout: 'symphony/issue-42-abc', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'push') return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'gh' && args?.[0] === 'pr') + return { stdout: 'https://github.com/owner/repo/pull/1', stderr: '', exitCode: 0 }; + return { stdout: '', stderr: '', exitCode: 0 }; + }); + + const handler = getStartHandler(); + await handler!({} as any, validStartParams); + + // Verify the state was written with fork info + const writeStateCall = vi + .mocked(fs.writeFile) + .mock.calls.find( + (call) => typeof call[0] === 'string' && call[0].includes('symphony-state.json') + ); + expect(writeStateCall).toBeDefined(); + const savedState = JSON.parse(writeStateCall![1] as string); + const savedContrib = savedState.active[0]; + expect(savedContrib.isFork).toBe(true); + expect(savedContrib.forkSlug).toBe('chris/repo'); + expect(savedContrib.upstreamSlug).toBe('owner/repo'); + }); + + it('should pass fork info to createDraftPR for cross-fork PRs', async () => { + vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT')); + vi.mocked(ensureForkSetup).mockResolvedValue({ isFork: true, forkSlug: 'chris/repo' }); + vi.mocked(execFileNoThrow).mockImplementation(async (cmd, args) => { + if (cmd === 'gh' && args?.[0] === 'auth') + return { stdout: 'Logged in', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'clone') + return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'symbolic-ref') + return { stdout: 'refs/remotes/origin/main', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'checkout') + return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'rev-parse') + return { stdout: 'symphony/issue-42-abc', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'push') return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'gh' && args?.[0] === 'pr') + return { stdout: 'https://github.com/owner/repo/pull/1', stderr: '', exitCode: 0 }; + return { stdout: '', stderr: '', exitCode: 0 }; + }); + + const handler = getStartHandler(); + await handler!({} as any, validStartParams); + + // Verify gh pr create was called with --head chris:branchName and --repo owner/repo + const prCall = vi + .mocked(execFileNoThrow) + .mock.calls.find( + (call) => call[0] === 'gh' && call[1]?.[0] === 'pr' && call[1]?.[1] === 'create' + ); + expect(prCall).toBeDefined(); + const prArgs = prCall![1] as string[]; + // Should have --head chris:branchName + const headIdx = prArgs.indexOf('--head'); + expect(headIdx).toBeGreaterThanOrEqual(0); + expect(prArgs[headIdx + 1]).toMatch(/^chris:/); + // Should have --repo owner/repo + const repoIdx = prArgs.indexOf('--repo'); + expect(repoIdx).toBeGreaterThanOrEqual(0); + expect(prArgs[repoIdx + 1]).toBe('owner/repo'); + }); + }); }); // ============================================================================ @@ -4502,6 +4650,104 @@ describe('Symphony IPC handlers', () => { expect(result.branchName).toBeUndefined(); }); }); + + describe('fork setup', () => { + it('should call ensureForkSetup after branch creation', async () => { + vi.mocked(ensureForkSetup).mockResolvedValue({ isFork: false }); + vi.mocked(execFileNoThrow).mockImplementation(async (cmd, args) => { + if (cmd === 'gh' && args?.[0] === 'auth') + return { stdout: 'Logged in', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'checkout') + return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'commit') + return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'symbolic-ref') + return { stdout: 'refs/remotes/origin/main', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'ls-remote') + return { stdout: 'abc123\trefs/heads/main', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'rev-parse') + return { stdout: 'symphony/issue-42-abc', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'push') return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'gh' && args?.[0] === 'pr') + return { stdout: 'https://github.com/owner/repo/pull/1', stderr: '', exitCode: 0 }; + return { stdout: '', stderr: '', exitCode: 0 }; + }); + + const handler = getStartContributionHandler(); + await handler!({} as any, validStartContributionParams); + + expect(ensureForkSetup).toHaveBeenCalledWith( + validStartContributionParams.localPath, + 'owner/repo' + ); + + // Verify ensureForkSetup ran after the checkout + const checkoutCallIdx = vi + .mocked(execFileNoThrow) + .mock.invocationCallOrder.find((order, i) => { + const call = vi.mocked(execFileNoThrow).mock.calls[i]; + return call[0] === 'git' && call[1]?.[0] === 'checkout'; + }); + const forkSetupCallIdx = vi.mocked(ensureForkSetup).mock.invocationCallOrder[0]; + expect(checkoutCallIdx).toBeDefined(); + expect(forkSetupCallIdx).toBeGreaterThan(checkoutCallIdx!); + }); + + it('should return error when fork setup fails', async () => { + vi.mocked(ensureForkSetup).mockResolvedValue({ isFork: false, error: 'permission denied' }); + vi.mocked(execFileNoThrow).mockImplementation(async (cmd, args) => { + if (cmd === 'gh' && args?.[0] === 'auth') + return { stdout: 'Logged in', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'checkout') + return { stdout: '', stderr: '', exitCode: 0 }; + return { stdout: '', stderr: '', exitCode: 0 }; + }); + + const handler = getStartContributionHandler(); + const result = await handler!({} as any, validStartContributionParams); + + expect(result.success).toBe(false); + expect(result.error).toContain('Fork setup failed'); + }); + + it('should write fork info to metadata when fork is needed', async () => { + vi.mocked(ensureForkSetup).mockResolvedValue({ isFork: true, forkSlug: 'chris/repo' }); + vi.mocked(execFileNoThrow).mockImplementation(async (cmd, args) => { + if (cmd === 'gh' && args?.[0] === 'auth') + return { stdout: 'Logged in', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'checkout') + return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'commit') + return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'symbolic-ref') + return { stdout: 'refs/remotes/origin/main', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'ls-remote') + return { stdout: 'abc123\trefs/heads/main', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'rev-parse') + return { stdout: 'symphony/issue-42-abc', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'push') return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'gh' && args?.[0] === 'pr') + return { stdout: 'https://github.com/owner/repo/pull/1', stderr: '', exitCode: 0 }; + return { stdout: '', stderr: '', exitCode: 0 }; + }); + + const handler = getStartContributionHandler(); + await handler!({} as any, validStartContributionParams); + + // Verify metadata was written with fork info + const metadataCall = vi + .mocked(fs.writeFile) + .mock.calls.find( + (call) => typeof call[0] === 'string' && call[0].includes('metadata.json') + ); + expect(metadataCall).toBeDefined(); + const metadata = JSON.parse(metadataCall![1] as string); + expect(metadata.isFork).toBe(true); + expect(metadata.forkSlug).toBe('chris/repo'); + expect(metadata.upstreamSlug).toBe('owner/repo'); + expect(metadata.upstreamDefaultBranch).toBe('main'); + }); + }); }); // ============================================================================ @@ -4523,6 +4769,10 @@ describe('Symphony IPC handlers', () => { prCreated: boolean; draftPrNumber?: number; draftPrUrl?: string; + isFork?: boolean; + forkSlug?: string; + upstreamSlug?: string; + upstreamDefaultBranch?: string; }> ) => ({ contributionId: 'contrib_draft_test', @@ -4996,6 +5246,199 @@ describe('Symphony IPC handlers', () => { expect(result.error).toBeUndefined(); }); }); + + describe('fork support', () => { + it('should pass fork info to gh pr create when metadata has fork info', async () => { + const metadata = createValidMetadata({ + isFork: true, + forkSlug: 'chris/repo', + upstreamSlug: 'owner/repo', + upstreamDefaultBranch: 'develop', + }); + const stateWithActiveContrib = { + active: [ + { + id: 'contrib_draft_test', + repoSlug: 'owner/repo', + issueNumber: 42, + status: 'running', + }, + ], + history: [], + stats: {}, + }; + vi.mocked(fs.readFile).mockImplementation(async (filePath) => { + if ((filePath as string).includes('metadata.json')) { + return JSON.stringify(metadata); + } + if ((filePath as string).includes('state.json')) { + return JSON.stringify(stateWithActiveContrib); + } + throw new Error('ENOENT'); + }); + vi.mocked(execFileNoThrow).mockImplementation(async (cmd, args) => { + if (cmd === 'gh' && args?.[0] === 'auth') + return { stdout: 'Logged in', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'symbolic-ref') + return { stdout: 'refs/remotes/origin/main', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'rev-list') + return { stdout: '1', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'rev-parse') + return { stdout: 'symphony/issue-42-abc123', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'push') return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'gh' && args?.[0] === 'pr') + return { stdout: 'https://github.com/owner/repo/pull/50', stderr: '', exitCode: 0 }; + return { stdout: '', stderr: '', exitCode: 0 }; + }); + + const handler = getCreateDraftPRHandler(); + const result = await handler!({} as any, { contributionId: 'contrib_draft_test' }); + + expect(result.success).toBe(true); + + // Verify gh pr create was called with fork args + const prCall = vi + .mocked(execFileNoThrow) + .mock.calls.find( + (call) => call[0] === 'gh' && call[1]?.[0] === 'pr' && call[1]?.[1] === 'create' + ); + expect(prCall).toBeDefined(); + const prArgs = prCall![1] as string[]; + // Should have --head chris:branchName + const headIdx = prArgs.indexOf('--head'); + expect(headIdx).toBeGreaterThanOrEqual(0); + expect(prArgs[headIdx + 1]).toMatch(/^chris:/); + // Should have --repo owner/repo + const repoIdx = prArgs.indexOf('--repo'); + expect(repoIdx).toBeGreaterThanOrEqual(0); + expect(prArgs[repoIdx + 1]).toBe('owner/repo'); + // Should use upstreamDefaultBranch from metadata as --base + const baseIdx = prArgs.indexOf('--base'); + expect(baseIdx).toBeGreaterThanOrEqual(0); + expect(prArgs[baseIdx + 1]).toBe('develop'); + }); + + it('should not pass fork args when metadata has no fork info', async () => { + const metadata = createValidMetadata(); + const stateWithActiveContrib = { + active: [ + { + id: 'contrib_draft_test', + repoSlug: 'owner/repo', + issueNumber: 42, + status: 'running', + }, + ], + history: [], + stats: {}, + }; + vi.mocked(fs.readFile).mockImplementation(async (filePath) => { + if ((filePath as string).includes('metadata.json')) { + return JSON.stringify(metadata); + } + if ((filePath as string).includes('state.json')) { + return JSON.stringify(stateWithActiveContrib); + } + throw new Error('ENOENT'); + }); + vi.mocked(execFileNoThrow).mockImplementation(async (cmd, args) => { + if (cmd === 'gh' && args?.[0] === 'auth') + return { stdout: 'Logged in', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'symbolic-ref') + return { stdout: 'refs/remotes/origin/main', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'rev-list') + return { stdout: '1', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'rev-parse') + return { stdout: 'symphony/issue-42-abc123', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'push') return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'gh' && args?.[0] === 'pr') + return { stdout: 'https://github.com/owner/repo/pull/50', stderr: '', exitCode: 0 }; + return { stdout: '', stderr: '', exitCode: 0 }; + }); + + const handler = getCreateDraftPRHandler(); + const result = await handler!({} as any, { contributionId: 'contrib_draft_test' }); + + expect(result.success).toBe(true); + + // Verify gh pr create was called WITHOUT --repo flag + const prCall = vi + .mocked(execFileNoThrow) + .mock.calls.find( + (call) => call[0] === 'gh' && call[1]?.[0] === 'pr' && call[1]?.[1] === 'create' + ); + expect(prCall).toBeDefined(); + const prArgs = prCall![1] as string[]; + expect(prArgs).not.toContain('--repo'); + // --head should be just the branch name, not prefixed + const headIdx = prArgs.indexOf('--head'); + expect(headIdx).toBeGreaterThanOrEqual(0); + expect(prArgs[headIdx + 1]).not.toContain(':'); + }); + + it('should pass --repo but not fork-prefixed --head when metadata has upstreamSlug only', async () => { + const metadata = createValidMetadata({ + upstreamSlug: 'owner/repo', + }); + const stateWithActiveContrib = { + active: [ + { + id: 'contrib_draft_test', + repoSlug: 'owner/repo', + issueNumber: 42, + status: 'running', + }, + ], + history: [], + stats: {}, + }; + vi.mocked(fs.readFile).mockImplementation(async (filePath) => { + if ((filePath as string).includes('metadata.json')) { + return JSON.stringify(metadata); + } + if ((filePath as string).includes('state.json')) { + return JSON.stringify(stateWithActiveContrib); + } + throw new Error('ENOENT'); + }); + vi.mocked(execFileNoThrow).mockImplementation(async (cmd, args) => { + if (cmd === 'gh' && args?.[0] === 'auth') + return { stdout: 'Logged in', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'symbolic-ref') + return { stdout: 'refs/remotes/origin/main', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'rev-list') + return { stdout: '1', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'rev-parse') + return { stdout: 'symphony/issue-42-abc123', stderr: '', exitCode: 0 }; + if (cmd === 'git' && args?.[0] === 'push') return { stdout: '', stderr: '', exitCode: 0 }; + if (cmd === 'gh' && args?.[0] === 'pr') + return { stdout: 'https://github.com/owner/repo/pull/50', stderr: '', exitCode: 0 }; + return { stdout: '', stderr: '', exitCode: 0 }; + }); + + const handler = getCreateDraftPRHandler(); + const result = await handler!({} as any, { contributionId: 'contrib_draft_test' }); + + expect(result.success).toBe(true); + + // Verify gh pr create was called with --repo but no fork-prefixed --head + const prCall = vi + .mocked(execFileNoThrow) + .mock.calls.find( + (call) => call[0] === 'gh' && call[1]?.[0] === 'pr' && call[1]?.[1] === 'create' + ); + expect(prCall).toBeDefined(); + const prArgs = prCall![1] as string[]; + // Should have --repo owner/repo (upstream slug from metadata) + const repoIdx = prArgs.indexOf('--repo'); + expect(repoIdx).toBeGreaterThan(-1); + expect(prArgs[repoIdx + 1]).toBe('owner/repo'); + // --head should be just the branch name (no fork owner prefix since no forkSlug) + const headIdx = prArgs.indexOf('--head'); + expect(headIdx).toBeGreaterThanOrEqual(0); + expect(prArgs[headIdx + 1]).not.toContain(':'); + }); + }); }); // ============================================================================ diff --git a/src/__tests__/main/services/symphony-runner.test.ts b/src/__tests__/main/services/symphony-runner.test.ts index b3c88c0f1..7ae084a8a 100644 --- a/src/__tests__/main/services/symphony-runner.test.ts +++ b/src/__tests__/main/services/symphony-runner.test.ts @@ -33,6 +33,11 @@ vi.mock('../../../main/utils/logger', () => ({ }, })); +// Mock symphony-fork +vi.mock('../../../main/utils/symphony-fork', () => ({ + ensureForkSetup: vi.fn(), +})); + // Mock global fetch const mockFetch = vi.fn(); global.fetch = mockFetch; @@ -41,6 +46,7 @@ global.fetch = mockFetch; import fs from 'fs/promises'; import { execFileNoThrow } from '../../../main/utils/execFile'; import { logger } from '../../../main/utils/logger'; +import { ensureForkSetup } from '../../../main/utils/symphony-fork'; import { startContribution, finalizeContribution, @@ -81,6 +87,9 @@ describe('Symphony Runner Service', () => { vi.mocked(fs.rm).mockResolvedValue(undefined); vi.mocked(fs.writeFile).mockResolvedValue(undefined); vi.mocked(fs.copyFile).mockResolvedValue(undefined); + + // Default: no fork needed + vi.mocked(ensureForkSetup).mockResolvedValue({ isFork: false }); }); afterEach(() => { @@ -1281,7 +1290,7 @@ describe('Symphony Runner Service', () => { ); }); - it('handles PR close failure gracefully (warns but continues)', async () => { + it('returns failure when PR close fails', async () => { vi.mocked(execFileNoThrow).mockResolvedValueOnce({ stdout: '', stderr: 'pr close failed', @@ -1295,8 +1304,8 @@ describe('Symphony Runner Service', () => { expect.any(String), expect.objectContaining({ prNumber: 42 }) ); - // Should still return success - expect(result.success).toBe(true); + expect(result.success).toBe(false); + expect(result.error).toContain('pr close failed'); }); it('removes local directory when cleanup=true', async () => { @@ -1323,7 +1332,7 @@ describe('Symphony Runner Service', () => { expect(fs.rm).not.toHaveBeenCalled(); }); - it('returns success:true even if PR close fails', async () => { + it('does not clean up local directory when PR close fails', async () => { vi.mocked(execFileNoThrow).mockResolvedValueOnce({ stdout: '', stderr: 'error', @@ -1332,7 +1341,198 @@ describe('Symphony Runner Service', () => { const result = await cancelContribution('/tmp/test-repo', 42, true); - expect(result.success).toBe(true); + expect(result.success).toBe(false); + expect(fs.rm).not.toHaveBeenCalled(); + }); + + it('adds --repo flag without --delete-branch for fork contributions', async () => { + vi.mocked(execFileNoThrow).mockResolvedValueOnce({ + stdout: '', + stderr: '', + exitCode: 0, + }); + + await cancelContribution('/tmp/test-repo', 42, true, 'upstream-owner/repo'); + + expect(execFileNoThrow).toHaveBeenCalledWith( + 'gh', + ['pr', 'close', '42', '--repo', 'upstream-owner/repo'], + '/tmp/test-repo' + ); + }); + }); + + // ============================================================================ + // Fork Support Tests + // ============================================================================ + + describe('fork support', () => { + const defaultOptions = { + contributionId: 'test-id', + repoSlug: 'upstream-owner/repo', + repoUrl: 'https://github.com/upstream-owner/repo.git', + issueNumber: 42, + issueTitle: 'Test Fork Issue', + documentPaths: [], + localPath: '/tmp/test-repo', + branchName: 'symphony/test-branch', + }; + + describe('startContribution with fork', () => { + it('calls ensureForkSetup after clone and branch creation', async () => { + mockSuccessfulWorkflow(); + + await startContribution(defaultOptions); + + expect(ensureForkSetup).toHaveBeenCalledWith('/tmp/test-repo', 'upstream-owner/repo'); + + // Verify ensureForkSetup runs after checkout -b (2nd execFileNoThrow call) + const checkoutOrder = vi.mocked(execFileNoThrow).mock.invocationCallOrder[1]; // checkout -b + const forkSetupOrder = vi.mocked(ensureForkSetup).mock.invocationCallOrder[0]; + expect(forkSetupOrder).toBeGreaterThan(checkoutOrder); + }); + + it('returns fork info when ensureForkSetup detects a fork', async () => { + vi.mocked(ensureForkSetup).mockResolvedValue({ + isFork: true, + forkSlug: 'myuser/repo', + }); + vi.mocked(execFileNoThrow) + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }) // clone + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }) // checkout -b + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }) // config user.name + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }) // config user.email + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }) // commit --allow-empty + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }) // push + .mockResolvedValueOnce({ stdout: 'symphony/test-branch', stderr: '', exitCode: 0 }) // git rev-parse --abbrev-ref HEAD + .mockResolvedValueOnce({ + stdout: 'https://github.com/upstream-owner/repo/pull/5', + stderr: '', + exitCode: 0, + }); // pr create + + const result = await startContribution(defaultOptions); + + expect(result.success).toBe(true); + expect(result.isFork).toBe(true); + expect(result.forkSlug).toBe('myuser/repo'); + }); + + it('passes --repo and --head to gh pr create for fork contributions', async () => { + vi.mocked(ensureForkSetup).mockResolvedValue({ + isFork: true, + forkSlug: 'myuser/repo', + }); + vi.mocked(execFileNoThrow) + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }) // clone + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }) // checkout -b + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }) // config user.name + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }) // config user.email + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }) // commit --allow-empty + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }) // push + .mockResolvedValueOnce({ stdout: 'symphony/test-branch', stderr: '', exitCode: 0 }) // git rev-parse --abbrev-ref HEAD + .mockResolvedValueOnce({ + stdout: 'https://github.com/upstream-owner/repo/pull/5', + stderr: '', + exitCode: 0, + }); // pr create + + await startContribution(defaultOptions); + + const prCreateCall = vi + .mocked(execFileNoThrow) + .mock.calls.find((call) => call[0] === 'gh' && call[1]?.includes('create')); + expect(prCreateCall).toBeDefined(); + expect(prCreateCall![1]).toContain('--repo'); + expect(prCreateCall![1]).toContain('upstream-owner/repo'); + expect(prCreateCall![1]).toContain('--head'); + expect(prCreateCall![1]).toContain('myuser:symphony/test-branch'); + }); + + it('cleans up and returns error when ensureForkSetup fails', async () => { + vi.mocked(ensureForkSetup).mockResolvedValue({ + isFork: false, + error: 'GitHub CLI not authenticated', + }); + vi.mocked(execFileNoThrow) + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }) // clone + .mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 }); // checkout -b + + const result = await startContribution(defaultOptions); + + expect(result.success).toBe(false); + expect(result.error).toContain('Fork setup failed'); + expect(fs.rm).toHaveBeenCalledWith('/tmp/test-repo', { recursive: true, force: true }); + }); + + it('does not pass --repo/--head for non-fork contributions', async () => { + vi.mocked(ensureForkSetup).mockResolvedValue({ isFork: false }); + mockSuccessfulWorkflow(); + + await startContribution(defaultOptions); + + const prCreateCall = vi + .mocked(execFileNoThrow) + .mock.calls.find((call) => call[0] === 'gh' && call[1]?.includes('create')); + expect(prCreateCall).toBeDefined(); + expect(prCreateCall![1]).not.toContain('--repo'); + expect(prCreateCall![1]).not.toContain('--head'); + }); + }); + + describe('finalizeContribution with fork', () => { + it('adds --repo flag to gh pr ready, edit, and view for fork contributions', async () => { + mockFinalizeWorkflow('https://github.com/upstream-owner/repo/pull/5'); + + await finalizeContribution('/tmp/test-repo', 5, 42, 'Test Issue', 'upstream-owner/repo'); + + const readyCall = vi + .mocked(execFileNoThrow) + .mock.calls.find((call) => call[0] === 'gh' && call[1]?.includes('ready')); + expect(readyCall![1]).toContain('--repo'); + expect(readyCall![1]).toContain('upstream-owner/repo'); + + const editCall = vi + .mocked(execFileNoThrow) + .mock.calls.find((call) => call[0] === 'gh' && call[1]?.includes('edit')); + expect(editCall![1]).toContain('--repo'); + expect(editCall![1]).toContain('upstream-owner/repo'); + + const viewCall = vi + .mocked(execFileNoThrow) + .mock.calls.find((call) => call[0] === 'gh' && call[1]?.includes('view')); + expect(viewCall![1]).toContain('--repo'); + expect(viewCall![1]).toContain('upstream-owner/repo'); + }); + + it('does not add --repo flag when upstreamSlug is not provided', async () => { + mockFinalizeWorkflow(); + + await finalizeContribution('/tmp/test-repo', 1, 123, 'Test Issue'); + + const readyCall = vi + .mocked(execFileNoThrow) + .mock.calls.find((call) => call[0] === 'gh' && call[1]?.includes('ready')); + expect(readyCall![1]).not.toContain('--repo'); + }); + }); + + describe('cancelContribution with fork', () => { + it('does not add --repo flag when upstreamSlug is not provided', async () => { + vi.mocked(execFileNoThrow).mockResolvedValueOnce({ + stdout: '', + stderr: '', + exitCode: 0, + }); + + await cancelContribution('/tmp/test-repo', 42, true); + + expect(execFileNoThrow).toHaveBeenCalledWith( + 'gh', + ['pr', 'close', '42', '--delete-branch'], + '/tmp/test-repo' + ); + }); }); }); }); diff --git a/src/__tests__/main/utils/symphony-fork.test.ts b/src/__tests__/main/utils/symphony-fork.test.ts new file mode 100644 index 000000000..422165312 --- /dev/null +++ b/src/__tests__/main/utils/symphony-fork.test.ts @@ -0,0 +1,185 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { ExecResult } from '../../../main/utils/execFile'; + +const mockExecFileNoThrow = vi.fn<(...args: unknown[]) => Promise>(); + +vi.mock('../../../main/utils/execFile', () => ({ + execFileNoThrow: (...args: unknown[]) => mockExecFileNoThrow(...args), +})); + +vi.mock('../../../main/utils/logger', () => ({ + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, +})); + +vi.mock('../../../main/agents/path-prober', () => ({ + getExpandedEnv: () => ({ PATH: '/usr/bin' }), +})); + +import { ensureForkSetup } from '../../../main/utils/symphony-fork'; + +function ok(stdout: string): ExecResult { + return { stdout, stderr: '', exitCode: 0 }; +} + +function fail(stderr: string, exitCode: number | string = 1): ExecResult { + return { stdout: '', stderr, exitCode }; +} + +describe('ensureForkSetup', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('returns error when gh is not authenticated', async () => { + mockExecFileNoThrow.mockResolvedValueOnce(fail('not logged in')); + + const result = await ensureForkSetup('/tmp/repo', 'owner/repo'); + + expect(result).toEqual({ isFork: false, error: 'GitHub CLI not authenticated' }); + }); + + it('returns isFork: false when user owns the repo', async () => { + mockExecFileNoThrow.mockResolvedValueOnce(ok('chris\n')); + + const result = await ensureForkSetup('/tmp/repo', 'chris/repo'); + + expect(result).toEqual({ isFork: false }); + expect(mockExecFileNoThrow).toHaveBeenCalledTimes(1); + }); + + it('returns isFork: false when user has push access', async () => { + mockExecFileNoThrow + .mockResolvedValueOnce(ok('chris\n')) // gh api user + .mockResolvedValueOnce(ok('true\n')); // permissions.push + + const result = await ensureForkSetup('/tmp/repo', 'owner/repo'); + + expect(result).toEqual({ isFork: false }); + }); + + it('forks and reconfigures remotes when no push access', async () => { + mockExecFileNoThrow + .mockResolvedValueOnce(ok('chris\n')) // gh api user + .mockResolvedValueOnce(ok('false\n')) // permissions.push + .mockResolvedValueOnce(ok('')) // gh repo fork + .mockResolvedValueOnce(ok('https://github.com/chris/repo.git\n')) // clone url + .mockResolvedValueOnce(ok('')) // git remote rename + .mockResolvedValueOnce(ok('')) // git remote add + .mockResolvedValueOnce(ok('')); // git remote set-head origin -a + + const result = await ensureForkSetup('/tmp/repo', 'owner/repo'); + + expect(result).toEqual({ isFork: true, forkSlug: 'chris/repo' }); + + // Verify fork command + expect(mockExecFileNoThrow).toHaveBeenCalledWith( + 'gh', + ['repo', 'fork', 'owner/repo', '--clone=false'], + undefined, + expect.any(Object) + ); + + // Verify remote rename + expect(mockExecFileNoThrow).toHaveBeenCalledWith( + 'git', + ['remote', 'rename', 'origin', 'upstream'], + '/tmp/repo' + ); + + // Verify remote add + expect(mockExecFileNoThrow).toHaveBeenCalledWith( + 'git', + ['remote', 'add', 'origin', 'https://github.com/chris/repo.git'], + '/tmp/repo' + ); + + // Verify set-head for correct default branch resolution + expect(mockExecFileNoThrow).toHaveBeenCalledWith( + 'git', + ['remote', 'set-head', 'origin', '-a'], + '/tmp/repo' + ); + }); + + it('handles fork already existing', async () => { + mockExecFileNoThrow + .mockResolvedValueOnce(ok('chris\n')) // gh api user + .mockResolvedValueOnce(ok('false\n')) // permissions.push + .mockResolvedValueOnce(fail('repo already exists', 1)) // gh repo fork - already exists + .mockResolvedValueOnce(ok('https://github.com/chris/repo.git\n')) // clone url + .mockResolvedValueOnce(ok('')) // git remote rename + .mockResolvedValueOnce(ok('')) // git remote add + .mockResolvedValueOnce(ok('')); // git remote set-head origin -a + + const result = await ensureForkSetup('/tmp/repo', 'owner/repo'); + + expect(result).toEqual({ isFork: true, forkSlug: 'chris/repo' }); + }); + + it('returns error when fork fails for non-existing reason', async () => { + mockExecFileNoThrow + .mockResolvedValueOnce(ok('chris\n')) // gh api user + .mockResolvedValueOnce(ok('false\n')) // permissions.push + .mockResolvedValueOnce(fail('permission denied', 1)); // gh repo fork + + const result = await ensureForkSetup('/tmp/repo', 'owner/repo'); + + expect(result.isFork).toBe(false); + expect(result.error).toContain('permission denied'); + }); + + it('falls back to set-url when remote rename fails', async () => { + mockExecFileNoThrow + .mockResolvedValueOnce(ok('chris\n')) // gh api user + .mockResolvedValueOnce(ok('false\n')) // permissions.push + .mockResolvedValueOnce(ok('')) // gh repo fork + .mockResolvedValueOnce(ok('https://github.com/chris/repo.git\n')) // clone url + .mockResolvedValueOnce(fail('upstream already exists')) // remote rename fails + .mockResolvedValueOnce(ok('')) // git remote set-url (fallback) + .mockResolvedValueOnce(ok('')); // git remote set-head origin -a + + const result = await ensureForkSetup('/tmp/repo', 'owner/repo'); + + expect(result).toEqual({ isFork: true, forkSlug: 'chris/repo' }); + + // Verify fallback to set-url + expect(mockExecFileNoThrow).toHaveBeenCalledWith( + 'git', + ['remote', 'set-url', 'origin', 'https://github.com/chris/repo.git'], + '/tmp/repo' + ); + }); + + it('returns error when both remote rename and set-url fail', async () => { + mockExecFileNoThrow + .mockResolvedValueOnce(ok('chris\n')) // gh api user + .mockResolvedValueOnce(ok('false\n')) // permissions.push + .mockResolvedValueOnce(ok('')) // gh repo fork + .mockResolvedValueOnce(ok('https://github.com/chris/repo.git\n')) // clone url + .mockResolvedValueOnce(fail('rename error')) // remote rename fails + .mockResolvedValueOnce(fail('set-url error')); // set-url also fails + + const result = await ensureForkSetup('/tmp/repo', 'owner/repo'); + + expect(result.isFork).toBe(false); + expect(result.error).toContain('set-url error'); + }); + + it('returns error when getting fork clone URL fails', async () => { + mockExecFileNoThrow + .mockResolvedValueOnce(ok('chris\n')) // gh api user + .mockResolvedValueOnce(ok('false\n')) // permissions.push + .mockResolvedValueOnce(ok('')) // gh repo fork + .mockResolvedValueOnce(fail('not found')); // clone url fails + + const result = await ensureForkSetup('/tmp/repo', 'owner/repo'); + + expect(result.isFork).toBe(false); + expect(result.error).toContain('Failed to get fork clone URL'); + }); +}); diff --git a/src/main/ipc/handlers/symphony.ts b/src/main/ipc/handlers/symphony.ts index 195754594..686b0bacd 100644 --- a/src/main/ipc/handlers/symphony.ts +++ b/src/main/ipc/handlers/symphony.ts @@ -20,6 +20,7 @@ import type { SessionsData, StoredSession } from '../../stores/types'; import { createIpcHandler, CreateHandlerOptions } from '../../utils/ipcHandler'; import { execFileNoThrow } from '../../utils/execFile'; import { getExpandedEnv } from '../../agents/path-prober'; +import { ensureForkSetup } from '../../utils/symphony-fork'; import { SYMPHONY_REGISTRY_URL, REGISTRY_CACHE_TTL_MS, @@ -742,7 +743,9 @@ async function createDraftPR( repoPath: string, baseBranch: string, title: string, - body: string + body: string, + upstreamSlug?: string, + forkOwner?: string ): Promise<{ success: boolean; prUrl?: string; prNumber?: number; error?: string }> { // Check gh authentication first const authCheck = await checkGhAuthentication(); @@ -769,27 +772,32 @@ async function createDraftPR( } // Create draft PR using gh CLI (use --head to explicitly specify the branch) - const prResult = await execFileNoThrow( - 'gh', - [ - 'pr', - 'create', - '--draft', - '--base', - baseBranch, - '--head', - branchName, - '--title', - title, - '--body', - body, - ], - repoPath, - getExpandedEnv() - ); + const prArgs = [ + 'pr', + 'create', + '--draft', + '--base', + baseBranch, + '--head', + // For fork contributions, use "forkOwner:branchName" to specify the fork's branch + upstreamSlug && forkOwner ? `${forkOwner}:${branchName}` : branchName, + '--title', + title, + '--body', + body, + ]; + + // For fork contributions, target the upstream repo + if (upstreamSlug) { + prArgs.push('--repo', upstreamSlug); + } + + const prResult = await execFileNoThrow('gh', prArgs, repoPath, getExpandedEnv()); if (prResult.exitCode !== 0) { - // If PR creation failed after push, try to delete the remote branch + // If PR creation failed after push, try to delete the remote branch. + // Note: In fork mode, `origin` points to the user's fork (set by ensureForkSetup), + // so this correctly deletes the branch from the fork, not the upstream repo. logger.warn('PR creation failed, attempting to clean up remote branch', LOG_CONTEXT); await execFileNoThrow('git', ['push', 'origin', '--delete', branchName], repoPath); return { success: false, error: `Failed to create PR: ${prResult.stderr}` }; @@ -808,14 +816,14 @@ async function createDraftPR( */ async function markPRReady( repoPath: string, - prNumber: number + prNumber: number, + upstreamSlug?: string ): Promise<{ success: boolean; error?: string }> { - const result = await execFileNoThrow( - 'gh', - ['pr', 'ready', String(prNumber)], - repoPath, - getExpandedEnv() - ); + const args = ['pr', 'ready', String(prNumber)]; + if (upstreamSlug) { + args.push('--repo', upstreamSlug); + } + const result = await execFileNoThrow('gh', args, repoPath, getExpandedEnv()); if (result.exitCode !== 0) { return { success: false, error: result.stderr }; @@ -831,13 +839,15 @@ async function markPRReady( */ async function discoverPRByBranch( repoSlug: string, - branchName: string + branchName: string, + headOwner?: string ): Promise<{ prNumber?: number; prUrl?: string }> { try { // Query GitHub API for PRs with this head branch // API: GET /repos/{owner}/{repo}/pulls?head={owner}:{branch}&state=all + // For cross-fork PRs, headOwner is the fork owner (branch lives on fork, PR targets upstream) const [owner] = repoSlug.split('/'); - const headRef = `${owner}:${branchName}`; + const headRef = `${headOwner || owner}:${branchName}`; const apiUrl = `${GITHUB_API_BASE}/repos/${repoSlug}/pulls?head=${encodeURIComponent(headRef)}&state=all&per_page=1`; const response = await fetch(apiUrl, { @@ -900,7 +910,8 @@ async function postPRComment( timeSpentMs: number; documentsProcessed: number; tasksCompleted: number; - } + }, + upstreamSlug?: string ): Promise<{ success: boolean; error?: string }> { // Format time spent const hours = Math.floor(stats.timeSpentMs / 3600000); @@ -935,12 +946,11 @@ This pull request was created using [Maestro Symphony](https://runmaestro.ai/sym --- *Powered by [Maestro](https://runmaestro.ai) • [Learn about Symphony](https://docs.runmaestro.ai/symphony)*`; - const result = await execFileNoThrow( - 'gh', - ['pr', 'comment', String(prNumber), '--body', commentBody], - repoPath, - getExpandedEnv() - ); + const commentArgs = ['pr', 'comment', String(prNumber), '--body', commentBody]; + if (upstreamSlug) { + commentArgs.push('--repo', upstreamSlug); + } + const result = await execFileNoThrow('gh', commentArgs, repoPath, getExpandedEnv()); if (result.exitCode !== 0) { return { success: false, error: result.stderr }; @@ -1486,6 +1496,22 @@ export function registerSymphonyHandlers({ return { error: `Branch creation failed: ${branchResult.error}` }; } + // Set up fork if user doesn't have push access + logger.info('Checking fork requirements', LOG_CONTEXT, { repoSlug }); + const forkResult = await ensureForkSetup(localPath, repoSlug); + if (forkResult.error) { + await fs.rm(localPath, { recursive: true, force: true }).catch(() => {}); + return { error: `Fork setup failed: ${forkResult.error}` }; + } + if (forkResult.isFork) { + logger.info('Using fork for contribution', LOG_CONTEXT, { + forkSlug: forkResult.forkSlug, + upstreamSlug: repoSlug, + }); + } else { + logger.info('User has push access, no fork needed', LOG_CONTEXT, { repoSlug }); + } + // Create draft PR to claim the issue const prTitle = `[WIP] Symphony: ${issueTitle} (#${issueNumber})`; const prBody = `## Maestro Symphony Contribution @@ -1501,7 +1527,22 @@ Contributed via [Maestro Symphony](https://runmaestro.ai). This PR will be updated automatically when the Auto Run completes.`; - const prResult = await createDraftPR(localPath, baseBranch, prTitle, prBody); + const forkOwner = forkResult.isFork ? forkResult.forkSlug?.split('/')[0] : undefined; + if (forkResult.isFork) { + logger.info('Creating cross-fork draft PR', LOG_CONTEXT, { + upstreamSlug: repoSlug, + forkSlug: forkResult.forkSlug, + branchName, + }); + } + const prResult = await createDraftPR( + localPath, + baseBranch, + prTitle, + prBody, + forkResult.isFork ? repoSlug : undefined, + forkOwner + ); if (!prResult.success) { // Cleanup await fs.rm(localPath, { recursive: true, force: true }).catch(() => {}); @@ -1535,6 +1576,11 @@ This PR will be updated automatically when the Auto Run completes.`; timeSpent: 0, sessionId, agentType, + isFork: forkResult.isFork, + ...(forkResult.isFork && { + forkSlug: forkResult.forkSlug, + upstreamSlug: repoSlug, + }), }; // Save state @@ -1737,8 +1783,14 @@ This PR will be updated automatically when the Auto Run completes.`; contribution.status = 'completing'; await writeState(app, state); - // Mark PR as ready - const readyResult = await markPRReady(contribution.localPath, contribution.draftPrNumber); + // Mark PR as ready (use upstreamSlug for fork contributions) + const upstreamSlug = + contribution.isFork && contribution.upstreamSlug ? contribution.upstreamSlug : undefined; + const readyResult = await markPRReady( + contribution.localPath, + contribution.draftPrNumber, + upstreamSlug + ); if (!readyResult.success) { contribution.status = 'failed'; contribution.error = readyResult.error; @@ -1759,7 +1811,8 @@ This PR will be updated automatically when the Auto Run completes.`; const commentResult = await postPRComment( contribution.localPath, contribution.draftPrNumber, - commentStats + commentStats, + upstreamSlug ); if (!commentResult.success) { @@ -1989,37 +2042,53 @@ This PR will be updated automatically when the Auto Run completes.`; } } - // First, sync PR info from metadata.json for any active contributions missing it + // First, sync PR info and fork info from metadata.json for active contributions // This handles cases where PR was created but state.json wasn't updated (migration) let prInfoSynced = false; for (const contribution of state.active) { - if (!contribution.draftPrNumber) { - try { - const metadataPath = path.join( - getSymphonyDir(app), - 'contributions', - contribution.id, - 'metadata.json' - ); - const metadataContent = await fs.readFile(metadataPath, 'utf-8'); - const metadata = JSON.parse(metadataContent) as { - prCreated?: boolean; - draftPrNumber?: number; - draftPrUrl?: string; - }; - if (metadata.prCreated && metadata.draftPrNumber) { - // Sync PR info from metadata to state - contribution.draftPrNumber = metadata.draftPrNumber; - contribution.draftPrUrl = metadata.draftPrUrl; - prInfoSynced = true; - logger.info('Synced PR info from metadata to state', LOG_CONTEXT, { - contributionId: contribution.id, - draftPrNumber: metadata.draftPrNumber, - }); - } - } catch { - // Metadata file might not exist - that's okay + // Skip if both PR info and fork info are already synced + if (contribution.draftPrNumber && contribution.isFork !== undefined) { + continue; + } + try { + const metadataPath = path.join( + getSymphonyDir(app), + 'contributions', + contribution.id, + 'metadata.json' + ); + const metadataContent = await fs.readFile(metadataPath, 'utf-8'); + const metadata = JSON.parse(metadataContent) as { + prCreated?: boolean; + draftPrNumber?: number; + draftPrUrl?: string; + isFork?: boolean; + forkSlug?: string; + upstreamSlug?: string; + }; + if (!contribution.draftPrNumber && metadata.prCreated && metadata.draftPrNumber) { + // Sync PR info from metadata to state + contribution.draftPrNumber = metadata.draftPrNumber; + contribution.draftPrUrl = metadata.draftPrUrl; + prInfoSynced = true; + logger.info('Synced PR info from metadata to state', LOG_CONTEXT, { + contributionId: contribution.id, + draftPrNumber: metadata.draftPrNumber, + }); } + // Sync fork info from metadata to state (independent of PR info) + if ( + metadata.isFork && + metadata.forkSlug && + metadata.upstreamSlug && + !contribution.isFork + ) { + contribution.isFork = metadata.isFork; + contribution.forkSlug = metadata.forkSlug; + contribution.upstreamSlug = metadata.upstreamSlug; + } + } catch { + // Metadata file might not exist - that's okay } } @@ -2027,9 +2096,13 @@ This PR will be updated automatically when the Auto Run completes.`; // This handles PRs created manually via gh CLI or GitHub UI for (const contribution of state.active) { if (!contribution.draftPrNumber && contribution.branchName && contribution.repoSlug) { + const forkHeadOwner = contribution.isFork + ? contribution.forkSlug?.split('/')[0] + : undefined; const discovered = await discoverPRByBranch( contribution.repoSlug, - contribution.branchName + contribution.branchName, + forkHeadOwner ); if (discovered.prNumber) { contribution.draftPrNumber = discovered.prNumber; @@ -2172,8 +2245,8 @@ This PR will be updated automatically when the Auto Run completes.`; let prClosed = false; try { - // Step 1: Check if we have PR info in metadata but not in state - if (!contribution.draftPrNumber) { + // Step 1: Check if we have PR info or fork info in metadata but not in state + if (!contribution.draftPrNumber || !contribution.isFork) { const metadataPath = path.join( getSymphonyDir(app), 'contributions', @@ -2186,8 +2259,11 @@ This PR will be updated automatically when the Auto Run completes.`; prCreated?: boolean; draftPrNumber?: number; draftPrUrl?: string; + isFork?: boolean; + forkSlug?: string; + upstreamSlug?: string; }; - if (metadata.prCreated && metadata.draftPrNumber) { + if (!contribution.draftPrNumber && metadata.prCreated && metadata.draftPrNumber) { contribution.draftPrNumber = metadata.draftPrNumber; contribution.draftPrUrl = metadata.draftPrUrl; prCreated = true; @@ -2197,6 +2273,17 @@ This PR will be updated automatically when the Auto Run completes.`; draftPrNumber: metadata.draftPrNumber, }); } + // Sync fork info from metadata to state (independent of PR info) + if ( + metadata.isFork && + metadata.forkSlug && + metadata.upstreamSlug && + !contribution.isFork + ) { + contribution.isFork = metadata.isFork; + contribution.forkSlug = metadata.forkSlug; + contribution.upstreamSlug = metadata.upstreamSlug; + } } catch { // Metadata file might not exist - that's okay, we'll try to create PR } @@ -2205,9 +2292,13 @@ This PR will be updated automatically when the Auto Run completes.`; // Step 2: If still no PR, try to discover it from GitHub by branch name // This handles PRs created manually via gh CLI or GitHub UI if (!contribution.draftPrNumber && contribution.branchName && contribution.repoSlug) { + const forkHeadOwner = contribution.isFork + ? contribution.forkSlug?.split('/')[0] + : undefined; const discovered = await discoverPRByBranch( contribution.repoSlug, - contribution.branchName + contribution.branchName, + forkHeadOwner ); if (discovered.prNumber) { contribution.draftPrNumber = discovered.prNumber; @@ -2523,6 +2614,24 @@ This PR will be updated automatically when the Auto Run completes.`; return { success: false, error: `Failed to create branch: ${branchResult.error}` }; } + // 1b. Capture upstream default branch before fork setup rewrites origin + const upstreamDefaultBranch = await getDefaultBranch(localPath); + + // 1c. Set up fork if user doesn't have push access + logger.info('Checking fork requirements', LOG_CONTEXT, { repoSlug }); + const forkResult = await ensureForkSetup(localPath, repoSlug); + if (forkResult.error) { + return { success: false, error: `Fork setup failed: ${forkResult.error}` }; + } + if (forkResult.isFork) { + logger.info('Using fork for contribution', LOG_CONTEXT, { + forkSlug: forkResult.forkSlug, + upstreamSlug: repoSlug, + }); + } else { + logger.info('User has push access, no fork needed', LOG_CONTEXT, { repoSlug }); + } + // 2. Set up Auto Run documents directory // External docs (GitHub attachments) go to cache dir to avoid polluting the repo // Repo-internal docs are referenced in place @@ -2608,6 +2717,12 @@ This PR will be updated automatically when the Auto Run completes.`; resolvedDocs, startedAt: new Date().toISOString(), prCreated: false, + upstreamDefaultBranch, + isFork: forkResult.isFork, + ...(forkResult.isFork && { + forkSlug: forkResult.forkSlug, + upstreamSlug: repoSlug, + }), }, null, 2 @@ -2626,7 +2741,7 @@ This PR will be updated automatically when the Auto Run completes.`; let draftPrNumber: number | undefined; let draftPrUrl: string | undefined; - const baseBranch = await getDefaultBranch(localPath); + const baseBranch = upstreamDefaultBranch; const commitMsg = `[Symphony] Start contribution for #${issueNumber}`; const emptyCommitResult = await execFileNoThrow( 'git', @@ -2649,7 +2764,22 @@ Contributed via [Maestro Symphony](https://runmaestro.ai). This PR will be updated automatically when the Auto Run completes.`; - const prResult = await createDraftPR(localPath, baseBranch, prTitle, prBody); + const forkOwner = forkResult.isFork ? forkResult.forkSlug?.split('/')[0] : undefined; + if (forkResult.isFork) { + logger.info('Creating cross-fork draft PR', LOG_CONTEXT, { + upstreamSlug: repoSlug, + forkSlug: forkResult.forkSlug, + branchName, + }); + } + const prResult = await createDraftPR( + localPath, + baseBranch, + prTitle, + prBody, + forkResult.isFork ? repoSlug : undefined, + forkOwner + ); if (prResult.success) { draftPrNumber = prResult.prNumber; draftPrUrl = prResult.prUrl; @@ -2749,6 +2879,10 @@ This PR will be updated automatically when the Auto Run completes.`; prCreated: boolean; draftPrNumber?: number; draftPrUrl?: string; + upstreamDefaultBranch?: string; + isFork?: boolean; + forkSlug?: string; + upstreamSlug?: string; }; try { @@ -2785,7 +2919,8 @@ This PR will be updated automatically when the Auto Run completes.`; // Check if there are any commits on this branch // Use rev-list to count commits not in the default branch - const baseBranch = await getDefaultBranch(localPath); + // Prefer persisted upstream default branch (fork setup may have reconfigured origin) + const baseBranch = metadata.upstreamDefaultBranch ?? (await getDefaultBranch(localPath)); const commitCheckResult = await execFileNoThrow( 'git', ['rev-list', '--count', `${baseBranch}..HEAD`], @@ -2823,7 +2958,22 @@ Contributed via [Maestro Symphony](https://runmaestro.ai). This PR will be updated automatically when the Auto Run completes.`; // Create draft PR (this also pushes the branch) - const prResult = await createDraftPR(localPath, baseBranch, prTitle, prBody); + const metaForkOwner = metadata.isFork ? metadata.forkSlug?.split('/')[0] : undefined; + if (metadata.isFork || metadata.upstreamSlug) { + logger.info('Creating cross-fork draft PR', LOG_CONTEXT, { + upstreamSlug: metadata.upstreamSlug, + forkSlug: metadata.forkSlug, + branchName: metadata.branchName, + }); + } + const prResult = await createDraftPR( + localPath, + baseBranch, + prTitle, + prBody, + metadata.upstreamSlug, + metaForkOwner + ); if (!prResult.success) { logger.error('Failed to create draft PR', LOG_CONTEXT, { contributionId, diff --git a/src/main/services/symphony-runner.ts b/src/main/services/symphony-runner.ts index 3720a21ca..59d7469b4 100644 --- a/src/main/services/symphony-runner.ts +++ b/src/main/services/symphony-runner.ts @@ -8,6 +8,7 @@ import path from 'path'; import fs from 'fs/promises'; import { logger } from '../utils/logger'; import { execFileNoThrow } from '../utils/execFile'; +import { ensureForkSetup } from '../utils/symphony-fork'; import type { DocumentReference } from '../../shared/symphony-types'; const LOG_CONTEXT = '[SymphonyRunner]'; @@ -105,7 +106,9 @@ async function pushBranch(localPath: string, branchName: string): Promise { const title = `[WIP] Symphony: ${issueTitle}`; const body = `## Symphony Contribution @@ -118,11 +121,26 @@ Closes #${issueNumber} *Work in progress - will be updated when Auto Run completes*`; - const result = await execFileNoThrow( - 'gh', - ['pr', 'create', '--draft', '--title', title, '--body', body], - localPath - ); + const args = ['pr', 'create', '--draft', '--title', title, '--body', body]; + + if (upstreamSlug) { + args.push('--repo', upstreamSlug); + } + if (upstreamSlug && forkOwner) { + // For cross-fork PRs, --head must specify the fork owner and branch + const branchResult = await execFileNoThrow( + 'git', + ['rev-parse', '--abbrev-ref', 'HEAD'], + localPath + ); + const branchName = branchResult.stdout.trim(); + if (!branchName || branchResult.exitCode !== 0) { + return { success: false, error: 'Failed to determine current branch name' }; + } + args.push('--head', `${forkOwner}:${branchName}`); + } + + const result = await execFileNoThrow('gh', args, localPath); if (result.exitCode !== 0) { return { success: false, error: `PR creation failed: ${result.stderr}` }; @@ -212,10 +230,20 @@ export async function startContribution(options: SymphonyRunnerOptions): Promise draftPrUrl?: string; draftPrNumber?: number; autoRunPath?: string; + isFork?: boolean; + forkSlug?: string; error?: string; }> { - const { repoUrl, localPath, branchName, issueNumber, issueTitle, documentPaths, onStatusChange } = - options; + const { + repoSlug, + repoUrl, + localPath, + branchName, + issueNumber, + issueTitle, + documentPaths, + onStatusChange, + } = options; try { // 1. Clone @@ -231,7 +259,23 @@ export async function startContribution(options: SymphonyRunnerOptions): Promise return { success: false, error: 'Branch creation failed' }; } - // 2.5. Configure git user for commits + // 2.5. Fork setup — detect if user needs a fork for push access + logger.info('Checking fork requirements', LOG_CONTEXT, { repoSlug }); + const forkResult = await ensureForkSetup(localPath, repoSlug); + if (forkResult.error) { + await cleanupLocalRepo(localPath); + return { success: false, error: `Fork setup failed: ${forkResult.error}` }; + } + if (forkResult.isFork) { + logger.info('Using fork for contribution', LOG_CONTEXT, { + forkSlug: forkResult.forkSlug, + upstreamSlug: repoSlug, + }); + } else { + logger.info('User has push access, no fork needed', LOG_CONTEXT, { repoSlug }); + } + + // 2.6. Configure git user for commits await configureGitUser(localPath); // 3. Empty commit @@ -247,8 +291,23 @@ export async function startContribution(options: SymphonyRunnerOptions): Promise return { success: false, error: 'Push failed' }; } - // 5. Create draft PR - const prResult = await createDraftPR(localPath, issueNumber, issueTitle); + // 5. Create draft PR (cross-fork if needed) + const forkOwner = + forkResult.isFork && forkResult.forkSlug ? forkResult.forkSlug.split('/')[0] : undefined; + if (forkResult.isFork) { + logger.info('Creating cross-fork draft PR', LOG_CONTEXT, { + upstreamSlug: repoSlug, + forkSlug: forkResult.forkSlug, + branchName, + }); + } + const prResult = await createDraftPR( + localPath, + issueNumber, + issueTitle, + forkResult.isFork ? repoSlug : undefined, + forkOwner + ); if (!prResult.success) { await cleanupLocalRepo(localPath); return { success: false, error: prResult.error }; @@ -265,6 +324,8 @@ export async function startContribution(options: SymphonyRunnerOptions): Promise draftPrUrl: prResult.prUrl, draftPrNumber: prResult.prNumber, autoRunPath, + isFork: forkResult.isFork, + forkSlug: forkResult.forkSlug, }; } catch (error) { await cleanupLocalRepo(localPath); @@ -282,7 +343,8 @@ export async function finalizeContribution( localPath: string, prNumber: number, issueNumber: number, - issueTitle: string + issueTitle: string, + upstreamSlug?: string ): Promise<{ success: boolean; prUrl?: string; error?: string }> { // Configure git user for commits (in case not already configured) await configureGitUser(localPath); @@ -299,14 +361,16 @@ Processed all Auto Run documents for: ${issueTitle}`; return { success: false, error: `Commit failed: ${commitResult.stderr}` }; } - // Push changes + // Push changes (origin points to fork if ensureForkSetup ran) const pushResult = await execFileNoThrow('git', ['push'], localPath); if (pushResult.exitCode !== 0) { return { success: false, error: `Push failed: ${pushResult.stderr}` }; } // Convert draft to ready for review - const readyResult = await execFileNoThrow('gh', ['pr', 'ready', prNumber.toString()], localPath); + const readyArgs = ['pr', 'ready', prNumber.toString()]; + if (upstreamSlug) readyArgs.push('--repo', upstreamSlug); + const readyResult = await execFileNoThrow('gh', readyArgs, localPath); if (readyResult.exitCode !== 0) { return { success: false, error: `Failed to mark PR ready: ${readyResult.stderr}` }; } @@ -324,14 +388,14 @@ Closes #${issueNumber} *Contributed by the Maestro Symphony community* 🎵`; - await execFileNoThrow('gh', ['pr', 'edit', prNumber.toString(), '--body', body], localPath); + const editArgs = ['pr', 'edit', prNumber.toString(), '--body', body]; + if (upstreamSlug) editArgs.push('--repo', upstreamSlug); + await execFileNoThrow('gh', editArgs, localPath); // Get final PR URL - const prInfoResult = await execFileNoThrow( - 'gh', - ['pr', 'view', prNumber.toString(), '--json', 'url', '-q', '.url'], - localPath - ); + const viewArgs = ['pr', 'view', prNumber.toString(), '--json', 'url', '-q', '.url']; + if (upstreamSlug) viewArgs.push('--repo', upstreamSlug); + const prInfoResult = await execFileNoThrow('gh', viewArgs, localPath); return { success: true, @@ -345,16 +409,24 @@ Closes #${issueNumber} export async function cancelContribution( localPath: string, prNumber: number, - cleanup: boolean = true + cleanup: boolean = true, + upstreamSlug?: string ): Promise<{ success: boolean; error?: string }> { // Close the draft PR - const closeResult = await execFileNoThrow( - 'gh', - ['pr', 'close', prNumber.toString(), '--delete-branch'], - localPath - ); + const closeArgs = ['pr', 'close', prNumber.toString()]; + if (!upstreamSlug) { + // Only delete branch for non-fork PRs — cross-fork delete-branch fails due to permissions + closeArgs.push('--delete-branch'); + } else { + closeArgs.push('--repo', upstreamSlug); + } + const closeResult = await execFileNoThrow('gh', closeArgs, localPath); if (closeResult.exitCode !== 0) { logger.warn('Failed to close PR', LOG_CONTEXT, { prNumber, error: closeResult.stderr }); + return { + success: false, + error: `Failed to close PR #${prNumber}: ${closeResult.stderr || closeResult.stdout}`, + }; } // Clean up local directory using Node.js fs API diff --git a/src/main/utils/symphony-fork.ts b/src/main/utils/symphony-fork.ts new file mode 100644 index 000000000..3711720bc --- /dev/null +++ b/src/main/utils/symphony-fork.ts @@ -0,0 +1,124 @@ +import { execFileNoThrow } from './execFile'; +import { logger } from './logger'; +import { getExpandedEnv } from '../agents/path-prober'; + +const LOG_CONTEXT = '[SymphonyFork]'; + +export interface ForkSetupResult { + isFork: boolean; + forkSlug?: string; + error?: string; +} + +/** + * Ensures the user has push access to the repo, forking if necessary. + * Reconfigures git remotes so `origin` points to the user's fork + * and `upstream` points to the original repo. + */ +export async function ensureForkSetup( + repoPath: string, + repoSlug: string +): Promise { + const env = getExpandedEnv(); + + // 1. Get authenticated GitHub user + logger.info('Checking GitHub authentication', LOG_CONTEXT); + const userResult = await execFileNoThrow('gh', ['api', 'user', '--jq', '.login'], undefined, env); + if (userResult.exitCode !== 0) { + logger.error('GitHub CLI not authenticated', LOG_CONTEXT, { stderr: userResult.stderr }); + return { isFork: false, error: 'GitHub CLI not authenticated' }; + } + const ghUser = userResult.stdout.trim(); + + // 2. Check if user owns the repo + const [owner, repoName] = repoSlug.split('/'); + if (owner === ghUser) { + logger.info('User owns the repo, no fork needed', LOG_CONTEXT); + return { isFork: false }; + } + + // 3. Check push access + logger.info(`Checking push access to ${repoSlug}`, LOG_CONTEXT); + const accessResult = await execFileNoThrow( + 'gh', + ['api', `repos/${repoSlug}`, '--jq', '.permissions.push'], + undefined, + env + ); + if (accessResult.exitCode === 0 && accessResult.stdout.trim() === 'true') { + logger.info('User has push access, no fork needed', LOG_CONTEXT); + return { isFork: false }; + } + + // 4. Fork the repo (idempotent) + logger.info(`Forking ${repoSlug}`, LOG_CONTEXT); + const forkResult = await execFileNoThrow( + 'gh', + ['repo', 'fork', repoSlug, '--clone=false'], + undefined, + env + ); + if (forkResult.exitCode !== 0) { + // gh repo fork returns non-zero if fork already exists but prints to stderr — check for that + const alreadyExists = forkResult.stderr.includes('already exists'); + if (!alreadyExists) { + logger.error('Failed to fork repo', LOG_CONTEXT, { stderr: forkResult.stderr }); + return { isFork: false, error: `Failed to fork repo: ${forkResult.stderr}` }; + } + logger.info('Fork already exists', LOG_CONTEXT); + } + + // 5. Get fork clone URL + const forkSlug = `${ghUser}/${repoName}`; + logger.info(`Getting clone URL for ${forkSlug}`, LOG_CONTEXT); + const urlResult = await execFileNoThrow( + 'gh', + ['api', `repos/${forkSlug}`, '--jq', '.clone_url'], + undefined, + env + ); + if (urlResult.exitCode !== 0) { + logger.error('Failed to get fork clone URL', LOG_CONTEXT, { stderr: urlResult.stderr }); + return { isFork: false, error: `Failed to get fork clone URL: ${urlResult.stderr}` }; + } + const forkCloneUrl = urlResult.stdout.trim(); + + // 6. Reconfigure remotes + logger.info('Reconfiguring git remotes', LOG_CONTEXT); + const renameResult = await execFileNoThrow( + 'git', + ['remote', 'rename', 'origin', 'upstream'], + repoPath + ); + if (renameResult.exitCode !== 0) { + // Fallback: upstream already exists, just set origin URL + logger.warn('Could not rename origin to upstream, trying set-url fallback', LOG_CONTEXT, { + stderr: renameResult.stderr, + }); + const setUrlResult = await execFileNoThrow( + 'git', + ['remote', 'set-url', 'origin', forkCloneUrl], + repoPath + ); + if (setUrlResult.exitCode !== 0) { + logger.error('Failed to set origin URL', LOG_CONTEXT, { stderr: setUrlResult.stderr }); + return { isFork: false, error: `Failed to reconfigure remotes: ${setUrlResult.stderr}` }; + } + } else { + const addResult = await execFileNoThrow( + 'git', + ['remote', 'add', 'origin', forkCloneUrl], + repoPath + ); + if (addResult.exitCode !== 0) { + logger.error('Failed to add origin remote', LOG_CONTEXT, { stderr: addResult.stderr }); + return { isFork: false, error: `Failed to add origin remote: ${addResult.stderr}` }; + } + } + + // Set HEAD for origin so getDefaultBranch() works correctly with fork remotes + await execFileNoThrow('git', ['remote', 'set-head', 'origin', '-a'], repoPath); + + logger.info(`Fork setup complete: ${forkSlug}`, LOG_CONTEXT); + return { isFork: true, forkSlug }; +} diff --git a/src/shared/symphony-types.ts b/src/shared/symphony-types.ts index 7fac754d8..27dde6ab7 100644 --- a/src/shared/symphony-types.ts +++ b/src/shared/symphony-types.ts @@ -188,6 +188,12 @@ export interface ActiveContribution { agentType: string; /** Error details if failed */ error?: string; + /** Whether this contribution uses a fork (user lacks push access to upstream) */ + isFork?: boolean; + /** The user's fork slug (e.g., "chris/repo-name") */ + forkSlug?: string; + /** The original upstream repo slug (e.g., "owner/repo-name") */ + upstreamSlug?: string; } // ============================================================================