Skip to content

SYMPHONY: Implement fork detection and integration in Symphony workflows #533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chr1syy wants to merge 10 commits into
base: main
Choose a base branch
from
Open

SYMPHONY: Implement fork detection and integration in Symphony workflows #533

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d64cf57
MAESTRO: add fork detection types and ensureForkSetup utility
chr1syy Mar 8, 2026
0e2967b
MAESTRO: integrate fork setup into Symphony IPC handlers
chr1syy Mar 8, 2026
4640182
MAESTRO: wire fork support into Symphony runner service
chr1syy Mar 8, 2026
5f0dc0f
MAESTRO: fork-aware completion handlers and fork workflow logging
chr1syy Mar 8, 2026
42c649b
fix: prettier formatting for symphony fork support files
chr1syy Mar 8, 2026
84732bd
fix: address PR review feedback for symphony fork support
chr1syy Mar 8, 2026
caab3fa
fix: address PR review feedback round 2 for symphony fork support
chr1syy Mar 8, 2026
3f3e57c
fix: cross-fork PR discovery and deferred PR base branch handling
chr1syy Mar 8, 2026
6ab5f87
test: add missing fork test assertions for upstreamDefaultBranch and …
chr1syy Mar 8, 2026
319c9bf
test: add guard assertions for --head and --base index lookups in for…
chr1syy Mar 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
438 changes: 438 additions & 0 deletions src/__tests__/main/ipc/handlers/symphony.test.ts
View file
Open in desktop

Large diffs are not rendered by default.

210 changes: 205 additions & 5 deletions src/__tests__/main/services/symphony-runner.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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',
Expand All @@ -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 () => {
Expand All @@ -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',
Expand All @@ -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'
);
});
});
});
});
Loading