diff --git a/src/main/github/client-issue-source.test.ts b/src/main/github/client-issue-source.test.ts index 7c64d5fd184..7b9082efc2c 100644 --- a/src/main/github/client-issue-source.test.ts +++ b/src/main/github/client-issue-source.test.ts @@ -9,6 +9,7 @@ const { execFileAsyncMock, ghExecFileAsyncMock, getOwnerRepoMock, + getOriginOwnerRepoMock, getIssueOwnerRepoMock, getOwnerRepoForRemoteMock, resolveIssueSourceMock, @@ -20,6 +21,7 @@ const { execFileAsyncMock: vi.fn(), ghExecFileAsyncMock: vi.fn(), getOwnerRepoMock: vi.fn(), + getOriginOwnerRepoMock: vi.fn(), getIssueOwnerRepoMock: vi.fn(), getOwnerRepoForRemoteMock: vi.fn(), resolveIssueSourceMock: vi.fn(), @@ -37,9 +39,11 @@ vi.mock('./gh-utils', async () => { ghExecFileAsync: ghExecFileAsyncMock, getOwnerRepo: getOwnerRepoMock, // Why: resolvePrWorkItemSource resolves the raw origin candidate via - // getOriginOwnerRepo; map it to the same mock these tests use for the - // origin PR repo. - getOriginOwnerRepo: getOwnerRepoMock, + // getOriginOwnerRepo (distinct from upstream-first getOwnerRepo). Keep it a + // separate mock so a regression that collapses originCandidate back onto + // getOwnerRepo is observable; it delegates to getOwnerRepoMock by default + // (see beforeEach) so existing origin-candidate setups keep working. + getOriginOwnerRepo: getOriginOwnerRepoMock, getIssueOwnerRepo: getIssueOwnerRepoMock, getOwnerRepoForRemote: getOwnerRepoForRemoteMock, resolveIssueSource: resolveIssueSourceMock, @@ -61,6 +65,7 @@ describe('GitHub issue source split', () => { execFileAsyncMock.mockReset() ghExecFileAsyncMock.mockReset() getOwnerRepoMock.mockReset() + getOriginOwnerRepoMock.mockReset() getIssueOwnerRepoMock.mockReset() getOwnerRepoForRemoteMock.mockReset() resolveIssueSourceMock.mockReset() @@ -79,6 +84,13 @@ describe('GitHub issue source split', () => { source: await getIssueOwnerRepoMock(), fellBack: false })) + // Why: originCandidate is resolved via getOriginOwnerRepo. Delegate to + // getOwnerRepoMock by default so the many existing tests that queue the + // origin PR repo on getOwnerRepoMock keep working unchanged; tests that + // must distinguish the two (the revert-guard below) override it. + getOriginOwnerRepoMock.mockImplementation((...args: unknown[]) => + (getOwnerRepoMock as (...a: unknown[]) => Promise)(...args) + ) // Default the upstream-candidate lookup to null so existing tests that // only mock `getIssueOwnerRepo` + `getOwnerRepo` don't need to think // about it. Tests that care set it explicitly. @@ -689,5 +701,28 @@ describe('GitHub issue source split', () => { upstreamCandidate: { owner: 'stablyai', repo: 'orca' } }) }) + + it('resolves originCandidate via raw origin, not upstream-first getOwnerRepo', async () => { + // Why: regression guard for the fork toggle. originCandidate must be the + // raw `origin` remote so the renderer's IssueSourceSelector keeps + // rendering (it gates on originCandidate && upstreamCandidate && distinct). + // If this collapses back onto upstream-first getOwnerRepo, originCandidate + // would equal upstreamCandidate and the toggle vanishes for every fork. + resolveIssueSourceMock.mockResolvedValueOnce({ + source: { owner: 'stablyai', repo: 'orca' }, + fellBack: false + }) + getOriginOwnerRepoMock.mockResolvedValueOnce({ owner: 'fork', repo: 'orca' }) + // What upstream-first getOwnerRepo would return — must NOT reach originCandidate. + getOwnerRepoMock.mockResolvedValue({ owner: 'stablyai', repo: 'orca' }) + getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) + ghExecFileAsyncMock.mockResolvedValue({ stdout: '[]' }) + + const result = await listWorkItems('/repo-root', 10, 'is:pr', undefined, 'upstream') + + expect(result.sources.originCandidate).toEqual({ owner: 'fork', repo: 'orca' }) + expect(result.sources.upstreamCandidate).toEqual({ owner: 'stablyai', repo: 'orca' }) + expect(getOriginOwnerRepoMock).toHaveBeenCalled() + }) }) }) diff --git a/src/main/github/client-work-items.test.ts b/src/main/github/client-work-items.test.ts index f39f2207295..d54b6780f6a 100644 --- a/src/main/github/client-work-items.test.ts +++ b/src/main/github/client-work-items.test.ts @@ -7,6 +7,7 @@ const { execFileAsyncMock, ghExecFileAsyncMock, getOwnerRepoMock, + getOriginOwnerRepoMock, getIssueOwnerRepoMock, getOwnerRepoForRemoteMock, resolveIssueSourceMock, @@ -19,6 +20,7 @@ const { execFileAsyncMock: vi.fn(), ghExecFileAsyncMock: vi.fn(), getOwnerRepoMock: vi.fn(), + getOriginOwnerRepoMock: vi.fn(), getIssueOwnerRepoMock: vi.fn(), getOwnerRepoForRemoteMock: vi.fn(), resolveIssueSourceMock: vi.fn(), @@ -51,8 +53,10 @@ vi.mock('./gh-utils', () => ({ : { cwd: context.repoPath, ...(context.wslDistro ? { wslDistro: context.wslDistro } : {}) }, getOwnerRepo: getOwnerRepoMock, // Why: resolvePrWorkItemSource resolves the raw origin candidate via - // getOriginOwnerRepo; map it to the same mock these tests use for origin. - getOriginOwnerRepo: getOwnerRepoMock, + // getOriginOwnerRepo (distinct from upstream-first getOwnerRepo). Keep a + // separate mock; it delegates to getOwnerRepoMock by default (see beforeEach) + // so existing origin-candidate setups keep working. + getOriginOwnerRepo: getOriginOwnerRepoMock, getIssueOwnerRepo: getIssueOwnerRepoMock, getOwnerRepoForRemote: getOwnerRepoForRemoteMock, resolveIssueSource: resolveIssueSourceMock, @@ -85,6 +89,7 @@ describe('listWorkItems', () => { execFileAsyncMock.mockReset() ghExecFileAsyncMock.mockReset() getOwnerRepoMock.mockReset() + getOriginOwnerRepoMock.mockReset() getIssueOwnerRepoMock.mockReset() getOwnerRepoForRemoteMock.mockReset() resolveIssueSourceMock.mockReset() @@ -102,6 +107,12 @@ describe('listWorkItems', () => { source: await getIssueOwnerRepoMock(), fellBack: false })) + // Why: originCandidate is resolved via getOriginOwnerRepo. Delegate to + // getOwnerRepoMock by default so existing tests that queue the origin PR + // repo on getOwnerRepoMock keep working unchanged. + getOriginOwnerRepoMock.mockImplementation((...args: unknown[]) => + (getOwnerRepoMock as (...a: unknown[]) => Promise)(...args) + ) getOwnerRepoForRemoteMock.mockResolvedValue(null) _resetOwnerRepoCache() _resetMergeQueueCacheForTests() diff --git a/src/main/github/gh-utils.test.ts b/src/main/github/gh-utils.test.ts index 397b934c37c..0ecf9e6cd5c 100644 --- a/src/main/github/gh-utils.test.ts +++ b/src/main/github/gh-utils.test.ts @@ -23,6 +23,7 @@ import { classifyGhError, classifyListIssuesError, getIssueOwnerRepo, + getOriginOwnerRepo, getOwnerRepo, getOwnerRepoForRemote, parseGitHubRemoteIdentity, @@ -144,12 +145,15 @@ describe('github owner/repo resolution', () => { }) it('does not mix origin and upstream cache entries for the same repo path', async () => { - // getOwnerRepo and getIssueOwnerRepo both resolve upstream-first now + // Why: getOriginOwnerRepo probes only `origin`; getIssueOwnerRepo is + // upstream-first. Distinct owner/repo values per remote prove the cache + // keys stay separate — a mixing regression would return `fork/orca` for + // the upstream lookup. Also gives getOriginOwnerRepo direct coverage. gitExecFileAsyncMock - .mockResolvedValueOnce({ stdout: 'git@github.com:stablyai/orca.git\n' }) + .mockResolvedValueOnce({ stdout: 'git@github.com:fork/orca.git\n' }) .mockResolvedValueOnce({ stdout: 'git@github.com:stablyai/orca.git\n' }) - await expect(getOwnerRepo('/repo')).resolves.toEqual({ owner: 'stablyai', repo: 'orca' }) + await expect(getOriginOwnerRepo('/repo')).resolves.toEqual({ owner: 'fork', repo: 'orca' }) await expect(getIssueOwnerRepo('/repo')).resolves.toEqual({ owner: 'stablyai', repo: 'orca' }) })