-
Notifications
You must be signed in to change notification settings - Fork 243
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.
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
Changes from 5 commits
d64cf57
0e2967b
4640182
5f0dc0f
42c649b
84732bd
caab3fa
3f3e57c
6ab5f87
319c9bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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(() => { | ||||||
|
|
@@ -1334,5 +1343,200 @@ describe('Symphony Runner Service', () => { | |||||
|
|
||||||
| expect(result.success).toBe(true); | ||||||
| }); | ||||||
|
|
||||||
| it('adds --repo flag 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', '--delete-branch', '--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'); | ||||||
| }); | ||||||
|
|
||||||
| 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 HEAD | ||||||
|
||||||
| .mockResolvedValueOnce({ stdout: 'symphony/test-branch', stderr: '', exitCode: 0 }) // git rev-parse HEAD | |
| .mockResolvedValueOnce({ stdout: 'symphony/test-branch', stderr: '', exitCode: 0 }) // git rev-parse --abbrev-ref HEAD |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
| import type { ExecResult } from '../../../main/utils/execFile'; | ||
|
|
||
| const mockExecFileNoThrow = vi.fn<(...args: unknown[]) => Promise<ExecResult>>(); | ||
|
|
||
| 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 | ||
|
|
||
| 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' | ||
| ); | ||
| }); | ||
|
|
||
| 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 | ||
|
|
||
| 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) | ||
|
|
||
| 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'); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.