From c10b77d160452120f8dbfdb0d9df8a9df0634bcf Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 25 Jun 2026 13:00:44 -0700 Subject: [PATCH] Reduce Git refresh subprocess fanout (#6324) Co-authored-by: Orca --- src/main/github/client.test.ts | 576 +++++++++++++++++- src/main/github/client.ts | 228 +++++-- src/main/github/conflict-summary.ts | 50 +- src/main/github/gh-utils.test.ts | 366 +++++++++++ .../github/github-remote-identity-parsing.ts | 49 ++ src/main/github/github-repository-identity.ts | 111 ++-- src/main/github/local-git-config-signature.ts | 239 ++++++++ .../github/stable-missing-git-remote-error.ts | 16 + src/main/ipc/worktrees.test.ts | 49 ++ src/main/ipc/worktrees.ts | 73 ++- 10 files changed, 1608 insertions(+), 149 deletions(-) create mode 100644 src/main/github/github-remote-identity-parsing.ts create mode 100644 src/main/github/local-git-config-signature.ts create mode 100644 src/main/github/stable-missing-git-remote-error.ts diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index 9e6e5eb92bf..a80e5ae18d0 100644 --- a/src/main/github/client.test.ts +++ b/src/main/github/client.test.ts @@ -20,6 +20,7 @@ const { ghRepoExecOptionsMock, githubRepoContextMock, getSshGitProviderMock, + readLocalGitConfigSignatureMock, acquireMock, releaseMock } = vi.hoisted(() => ({ @@ -48,6 +49,7 @@ const { ...localGitOptions })), getSshGitProviderMock: vi.fn(), + readLocalGitConfigSignatureMock: vi.fn(), acquireMock: vi.fn(), releaseMock: vi.fn() })) @@ -90,6 +92,10 @@ vi.mock('../providers/ssh-git-dispatch', () => ({ getSshGitProvider: getSshGitProviderMock })) +vi.mock('./local-git-config-signature', () => ({ + readLocalGitConfigSignature: readLocalGitConfigSignatureMock +})) + vi.mock('./rate-limit', () => ({ getRateLimit: getRateLimitMock, rateLimitGuard: rateLimitGuardMock, @@ -113,6 +119,7 @@ import { _resetMergeQueueCacheForTests, __resetTrackedUpstreamBranchCacheForTests } from './client' +import { __resetPRConflictSummaryGitCapabilityCacheForTests } from './conflict-summary' describe('checkOrcaStarred', () => { beforeEach(() => { @@ -175,12 +182,15 @@ describe('getPRForBranch', () => { ghRepoExecOptionsMock.mockClear() githubRepoContextMock.mockClear() getSshGitProviderMock.mockReset() + readLocalGitConfigSignatureMock.mockReset() + readLocalGitConfigSignatureMock.mockResolvedValue(undefined) acquireMock.mockReset() releaseMock.mockReset() acquireMock.mockResolvedValue(undefined) _resetOwnerRepoCache() _resetMergeQueueCacheForTests() __resetTrackedUpstreamBranchCacheForTests() + __resetPRConflictSummaryGitCapabilityCacheForTests() }) it('queries GitHub by head branch when the remote is on github.com', async () => { @@ -959,7 +969,7 @@ describe('getPRForBranch', () => { }) getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'fork-owner', repo: 'orca' }) gitExecFileAsyncMock.mockResolvedValueOnce({ - stdout: 'fork/contributor/original\n', + stdout: 'local-created-from-pr\0fork/contributor/original\n', stderr: '' }) ghExecFileAsyncMock @@ -1122,14 +1132,14 @@ describe('getPRForBranch', () => { }) }) gitExecFileAsyncMock.mockResolvedValueOnce({ - stdout: 'origin/contributor/original\n', + stdout: 'local-created-from-pr\0origin/contributor/original\n', stderr: '' }) const pr = await getPRForBranch('/repo-root', 'local-created-from-pr') expect(gitExecFileAsyncMock).toHaveBeenCalledWith( - ['rev-parse', '--abbrev-ref', '--symbolic-full-name', 'local-created-from-pr@{upstream}'], + ['for-each-ref', '--format=%(refname)%00%(upstream)', 'refs/heads'], { cwd: '/repo-root' } ) expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( @@ -1168,20 +1178,368 @@ describe('getPRForBranch', () => { headRepo: { owner: 'acme', repo: 'widgets' } }) ghExecFileAsyncMock.mockResolvedValue({ stdout: JSON.stringify([]) }) - gitExecFileAsyncMock.mockRejectedValue( - new Error("fatal: no upstream configured for branch 'no-pr-branch'") - ) + gitExecFileAsyncMock.mockResolvedValue({ stdout: 'no-pr-branch\0\n', stderr: '' }) await getPRForBranch('/repo-root', 'no-pr-branch') await getPRForBranch('/repo-root', 'no-pr-branch') await getPRForBranch('/repo-root', 'no-pr-branch') const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => - (args as string[]).includes('no-pr-branch@{upstream}') + (args as string[]).includes('refs/heads') ) expect(trackedUpstreamCalls).toHaveLength(1) }) + it('does not fan out tracked-upstream probes after a transient for-each-ref failure', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValue({ + candidates: [{ owner: 'acme', repo: 'widgets' }], + headRepo: { owner: 'acme', repo: 'widgets' } + }) + ghExecFileAsyncMock.mockResolvedValue({ stdout: JSON.stringify([]) }) + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error('fatal: cannot lock ref')) + .mockResolvedValue({ stdout: 'alpha\0\nbeta\0\ngamma\0\n', stderr: '' }) + + await getPRForBranch('/repo-root', 'alpha') + await getPRForBranch('/repo-root', 'beta') + await getPRForBranch('/repo-root', 'gamma') + + const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => + (args as string[]).includes('refs/heads') + ) + expect(trackedUpstreamCalls).toHaveLength(2) + }) + + it('refreshes the tracked-upstream snapshot when a branch appears inside the TTL', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValue({ + candidates: [{ owner: 'acme', repo: 'widgets' }], + headRepo: { owner: 'acme', repo: 'widgets' } + }) + getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'existing\0\n', stderr: '' }) + .mockResolvedValueOnce({ + stdout: 'existing\0\nnew-feature\0origin/contributor/original\n', + stderr: '' + }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 78, + title: 'New branch upstream PR', + state: 'open', + html_url: 'https://github.com/acme/widgets/pull/78', + updated_at: '2026-03-28T00:00:00Z', + draft: false, + mergeable: true, + base: { ref: 'main', sha: 'base-oid' }, + head: { ref: 'contributor/original', sha: 'upstream-head-oid' } + } + ]) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 78, + title: 'Hydrated new branch upstream PR', + state: 'OPEN', + url: 'https://github.com/acme/widgets/pull/78', + statusCheckRollup: [], + updatedAt: '2026-03-28T00:00:00Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'contributor/original', + baseRefOid: 'base-oid', + headRefOid: 'upstream-head-oid' + }) + }) + + await getPRForBranch('/repo-root', 'existing') + const pr = await getPRForBranch('/repo-root', 'new-feature') + + const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => + (args as string[]).includes('refs/heads') + ) + expect(trackedUpstreamCalls).toHaveLength(2) + expect(pr).toMatchObject({ + number: 78, + title: 'Hydrated new branch upstream PR' + }) + }) + + it('parses full local branch refs from the tracked-upstream snapshot', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: 'refs/heads/feature\0origin/contributor/original\n', + stderr: '' + }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 80, + title: 'Ambiguous ref upstream PR', + state: 'open', + html_url: 'https://github.com/acme/widgets/pull/80', + updated_at: '2026-03-28T00:00:00Z', + draft: false, + mergeable: true, + base: { ref: 'main', sha: 'base-oid' }, + head: { ref: 'contributor/original', sha: 'upstream-head-oid' } + } + ]) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 80, + title: 'Hydrated ambiguous ref upstream PR', + state: 'OPEN', + url: 'https://github.com/acme/widgets/pull/80', + statusCheckRollup: [], + updatedAt: '2026-03-28T00:00:00Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'contributor/original', + baseRefOid: 'base-oid', + headRefOid: 'upstream-head-oid' + }) + }) + + const pr = await getPRForBranch('/repo-root', 'feature') + + expect(pr).toMatchObject({ + number: 80, + title: 'Hydrated ambiguous ref upstream PR' + }) + }) + + it('parses full upstream refs from the tracked-upstream snapshot', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'fork-owner', repo: 'widgets' }) + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: 'refs/heads/feature\0refs/remotes/fork/contributor/original\n', + stderr: '' + }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 83, + title: 'Full upstream ref PR', + state: 'open', + html_url: 'https://github.com/acme/widgets/pull/83', + updated_at: '2026-03-28T00:00:00Z', + draft: false, + mergeable: true, + base: { ref: 'main', sha: 'base-oid' }, + head: { ref: 'contributor/original', sha: 'upstream-head-oid' } + } + ]) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 83, + title: 'Hydrated full upstream ref PR', + state: 'OPEN', + url: 'https://github.com/acme/widgets/pull/83', + statusCheckRollup: [], + updatedAt: '2026-03-28T00:00:00Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'contributor/original', + baseRefOid: 'base-oid', + headRefOid: 'upstream-head-oid' + }) + }) + + const pr = await getPRForBranch('/repo-root', 'feature') + + expect(getOwnerRepoForRemoteMock).toHaveBeenCalledWith('/repo-root', 'fork', undefined) + expect(pr).toMatchObject({ + number: 83, + title: 'Hydrated full upstream ref PR', + headRepo: { owner: 'fork-owner', repo: 'widgets' } + }) + }) + + it('ignores full local-branch upstream refs from the tracked-upstream snapshot', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: 'refs/heads/feature\0refs/heads/main\n', + stderr: '' + }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + + const pr = await getPRForBranch('/repo-root', 'feature') + + expect(pr).toBeNull() + expect(getOwnerRepoForRemoteMock).not.toHaveBeenCalled() + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + }) + + it('invalidates the local tracked-upstream snapshot when git config changes', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValue({ + candidates: [{ owner: 'acme', repo: 'widgets' }], + headRepo: { owner: 'acme', repo: 'widgets' } + }) + getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + readLocalGitConfigSignatureMock + .mockResolvedValueOnce('/repo-root/.git/config\u0000mtime-a\u0000100') + .mockResolvedValueOnce('/repo-root/.git/config\u0000mtime-a\u0000100') + .mockResolvedValueOnce('/repo-root/.git/config\u0000mtime-b\u0000120') + .mockResolvedValueOnce('/repo-root/.git/config\u0000mtime-b\u0000120') + .mockResolvedValueOnce('/repo-root/.git/config\u0000mtime-b\u0000120') + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\0\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'feature\0origin/contributor/original\n', stderr: '' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 79, + title: 'Reconfigured upstream PR', + state: 'open', + html_url: 'https://github.com/acme/widgets/pull/79', + updated_at: '2026-03-28T00:00:00Z', + draft: false, + mergeable: true, + base: { ref: 'main', sha: 'base-oid' }, + head: { ref: 'contributor/original', sha: 'upstream-head-oid' } + } + ]) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 79, + title: 'Hydrated reconfigured upstream PR', + state: 'OPEN', + url: 'https://github.com/acme/widgets/pull/79', + statusCheckRollup: [], + updatedAt: '2026-03-28T00:00:00Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'contributor/original', + baseRefOid: 'base-oid', + headRefOid: 'upstream-head-oid' + }) + }) + + await getPRForBranch('/repo-root', 'feature') + const pr = await getPRForBranch('/repo-root', 'feature') + + const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => + (args as string[]).includes('refs/heads') + ) + expect(trackedUpstreamCalls).toHaveLength(2) + expect(pr).toMatchObject({ + number: 79, + title: 'Hydrated reconfigured upstream PR' + }) + }) + + it('does not cache positive tracked-upstream entries when config changes during the snapshot', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValue({ + candidates: [{ owner: 'acme', repo: 'widgets' }], + headRepo: { owner: 'acme', repo: 'widgets' } + }) + getOwnerRepoForRemoteMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' }) + readLocalGitConfigSignatureMock + .mockResolvedValueOnce('/repo-root/.git/config\u0000mtime-a\u0000100') + .mockResolvedValueOnce('/repo-root/.git/config\u0000mtime-b\u0000120') + .mockResolvedValueOnce('/repo-root/.git/config\u0000mtime-b\u0000120') + .mockResolvedValueOnce('/repo-root/.git/config\u0000mtime-b\u0000120') + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\0origin/old-upstream\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'feature\0origin/contributor/original\n', stderr: '' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 82, + title: 'Stable config upstream PR', + state: 'open', + html_url: 'https://github.com/acme/widgets/pull/82', + updated_at: '2026-03-28T00:00:00Z', + draft: false, + mergeable: true, + base: { ref: 'main', sha: 'base-oid' }, + head: { ref: 'contributor/original', sha: 'upstream-head-oid' } + } + ]) + }) + + await getPRForBranch('/repo-root', 'feature') + const pr = await getPRForBranch('/repo-root', 'feature') + + const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => + (args as string[]).includes('refs/heads') + ) + expect(trackedUpstreamCalls).toHaveLength(2) + expect(pr).toMatchObject({ + number: 82, + title: 'Stable config upstream PR' + }) + }) + + it('does not cache null tracked-upstream entries when config changes during the snapshot', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValue({ + candidates: [{ owner: 'acme', repo: 'widgets' }], + headRepo: { owner: 'acme', repo: 'widgets' } + }) + getOwnerRepoForRemoteMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' }) + readLocalGitConfigSignatureMock + .mockResolvedValueOnce('/repo-root/.git/config\u0000mtime-a\u0000100') + .mockResolvedValueOnce('/repo-root/.git/config\u0000mtime-b\u0000120') + .mockResolvedValueOnce('/repo-root/.git/config\u0000mtime-b\u0000120') + .mockResolvedValueOnce('/repo-root/.git/config\u0000mtime-b\u0000120') + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\0\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'feature\0origin/contributor/original\n', stderr: '' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 85, + title: 'Unstable config upstream PR', + state: 'open', + html_url: 'https://github.com/acme/widgets/pull/85', + updated_at: '2026-03-28T00:00:00Z', + draft: false, + mergeable: true, + base: { ref: 'main', sha: 'base-oid' }, + head: { ref: 'contributor/original', sha: 'upstream-head-oid' } + } + ]) + }) + + await getPRForBranch('/repo-root', 'feature') + const pr = await getPRForBranch('/repo-root', 'feature') + + const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => + (args as string[]).includes('refs/heads') + ) + expect(trackedUpstreamCalls).toHaveLength(2) + expect(pr).toMatchObject({ + number: 85, + title: 'Unstable config upstream PR' + }) + }) + it('coalesces concurrent missing tracked-upstream probes', async () => { resolvePRRepositoryCandidatesMock.mockResolvedValue({ candidates: [{ owner: 'acme', repo: 'widgets' }], @@ -1190,7 +1548,7 @@ describe('getPRForBranch', () => { ghExecFileAsyncMock.mockResolvedValue({ stdout: JSON.stringify([]) }) gitExecFileAsyncMock.mockImplementation(async () => { await Promise.resolve() - throw new Error("fatal: no upstream configured for branch 'no-pr-branch'") + return { stdout: 'no-pr-branch\0\n', stderr: '' } }) await Promise.all([ @@ -1200,20 +1558,71 @@ describe('getPRForBranch', () => { ]) const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => - (args as string[]).includes('no-pr-branch@{upstream}') + (args as string[]).includes('refs/heads') ) expect(trackedUpstreamCalls).toHaveLength(1) }) + it('does not cache synthetic nulls from concurrent tracked-upstream waiters', async () => { + let resolveSnapshot: (value: { stdout: string; stderr: string }) => void + resolvePRRepositoryCandidatesMock.mockResolvedValue({ + candidates: [{ owner: 'acme', repo: 'widgets' }], + headRepo: { owner: 'acme', repo: 'widgets' } + }) + getOwnerRepoForRemoteMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' }) + readLocalGitConfigSignatureMock.mockResolvedValue( + '/repo-root/.git/config\u0000mtime-a\u0000100' + ) + gitExecFileAsyncMock + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSnapshot = resolve + }) + ) + .mockResolvedValueOnce({ stdout: 'new-feature\0origin/contributor/original\n', stderr: '' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 84, + title: 'Concurrent waiter upstream PR', + state: 'open', + html_url: 'https://github.com/acme/widgets/pull/84', + updated_at: '2026-03-28T00:00:00Z', + draft: false, + mergeable: true, + base: { ref: 'main', sha: 'base-oid' }, + head: { ref: 'contributor/original', sha: 'upstream-head-oid' } + } + ]) + }) + + const existingLookup = getPRForBranch('/repo-root', 'existing') + const waiterLookup = getPRForBranch('/repo-root', 'new-feature') + await vi.waitFor(() => expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1)) + resolveSnapshot!({ stdout: 'existing\0\n', stderr: '' }) + const [, waiterPr] = await Promise.all([existingLookup, waiterLookup]) + + const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => + (args as string[]).includes('refs/heads') + ) + expect(trackedUpstreamCalls).toHaveLength(2) + expect(waiterPr).toMatchObject({ + number: 84, + title: 'Concurrent waiter upstream PR' + }) + }) + it('keeps missing tracked-upstream probes separate for host and WSL runtimes', async () => { resolvePRRepositoryCandidatesMock.mockResolvedValue({ candidates: [{ owner: 'acme', repo: 'widgets' }], headRepo: { owner: 'acme', repo: 'widgets' } }) ghExecFileAsyncMock.mockResolvedValue({ stdout: JSON.stringify([]) }) - gitExecFileAsyncMock.mockRejectedValue( - new Error("fatal: no upstream configured for branch 'no-pr-branch'") - ) + gitExecFileAsyncMock.mockResolvedValue({ stdout: 'no-pr-branch\0\n', stderr: '' }) await getPRForBranch('/repo-root', 'no-pr-branch') await getPRForBranch('/repo-root', 'no-pr-branch', null, null, null, { @@ -1225,7 +1634,7 @@ describe('getPRForBranch', () => { }) const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => - (args as string[]).includes('no-pr-branch@{upstream}') + (args as string[]).includes('refs/heads') ) expect(trackedUpstreamCalls).toHaveLength(2) expect(trackedUpstreamCalls[0][1]).toEqual({ cwd: '/repo-root' }) @@ -1245,14 +1654,14 @@ describe('getPRForBranch', () => { ghExecFileAsyncMock.mockResolvedValue({ stdout: JSON.stringify([]) }) gitExecFileAsyncMock .mockRejectedValueOnce(new Error("fatal: no upstream configured for branch 'feature'")) - .mockResolvedValueOnce({ stdout: 'origin/contributor/original\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'feature\0origin/contributor/original\n', stderr: '' }) await getPRForBranch('/repo-root', 'feature') await vi.advanceTimersByTimeAsync(30_001) await getPRForBranch('/repo-root', 'feature') const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => - (args as string[]).includes('feature@{upstream}') + (args as string[]).includes('refs/heads') ) expect(trackedUpstreamCalls).toHaveLength(2) } finally { @@ -1304,7 +1713,7 @@ describe('getPRForBranch', () => { }) }) gitExecFileAsyncMock.mockResolvedValueOnce({ - stdout: 'fork/contributor/original\n', + stdout: 'local-created-from-pr\0fork/contributor/original\n', stderr: '' }) @@ -1330,7 +1739,7 @@ describe('getPRForBranch', () => { it('checks the tracked upstream branch through the SSH git provider', async () => { const sshGitProvider = { exec: vi.fn().mockResolvedValue({ - stdout: 'origin/contributor/original\n', + stdout: 'local-created-from-pr\0origin/contributor/original\n', stderr: '' }) } @@ -1363,7 +1772,7 @@ describe('getPRForBranch', () => { ) expect(sshGitProvider.exec).toHaveBeenCalledWith( - ['rev-parse', '--abbrev-ref', '--symbolic-full-name', 'local-created-from-pr@{upstream}'], + ['for-each-ref', '--format=%(refname)%00%(upstream)', 'refs/heads'], '/remote/repo-root' ) expect(gitExecFileAsyncMock).not.toHaveBeenCalled() @@ -1375,6 +1784,77 @@ describe('getPRForBranch', () => { expect(pr).toMatchObject({ number: 78, title: 'SSH upstream branch PR' }) }) + it('caches positive tracked-upstream entries for unsigned SSH runtimes during PR refresh polling', async () => { + const sshGitProvider = { + exec: vi.fn().mockResolvedValue({ + stdout: 'refs/heads/feature\0origin/contributor/original\n', + stderr: '' + }) + } + getSshGitProviderMock.mockReturnValue(sshGitProvider) + getOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' }) + getOwnerRepoForRemoteMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock.mockResolvedValue({ stdout: JSON.stringify([]) }) + + await getPRForBranch('/remote/repo-root', 'feature', undefined, 'ssh-1') + await getPRForBranch('/remote/repo-root', 'feature', undefined, 'ssh-1') + await getPRForBranch('/remote/repo-root', 'feature', undefined, 'ssh-1') + + expect(sshGitProvider.exec).toHaveBeenCalledTimes(1) + }) + + it('refreshes positive tracked-upstream entries for unsigned SSH runtimes after the TTL', async () => { + vi.useFakeTimers() + try { + const sshGitProvider = { + exec: vi + .fn() + .mockResolvedValueOnce({ + stdout: 'refs/heads/feature\0origin/old-upstream\n', + stderr: '' + }) + .mockResolvedValueOnce({ + stdout: 'refs/heads/feature\0origin/contributor/original\n', + stderr: '' + }) + } + getSshGitProviderMock.mockReturnValue(sshGitProvider) + getOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' }) + getOwnerRepoForRemoteMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 81, + title: 'Fresh SSH upstream PR', + state: 'open', + html_url: 'https://github.com/acme/widgets/pull/81', + updated_at: '2026-03-28T00:00:00Z', + draft: false, + mergeable: true, + base: { ref: 'main', sha: 'base-oid' }, + head: { ref: 'contributor/original', sha: 'upstream-head-oid' } + } + ]) + }) + + await getPRForBranch('/remote/repo-root', 'feature', undefined, 'ssh-1') + await vi.advanceTimersByTimeAsync(30_001) + const pr = await getPRForBranch('/remote/repo-root', 'feature', undefined, 'ssh-1') + + expect(sshGitProvider.exec).toHaveBeenCalledTimes(2) + expect(pr).toMatchObject({ + number: 81, + title: 'Fresh SSH upstream PR' + }) + } finally { + vi.useRealTimers() + } + }) + it('uses linked PR number as the source of truth when provided', async () => { getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) ghExecFileAsyncMock.mockResolvedValueOnce({ @@ -1742,6 +2222,66 @@ describe('getPRForBranch', () => { expect(pr?.conflictSummary?.files).toEqual(['src/conflict.ts']) }) + it('skips the unsupported merge-tree --merge-base retry after the first capability miss', async () => { + getOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' }) + const branchLookup = { + number: 42, + title: 'Fix PR discovery', + state: 'open', + html_url: 'https://github.com/acme/widgets/pull/42', + updated_at: '2026-03-28T00:00:00Z', + draft: false, + mergeable_state: 'dirty', + base: { ref: 'main', sha: 'base-oid' }, + head: { ref: 'feature/test', sha: 'head-oid' } + } + const exactLookup = { + number: 42, + title: 'Fix PR discovery', + state: 'OPEN', + url: 'https://github.com/acme/widgets/pull/42', + statusCheckRollup: [], + updatedAt: '2026-03-28T00:00:00Z', + isDraft: false, + mergeable: 'CONFLICTING', + baseRefName: 'main', + headRefName: 'feature/test', + baseRefOid: 'base-oid', + headRefOid: 'head-oid' + } + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([branchLookup]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify(exactLookup) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([branchLookup]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify(exactLookup) }) + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: '' }) + .mockResolvedValueOnce({ stdout: 'latest-base-oid\n' }) + .mockResolvedValueOnce({ stdout: 'merge-base-oid\n' }) + .mockResolvedValueOnce({ stdout: '2\n' }) + .mockRejectedValueOnce({ stderr: "error: unknown option `merge-base'" }) + .mockRejectedValueOnce({ stdout: 'result-tree-oid\u0000src/conflict.ts\u0000' }) + .mockResolvedValueOnce({ stdout: '' }) + .mockResolvedValueOnce({ stdout: 'latest-base-oid\n' }) + .mockResolvedValueOnce({ stdout: 'merge-base-oid\n' }) + .mockResolvedValueOnce({ stdout: '2\n' }) + .mockRejectedValueOnce({ stdout: 'result-tree-oid\u0000src/conflict.ts\u0000' }) + + await getPRForBranch('/repo-root', 'feature/test') + await getPRForBranch('/repo-root', 'feature/test') + + const modernMergeTreeCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => + (args as string[]).includes('--merge-base') + ) + const legacyMergeTreeCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => { + const argv = args as string[] + return argv[0] === 'merge-tree' && !argv.includes('--merge-base') + }) + + expect(modernMergeTreeCalls).toHaveLength(1) + expect(legacyMergeTreeCalls).toHaveLength(2) + }) + it('does not retry legacy merge-tree for older Git failures unrelated to --merge-base', async () => { getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) ghExecFileAsyncMock.mockResolvedValueOnce({ diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 158c28ef4d6..0579ca9c50a 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -67,6 +67,7 @@ import { getHostedReviewLocalGitOptions, type HostedReviewExecutionOptions } from '../source-control/hosted-review-git-options' +import { readLocalGitConfigSignature } from './local-git-config-signature' export { _resetOwnerRepoCache } from './gh-utils' export { getIssue, @@ -2273,18 +2274,38 @@ type TrackedUpstreamBranch = { branchName: string } -const TRACKED_UPSTREAM_NULL_CACHE_TTL_MS = 30_000 +const TRACKED_UPSTREAM_SNAPSHOT_CACHE_TTL_MS = 30_000 -type TrackedUpstreamNullCacheEntry = { +type TrackedUpstreamSnapshotCacheEntry = { expiresAt: number + gitConfigSignature?: string + upstreamsByBranchName: Map } -const trackedUpstreamNullCache = new Map() -const trackedUpstreamInFlight = new Map>() +type TrackedUpstreamSnapshotProbeResult = { + cacheable: boolean + gitConfigSignature?: string + probeFailed: boolean + upstreamsByBranchName: Map +} + +const trackedUpstreamSnapshotCache = new Map() +const trackedUpstreamSnapshotInFlight = new Map< + string, + Promise +>() +const trackedUpstreamSnapshotGenerations = new Map() + +function beginTrackedUpstreamSnapshotProbe(cacheKey: string): number { + const nextGeneration = (trackedUpstreamSnapshotGenerations.get(cacheKey) ?? 0) + 1 + trackedUpstreamSnapshotGenerations.set(cacheKey, nextGeneration) + return nextGeneration +} export function __resetTrackedUpstreamBranchCacheForTests(): void { - trackedUpstreamNullCache.clear() - trackedUpstreamInFlight.clear() + trackedUpstreamSnapshotCache.clear() + trackedUpstreamSnapshotInFlight.clear() + trackedUpstreamSnapshotGenerations.clear() } function parseTrackedUpstreamBranch( @@ -2304,66 +2325,160 @@ async function getTrackedUpstreamBranch( connectionId?: string | null, localGitOptions: { wslDistro?: string } = {} ): Promise { - // Why: branches without configured upstreams are stable misses during PR - // polling; cache only nulls so positive PR discovery stays fresh. - const cacheKey = getTrackedUpstreamBranchCacheKey( - repoPath, - branchName, - connectionId, - localGitOptions - ) + const cacheKey = getTrackedUpstreamBranchCacheKey(repoPath, connectionId, localGitOptions) const now = Date.now() - const cachedNull = trackedUpstreamNullCache.get(cacheKey) - if (cachedNull && cachedNull.expiresAt > now) { - return null + const cached = trackedUpstreamSnapshotCache.get(cacheKey) + if (cached && cached.expiresAt > now) { + const configSignatureMatches = await doesTrackedUpstreamCacheConfigSignatureMatch( + cached, + repoPath, + connectionId, + localGitOptions + ) + if ( + configSignatureMatches && + cached.upstreamsByBranchName.has(branchName) && + canUseCachedTrackedUpstreamBranch(cached, branchName) + ) { + return cached.upstreamsByBranchName.get(branchName) ?? null + } + trackedUpstreamSnapshotCache.delete(cacheKey) } - if (cachedNull) { - trackedUpstreamNullCache.delete(cacheKey) + if (cached) { + trackedUpstreamSnapshotCache.delete(cacheKey) } - const inFlight = trackedUpstreamInFlight.get(cacheKey) + const inFlight = trackedUpstreamSnapshotInFlight.get(cacheKey) if (inFlight) { - return inFlight + const result = await inFlight + if (result.upstreamsByBranchName.has(branchName)) { + return result.upstreamsByBranchName.get(branchName) ?? null + } + // Why: a concurrent snapshot may finish before this branch exists in git. + // Re-probe instead of returning a one-shot synthetic null. + const retryInFlight = trackedUpstreamSnapshotInFlight.get(cacheKey) + if (retryInFlight) { + const retryResult = await retryInFlight + return retryResult.upstreamsByBranchName.get(branchName) ?? null + } } - const probe = probeTrackedUpstreamBranch(repoPath, branchName, connectionId, localGitOptions) - trackedUpstreamInFlight.set(cacheKey, probe) + // Why: PR polling can ask about hundreds of local worktree branches at once. + // Read the branch upstream snapshot in one git process per repo/runtime + // instead of spawning one failing `branch@{upstream}` probe per branch. + const probeGeneration = beginTrackedUpstreamSnapshotProbe(cacheKey) + const probe = probeTrackedUpstreamSnapshot(repoPath, connectionId, localGitOptions) + trackedUpstreamSnapshotInFlight.set(cacheKey, probe) try { const result = await probe - if (result) { - trackedUpstreamNullCache.delete(cacheKey) - } else { - trackedUpstreamNullCache.set(cacheKey, { - expiresAt: now + TRACKED_UPSTREAM_NULL_CACHE_TTL_MS + if (result.cacheable && trackedUpstreamSnapshotGenerations.get(cacheKey) === probeGeneration) { + trackedUpstreamSnapshotCache.set(cacheKey, { + ...(result.gitConfigSignature ? { gitConfigSignature: result.gitConfigSignature } : {}), + upstreamsByBranchName: getCacheableTrackedUpstreamSnapshot(result.upstreamsByBranchName), + expiresAt: Date.now() + TRACKED_UPSTREAM_SNAPSHOT_CACHE_TTL_MS }) } - return result + if (trackedUpstreamSnapshotGenerations.get(cacheKey) !== probeGeneration) { + const fresherCached = trackedUpstreamSnapshotCache.get(cacheKey) + if (fresherCached?.upstreamsByBranchName.has(branchName)) { + return fresherCached.upstreamsByBranchName.get(branchName) ?? null + } + } + return result.upstreamsByBranchName.get(branchName) ?? null } finally { - if (trackedUpstreamInFlight.get(cacheKey) === probe) { - trackedUpstreamInFlight.delete(cacheKey) + if (trackedUpstreamSnapshotInFlight.get(cacheKey) === probe) { + trackedUpstreamSnapshotInFlight.delete(cacheKey) } } } +async function probeTrackedUpstreamSnapshot( + repoPath: string, + connectionId?: string | null, + localGitOptions: { wslDistro?: string } = {} +): Promise { + const startingGitConfigSignature = await readLocalGitConfigSignature({ + repoPath, + connectionId: connectionId ?? null, + ...localGitOptions + }) + const { probeFailed, upstreamsByBranchName } = await probeTrackedUpstreamBranches( + repoPath, + connectionId, + localGitOptions + ) + const endingGitConfigSignature = await readLocalGitConfigSignature({ + repoPath, + connectionId: connectionId ?? null, + ...localGitOptions + }) + const isLocalHostRuntime = !connectionId && !localGitOptions.wslDistro + const configSignatureChanged = + isLocalHostRuntime && startingGitConfigSignature !== endingGitConfigSignature + const gitConfigSignature = + startingGitConfigSignature === endingGitConfigSignature ? endingGitConfigSignature : undefined + return { + // Why: transient git failures must not cache an empty snapshot that forces + // every branch lookup to delete and re-probe on the next refresh tick. + cacheable: !configSignatureChanged && !probeFailed, + probeFailed, + ...(gitConfigSignature ? { gitConfigSignature } : {}), + upstreamsByBranchName + } +} + +function getCacheableTrackedUpstreamSnapshot( + upstreamsByBranchName: Map +): Map { + // Why: SSH/WSL cannot cheaply inspect remote .git/config here. The short TTL + // still bounds stale positives, while repeated PR refreshes share one scan. + return upstreamsByBranchName +} + +function canUseCachedTrackedUpstreamBranch( + cached: TrackedUpstreamSnapshotCacheEntry, + branchName: string +): boolean { + return cached.upstreamsByBranchName.has(branchName) +} + +async function doesTrackedUpstreamCacheConfigSignatureMatch( + cached: TrackedUpstreamSnapshotCacheEntry, + repoPath: string, + connectionId?: string | null, + localGitOptions: { wslDistro?: string } = {} +): Promise { + if (!cached.gitConfigSignature) { + return true + } + const currentSignature = await readLocalGitConfigSignature({ + repoPath, + connectionId: connectionId ?? null, + ...localGitOptions + }) + return currentSignature === cached.gitConfigSignature +} + function getTrackedUpstreamBranchCacheKey( repoPath: string, - branchName: string, connectionId?: string | null, localGitOptions: { wslDistro?: string } = {} ): string { const runtimeKey = connectionId ? `ssh:${connectionId}` : `local:${localGitOptions.wslDistro ?? 'host'}` - return [runtimeKey, repoPath, branchName].join('\0') + return [runtimeKey, repoPath].join('\0') } -async function probeTrackedUpstreamBranch( +async function probeTrackedUpstreamBranches( repoPath: string, - branchName: string, connectionId?: string | null, localGitOptions: { wslDistro?: string } = {} -): Promise { - const args = ['rev-parse', '--abbrev-ref', '--symbolic-full-name', `${branchName}@{upstream}`] +): Promise<{ + probeFailed: boolean + upstreamsByBranchName: Map +}> { + const args = ['for-each-ref', '--format=%(refname)%00%(upstream)', 'refs/heads'] try { const provider = connectionId ? getSshGitProvider(connectionId) : null const result = provider @@ -2372,10 +2487,47 @@ async function probeTrackedUpstreamBranch( cwd: repoPath, ...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}) }) - return parseTrackedUpstreamBranch(result.stdout, branchName) + return { + probeFailed: false, + upstreamsByBranchName: parseTrackedUpstreamBranches(result.stdout) + } } catch { + return { probeFailed: true, upstreamsByBranchName: new Map() } + } +} + +function parseTrackedUpstreamBranches(stdout: string): Map { + const upstreamsByBranchName = new Map() + for (const line of stdout.split(/\r?\n/)) { + if (!line) { + continue + } + const [branchName, upstreamRef] = line.split('\0') + const localBranchName = branchName?.replace(/^refs\/heads\//, '') + if (!localBranchName) { + continue + } + upstreamsByBranchName.set( + localBranchName, + parseTrackedUpstreamRef(upstreamRef ?? '', localBranchName) + ) + } + return upstreamsByBranchName +} + +function parseTrackedUpstreamRef( + upstreamRef: string, + branchName: string +): TrackedUpstreamBranch | null { + const remoteRefPrefix = 'refs/remotes/' + const normalizedRef = upstreamRef.trim() + if (normalizedRef.startsWith(remoteRefPrefix)) { + return parseTrackedUpstreamBranch(normalizedRef.slice(remoteRefPrefix.length), branchName) + } + if (normalizedRef.startsWith('refs/heads/')) { return null } + return parseTrackedUpstreamBranch(normalizedRef, branchName) } async function lookupPRByBranchName(args: { diff --git a/src/main/github/conflict-summary.ts b/src/main/github/conflict-summary.ts index 62a162a7833..ce0c4b361b7 100644 --- a/src/main/github/conflict-summary.ts +++ b/src/main/github/conflict-summary.ts @@ -5,6 +5,12 @@ type LocalGitExecOptions = { wslDistro?: string } +const mergeTreeMergeBaseUnsupportedRuntimes = new Set() + +export function __resetPRConflictSummaryGitCapabilityCacheForTests(): void { + mergeTreeMergeBaseUnsupportedRuntimes.clear() +} + export async function getPRConflictSummary( repoPath: string, baseRefName: string, @@ -117,6 +123,7 @@ async function loadConflictingFiles( baseOid: string, localGitOptions: LocalGitExecOptions ): Promise { + const capabilityKey = getMergeTreeCapabilityKey(localGitOptions) const modernArgs = [ 'merge-tree', '--write-tree', @@ -138,6 +145,10 @@ async function loadConflictingFiles( baseOid ] + if (mergeTreeMergeBaseUnsupportedRuntimes.has(capabilityKey)) { + return loadConflictingFilesWithLegacyMergeTree(repoPath, legacyArgs, localGitOptions) + } + try { const result = await gitExecFileAsync(modernArgs, { cwd: repoPath, @@ -157,22 +168,35 @@ async function loadConflictingFiles( throw error } - try { - const result = await gitExecFileAsync(legacyArgs, { - cwd: repoPath, - ...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}) - }) - return parseMergeTreeNameOnlyOutput(result.stdout) - } catch (fallbackError) { - const fallbackStdout = getGitErrorOutput(fallbackError, 'stdout') - if (fallbackStdout) { - return parseMergeTreeNameOnlyOutput(fallbackStdout) - } - throw fallbackError - } + mergeTreeMergeBaseUnsupportedRuntimes.add(capabilityKey) + return loadConflictingFilesWithLegacyMergeTree(repoPath, legacyArgs, localGitOptions) } } +async function loadConflictingFilesWithLegacyMergeTree( + repoPath: string, + legacyArgs: string[], + localGitOptions: LocalGitExecOptions +): Promise { + try { + const result = await gitExecFileAsync(legacyArgs, { + cwd: repoPath, + ...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}) + }) + return parseMergeTreeNameOnlyOutput(result.stdout) + } catch (fallbackError) { + const fallbackStdout = getGitErrorOutput(fallbackError, 'stdout') + if (fallbackStdout) { + return parseMergeTreeNameOnlyOutput(fallbackStdout) + } + throw fallbackError + } +} + +function getMergeTreeCapabilityKey(localGitOptions: LocalGitExecOptions): string { + return localGitOptions.wslDistro ? `wsl:${localGitOptions.wslDistro}` : 'local:host' +} + function parseMergeTreeNameOnlyOutput(stdout: string): string[] { const entries = stdout.split('\0').filter(Boolean) if (entries.length === 0) { diff --git a/src/main/github/gh-utils.test.ts b/src/main/github/gh-utils.test.ts index 9fc4bfb3082..bc04b2bdc5c 100644 --- a/src/main/github/gh-utils.test.ts +++ b/src/main/github/gh-utils.test.ts @@ -1,3 +1,6 @@ +import { mkdtemp, mkdir, rm, writeFile } from 'fs/promises' +import { tmpdir } from 'os' +import { join } from 'path' import { beforeEach, describe, expect, it, vi } from 'vitest' const { gitExecFileAsyncMock, getSshGitProviderMock } = vi.hoisted(() => ({ @@ -27,12 +30,17 @@ import { resolvePRRepositoryCandidates, resolveIssueSource } from './gh-utils' +import { + __resetLocalGitConfigSignatureCacheForTests, + readLocalGitConfigSignature +} from './local-git-config-signature' describe('github owner/repo resolution', () => { beforeEach(() => { gitExecFileAsyncMock.mockReset() getSshGitProviderMock.mockReset() _resetOwnerRepoCache() + __resetLocalGitConfigSignatureCacheForTests() }) it('parses GitHub HTTPS and SSH remotes', () => { @@ -289,6 +297,364 @@ describe('github owner/repo resolution', () => { vi.useRealTimers() } }) + + it('keeps local missing-remote probes cached beyond the short positive TTL', async () => { + const repoPath = await mkdtemp(join(tmpdir(), 'orca-gh-utils-')) + await mkdir(join(repoPath, '.git')) + await writeFile(join(repoPath, '.git', 'config'), '[core]\n\trepositoryformatversion = 0\n') + vi.useFakeTimers() + try { + vi.setSystemTime(1_000) + gitExecFileAsyncMock.mockRejectedValue(new Error("error: No such remote 'origin'")) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toBeNull() + vi.setSystemTime(32_000) + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toBeNull() + + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + await rm(repoPath, { recursive: true, force: true }) + } + }) + + it('treats stderr-only missing-remote errors as stable negatives', async () => { + const repoPath = await mkdtemp(join(tmpdir(), 'orca-gh-utils-')) + await mkdir(join(repoPath, '.git')) + await writeFile(join(repoPath, '.git', 'config'), '[core]\n\trepositoryformatversion = 0\n') + vi.useFakeTimers() + try { + vi.setSystemTime(1_000) + gitExecFileAsyncMock.mockRejectedValue( + Object.assign(new Error('Command failed'), { + stderr: "fatal: No such remote 'origin'" + }) + ) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toBeNull() + vi.setSystemTime(32_000) + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toBeNull() + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + await rm(repoPath, { recursive: true, force: true }) + } + }) + + it('does not apply the long negative TTL when git remote get-url fails transiently', async () => { + const repoPath = await mkdtemp(join(tmpdir(), 'orca-gh-utils-')) + await mkdir(join(repoPath, '.git')) + await writeFile(join(repoPath, '.git', 'config'), '[core]\n\trepositoryformatversion = 0\n') + try { + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error('fatal: cannot lock ref')) + .mockResolvedValueOnce({ stdout: 'git@github.com:acme/widgets.git\n' }) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toBeNull() + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toEqual({ + owner: 'acme', + repo: 'widgets' + }) + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2) + } finally { + await rm(repoPath, { recursive: true, force: true }) + } + }) + + it('invalidates a cached local missing remote when git config changes', async () => { + const repoPath = await mkdtemp(join(tmpdir(), 'orca-gh-utils-')) + await mkdir(join(repoPath, '.git')) + const configPath = join(repoPath, '.git', 'config') + await writeFile(configPath, '[core]\n\trepositoryformatversion = 0\n') + vi.useFakeTimers() + try { + vi.setSystemTime(1_000) + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error("error: No such remote 'origin'")) + .mockResolvedValueOnce({ stdout: 'git@github.com:acme/widgets.git\n' }) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toBeNull() + await writeFile( + configPath, + '[core]\n\trepositoryformatversion = 0\n[remote "origin"]\n\turl = git@github.com:acme/widgets.git\n' + ) + vi.setSystemTime(32_000) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toEqual({ + owner: 'acme', + repo: 'widgets' + }) + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + await rm(repoPath, { recursive: true, force: true }) + } + }) + + it('invalidates a cached local missing remote when an included git config changes', async () => { + const repoPath = await mkdtemp(join(tmpdir(), 'orca-gh-utils-')) + await mkdir(join(repoPath, '.git')) + const includedConfigPath = join(repoPath, 'remote.inc') + await writeFile( + join(repoPath, '.git', 'config'), + `[core]\n\trepositoryformatversion = 0\n[include]\n\tpath = ${includedConfigPath}\n` + ) + await writeFile(includedConfigPath, '') + vi.useFakeTimers() + try { + vi.setSystemTime(1_000) + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error("error: No such remote 'origin'")) + .mockResolvedValueOnce({ stdout: 'git@github.com:acme/widgets.git\n' }) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toBeNull() + await writeFile( + includedConfigPath, + '[remote "origin"]\n\turl = git@github.com:acme/widgets.git\n' + ) + vi.setSystemTime(32_000) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toEqual({ + owner: 'acme', + repo: 'widgets' + }) + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + await rm(repoPath, { recursive: true, force: true }) + } + }) + + it('tracks included git config paths with inline comments', async () => { + const repoPath = await mkdtemp(join(tmpdir(), 'orca-gh-utils-')) + await mkdir(join(repoPath, '.git')) + const includedConfigPath = join(repoPath, 'remote-with-comment.inc') + await writeFile( + join(repoPath, '.git', 'config'), + `[core]\n\trepositoryformatversion = 0\n[include]\n\tpath = ${includedConfigPath} # origin remote lives here\n` + ) + await writeFile(includedConfigPath, '') + vi.useFakeTimers() + try { + vi.setSystemTime(1_000) + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error("error: No such remote 'origin'")) + .mockResolvedValueOnce({ stdout: 'git@github.com:acme/widgets.git\n' }) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toBeNull() + await writeFile( + includedConfigPath, + '[remote "origin"]\n\turl = git@github.com:acme/widgets.git\n' + ) + vi.setSystemTime(32_000) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toEqual({ + owner: 'acme', + repo: 'widgets' + }) + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + await rm(repoPath, { recursive: true, force: true }) + } + }) + + it('tracks included git config paths when section headers have inline comments', async () => { + const repoPath = await mkdtemp(join(tmpdir(), 'orca-gh-utils-')) + await mkdir(join(repoPath, '.git')) + const includedConfigPath = join(repoPath, 'section-comment.inc') + await writeFile( + join(repoPath, '.git', 'config'), + `[core]\n\trepositoryformatversion = 0\n[include] # comment\n\tpath = ${includedConfigPath}\n` + ) + await writeFile(includedConfigPath, '') + vi.useFakeTimers() + try { + vi.setSystemTime(1_000) + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error("error: No such remote 'origin'")) + .mockResolvedValueOnce({ stdout: 'git@github.com:acme/widgets.git\n' }) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toBeNull() + await writeFile( + includedConfigPath, + '[remote "origin"]\n\turl = git@github.com:acme/widgets.git\n' + ) + vi.setSystemTime(32_000) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toEqual({ + owner: 'acme', + repo: 'widgets' + }) + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + await rm(repoPath, { recursive: true, force: true }) + } + }) + + it('tracks quoted included git config paths with inline comments', async () => { + const repoPath = await mkdtemp(join(tmpdir(), 'orca-gh-utils-')) + await mkdir(join(repoPath, '.git')) + const includedConfigPath = join(repoPath, 'quoted-comment.inc') + await writeFile( + join(repoPath, '.git', 'config'), + `[core]\n\trepositoryformatversion = 0\n[include]\n\tpath = "${includedConfigPath}" # comment\n` + ) + await writeFile(includedConfigPath, '') + vi.useFakeTimers() + try { + vi.setSystemTime(1_000) + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error("error: No such remote 'origin'")) + .mockResolvedValueOnce({ stdout: 'git@github.com:acme/widgets.git\n' }) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toBeNull() + await writeFile( + includedConfigPath, + '[remote "origin"]\n\turl = git@github.com:acme/widgets.git\n' + ) + vi.setSystemTime(32_000) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toEqual({ + owner: 'acme', + repo: 'widgets' + }) + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + await rm(repoPath, { recursive: true, force: true }) + } + }) + + it('tracks quoted included git config paths with comment characters in the path', async () => { + const repoPath = await mkdtemp(join(tmpdir(), 'orca-gh-utils-')) + await mkdir(join(repoPath, '.git')) + const includeDir = join(repoPath, 'include # hash') + await mkdir(includeDir) + const includedConfigPath = join(includeDir, 'remote.inc') + await writeFile( + join(repoPath, '.git', 'config'), + `[core]\n\trepositoryformatversion = 0\n[include]\n\tpath = "${includedConfigPath}"\n` + ) + await writeFile(includedConfigPath, '') + vi.useFakeTimers() + try { + vi.setSystemTime(1_000) + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error("error: No such remote 'origin'")) + .mockResolvedValueOnce({ stdout: 'git@github.com:acme/widgets.git\n' }) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toBeNull() + await writeFile( + includedConfigPath, + '[remote "origin"]\n\turl = git@github.com:acme/widgets.git\n' + ) + vi.setSystemTime(32_000) + + await expect(getOwnerRepoForRemote(repoPath, 'origin')).resolves.toEqual({ + owner: 'acme', + repo: 'widgets' + }) + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + await rm(repoPath, { recursive: true, force: true }) + } + }) + + it('includes per-worktree git config in local config signatures', async () => { + const repoPath = await mkdtemp(join(tmpdir(), 'orca-gh-utils-')) + const gitDir = join(repoPath, '.git') + await mkdir(gitDir) + await writeFile(join(gitDir, 'config'), '[core]\n\trepositoryformatversion = 0\n') + try { + const firstSignature = await readLocalGitConfigSignature({ + repoPath, + connectionId: null + }) + + await writeFile( + join(gitDir, 'config.worktree'), + '[remote "origin"]\n\turl = git@github.com:acme/widgets.git\n' + ) + const secondSignature = await readLocalGitConfigSignature({ + repoPath, + connectionId: null + }) + + expect(secondSignature).not.toEqual(firstSignature) + } finally { + await rm(repoPath, { recursive: true, force: true }) + } + }) + + it('includes linked worktree config in local config signatures', async () => { + const repoPath = await mkdtemp(join(tmpdir(), 'orca-gh-utils-')) + const commonGitDir = join(repoPath, 'common-git') + const worktreeGitDir = join(commonGitDir, 'worktrees', 'feature') + const worktreePath = join(repoPath, 'feature-worktree') + await mkdir(worktreeGitDir, { recursive: true }) + await mkdir(worktreePath) + await writeFile(join(worktreePath, '.git'), `gitdir: ${worktreeGitDir}\n`) + await writeFile(join(worktreeGitDir, 'commondir'), '../..\n') + await writeFile(join(commonGitDir, 'config'), '[core]\n\trepositoryformatversion = 0\n') + try { + const firstSignature = await readLocalGitConfigSignature({ + repoPath: worktreePath, + connectionId: null + }) + + await writeFile( + join(worktreeGitDir, 'config.worktree'), + '[branch "feature"]\n\tremote = origin\n\tmerge = refs/heads/contributor/original\n' + ) + const secondSignature = await readLocalGitConfigSignature({ + repoPath: worktreePath, + connectionId: null + }) + + expect(secondSignature).not.toEqual(firstSignature) + } finally { + await rm(repoPath, { recursive: true, force: true }) + } + }) + + it('tracks includeIf paths with comment markers inside quoted section headers', async () => { + const repoPath = await mkdtemp(join(tmpdir(), 'orca-gh-utils-')) + const gitDir = join(repoPath, '.git') + const includedDir = join(repoPath, 'Work #1') + const includedConfigPath = join(includedDir, 'included.gitconfig') + await mkdir(gitDir) + await mkdir(includedDir) + await writeFile( + includedConfigPath, + '[remote "origin"]\n\turl = git@github.com:acme/widgets.git\n' + ) + await writeFile( + join(gitDir, 'config'), + `[includeIf "gitdir:${includedDir}/"]\n\tpath = "${includedConfigPath}"\n` + ) + try { + const firstSignature = await readLocalGitConfigSignature({ + repoPath, + connectionId: null + }) + + await writeFile( + includedConfigPath, + '[remote "origin"]\n\turl = git@github.com:acme/renamed-widgets.git\n' + ) + const secondSignature = await readLocalGitConfigSignature({ + repoPath, + connectionId: null + }) + + expect(secondSignature).not.toEqual(firstSignature) + } finally { + await rm(repoPath, { recursive: true, force: true }) + } + }) }) describe('resolveIssueSource', () => { diff --git a/src/main/github/github-remote-identity-parsing.ts b/src/main/github/github-remote-identity-parsing.ts new file mode 100644 index 00000000000..e7bd21242f1 --- /dev/null +++ b/src/main/github/github-remote-identity-parsing.ts @@ -0,0 +1,49 @@ +import type { GitHubOwnerRepo } from '../../shared/types' + +export type GitHubRemoteIdentity = GitHubOwnerRepo & { host: string } + +function normalizeGitHubRemoteHost(host: string): string { + const normalizedHost = host.toLowerCase() + // Why: GitHub documents ssh.github.com:443 as SSH-over-HTTPS for github.com repos. + return normalizedHost === 'ssh.github.com' ? 'github.com' : normalizedHost +} + +function parseGitHubRemotePath(path: string): Pick | null { + const parts = path.replace(/^\/+/, '').replace(/\/+$/, '').split('/') + if (parts.length !== 2) { + return null + } + const [owner, repoWithSuffix] = parts + const repo = repoWithSuffix.replace(/\.git$/i, '') + if (!owner || !repo) { + return null + } + return { owner, repo } +} + +export function parseGitHubRemoteIdentity(remoteUrl: string): GitHubRemoteIdentity | null { + const trimmed = remoteUrl.trim() + const sshMatch = trimmed.match(/^git@([^:]+):([^/]+)\/([^/]+?)(?:\.git)?$/i) + if (sshMatch) { + return { host: normalizeGitHubRemoteHost(sshMatch[1]), owner: sshMatch[2], repo: sshMatch[3] } + } + + try { + const url = new URL(trimmed) + if (!['git:', 'git+ssh:', 'http:', 'https:', 'ssh:'].includes(url.protocol.toLowerCase())) { + return null + } + const path = parseGitHubRemotePath(url.pathname) + return path ? { host: normalizeGitHubRemoteHost(url.hostname), ...path } : null + } catch { + return null + } +} + +export function parseGitHubOwnerRepo(remoteUrl: string): GitHubOwnerRepo | null { + const identity = parseGitHubRemoteIdentity(remoteUrl) + if (!identity || identity.host.toLowerCase() !== 'github.com') { + return null + } + return { owner: identity.owner, repo: identity.repo } +} diff --git a/src/main/github/github-repository-identity.ts b/src/main/github/github-repository-identity.ts index b94a756a8b9..aa106972cdf 100644 --- a/src/main/github/github-repository-identity.ts +++ b/src/main/github/github-repository-identity.ts @@ -1,10 +1,18 @@ import { gitExecFileAsync } from '../git/runner' import type { GitHubOwnerRepo, IssueSourcePreference } from '../../shared/types' import { getSshGitProvider } from '../providers/ssh-git-dispatch' +import { readLocalGitConfigSignature } from './local-git-config-signature' +import { + parseGitHubOwnerRepo, + parseGitHubRemoteIdentity, + type GitHubRemoteIdentity +} from './github-remote-identity-parsing' +import { isStableMissingGitRemoteError } from './stable-missing-git-remote-error' export type OwnerRepo = GitHubOwnerRepo -export type GitHubRemoteIdentity = GitHubOwnerRepo & { host: string } +export type { GitHubRemoteIdentity } +export { parseGitHubOwnerRepo, parseGitHubRemoteIdentity } export type GitHubRepoContext = { repoPath: string @@ -41,12 +49,14 @@ export function ghRepoExecOptions(context: GitHubRepoContext): { } } -const OWNER_REPO_CACHE_TTL_MS = 30_000 +const OWNER_REPO_POSITIVE_CACHE_TTL_MS = 30_000 +const OWNER_REPO_NEGATIVE_CACHE_TTL_MS = 5 * 60_000 const OWNER_REPO_CACHE_MAX_ENTRIES = 512 type OwnerRepoCacheEntry = { value: OwnerRepo | null expiresAt: number + configSignature?: string } const ownerRepoCache = new Map() @@ -78,52 +88,6 @@ function pruneOwnerRepoCache(now: number): void { } } -export function parseGitHubOwnerRepo(remoteUrl: string): OwnerRepo | null { - const identity = parseGitHubRemoteIdentity(remoteUrl) - if (!identity || identity.host.toLowerCase() !== 'github.com') { - return null - } - return { owner: identity.owner, repo: identity.repo } -} - -function normalizeGitHubRemoteHost(host: string): string { - const normalizedHost = host.toLowerCase() - // Why: GitHub documents ssh.github.com:443 as SSH-over-HTTPS for github.com repos. - return normalizedHost === 'ssh.github.com' ? 'github.com' : normalizedHost -} - -function parseGitHubRemotePath(path: string): Pick | null { - const parts = path.replace(/^\/+/, '').replace(/\/+$/, '').split('/') - if (parts.length !== 2) { - return null - } - const [owner, repoWithSuffix] = parts - const repo = repoWithSuffix.replace(/\.git$/i, '') - if (!owner || !repo) { - return null - } - return { owner, repo } -} - -export function parseGitHubRemoteIdentity(remoteUrl: string): GitHubRemoteIdentity | null { - const trimmed = remoteUrl.trim() - const sshMatch = trimmed.match(/^git@([^:]+):([^/]+)\/([^/]+?)(?:\.git)?$/i) - if (sshMatch) { - return { host: normalizeGitHubRemoteHost(sshMatch[1]), owner: sshMatch[2], repo: sshMatch[3] } - } - - try { - const url = new URL(trimmed) - if (!['git:', 'git+ssh:', 'http:', 'https:', 'ssh:'].includes(url.protocol.toLowerCase())) { - return null - } - const path = parseGitHubRemotePath(url.pathname) - return path ? { host: normalizeGitHubRemoteHost(url.hostname), ...path } : null - } catch { - return null - } -} - export async function getRemoteUrlForRepo( context: GitHubRepoContext, remoteName: string @@ -143,6 +107,13 @@ export async function getRemoteUrlForRepo( return stdout } +function getOwnerRepoCacheTtl(value: OwnerRepo | null, configSignature?: string): number { + if (value) { + return OWNER_REPO_POSITIVE_CACHE_TTL_MS + } + return configSignature ? OWNER_REPO_NEGATIVE_CACHE_TTL_MS : OWNER_REPO_POSITIVE_CACHE_TTL_MS +} + export async function getOwnerRepoForRemote( repoPath: string, remoteName: string, @@ -156,7 +127,26 @@ export async function getOwnerRepoForRemote( pruneOwnerRepoCache(now) const cached = ownerRepoCache.get(cacheKey) if (cached && cached.expiresAt > now) { - return cached.value + if (cached.value === null && cached.configSignature !== undefined) { + const currentSignature = await readLocalGitConfigSignature(context) + if (currentSignature !== cached.configSignature) { + ownerRepoCache.delete(cacheKey) + } else { + return cached.value + } + } else { + return cached.value + } + } + if (cached && cached.expiresAt <= now) { + ownerRepoCache.delete(cacheKey) + } + + const nextConfigSignature = await readLocalGitConfigSignature(context) + const refreshedNow = Date.now() + const refreshedCached = ownerRepoCache.get(cacheKey) + if (refreshedCached && refreshedCached.expiresAt > refreshedNow) { + return refreshedCached.value } const inFlight = ownerRepoInFlight.get(cacheKey) @@ -166,7 +156,7 @@ export async function getOwnerRepoForRemote( // Why: startup can resolve issue sources, PR candidates, and repo metadata // for the same repo concurrently. Coalesce missing-remote probes. - const probe = resolveOwnerRepoForRemote(context, remoteName, cacheKey) + const probe = resolveOwnerRepoForRemote(context, remoteName, cacheKey, nextConfigSignature) ownerRepoInFlight.set(cacheKey, probe) try { return await probe @@ -180,7 +170,8 @@ export async function getOwnerRepoForRemote( async function resolveOwnerRepoForRemote( context: GitHubRepoContext, remoteName: string, - cacheKey: string + cacheKey: string, + configSignature?: string ): Promise { const now = Date.now() try { @@ -189,15 +180,25 @@ async function resolveOwnerRepoForRemote( if (result) { ownerRepoCache.set(cacheKey, { value: result, - expiresAt: now + OWNER_REPO_CACHE_TTL_MS + expiresAt: now + getOwnerRepoCacheTtl(result, configSignature) }) pruneOwnerRepoCache(now) return result } - } catch { - // ignore - non-GitHub remote or no remote + } catch (error) { + // Why: only stable "no such remote" misses are safe to hold for minutes. + // Transient git lock/IO failures must retry on the next lookup. + if (!isStableMissingGitRemoteError(error)) { + return null + } } - ownerRepoCache.set(cacheKey, { value: null, expiresAt: now + OWNER_REPO_CACHE_TTL_MS }) + // Why: a missing/non-GitHub remote is stable until `.git/config` changes. + // Holding that negative longer avoids Git process churn across PR polling. + ownerRepoCache.set(cacheKey, { + value: null, + expiresAt: now + getOwnerRepoCacheTtl(null, configSignature), + ...(configSignature ? { configSignature } : {}) + }) pruneOwnerRepoCache(now) return null } diff --git a/src/main/github/local-git-config-signature.ts b/src/main/github/local-git-config-signature.ts new file mode 100644 index 00000000000..172a44585c6 --- /dev/null +++ b/src/main/github/local-git-config-signature.ts @@ -0,0 +1,239 @@ +import { readFile, stat } from 'fs/promises' +import { homedir } from 'os' +import { dirname, isAbsolute, join, resolve } from 'path' +import type { GitHubRepoContext } from './github-repository-identity' + +type LocalGitConfigPaths = { + commonConfigPath: string + worktreeConfigPath: string +} + +const localGitConfigSignatureInFlight = new Map>() + +export async function readLocalGitConfigSignature( + context: GitHubRepoContext +): Promise { + if (context.connectionId || context.wslDistro) { + // Why: this signature only covers host filesystem config files; remote + // runtimes are already separated by cache key and probed through git. + return undefined + } + const cacheKey = context.repoPath + const inFlight = localGitConfigSignatureInFlight.get(cacheKey) + if (inFlight) { + return inFlight + } + + const read = readUncachedLocalGitConfigSignature(context.repoPath) + localGitConfigSignatureInFlight.set(cacheKey, read) + try { + return await read + } finally { + if (localGitConfigSignatureInFlight.get(cacheKey) === read) { + localGitConfigSignatureInFlight.delete(cacheKey) + } + } +} + +export function __resetLocalGitConfigSignatureCacheForTests(): void { + localGitConfigSignatureInFlight.clear() +} + +async function readUncachedLocalGitConfigSignature(repoPath: string): Promise { + const configPaths = await resolveLocalGitConfigPaths(repoPath) + if (!configPaths) { + return undefined + } + const signatures = await Promise.all([ + readConfigPathSignatures(configPaths.commonConfigPath), + readConfigPathSignatures(configPaths.worktreeConfigPath) + ]) + return signatures.flat().join('\0') +} + +async function readConfigPathSignatures( + configPath: string, + visited = new Set() +): Promise { + if (visited.has(configPath)) { + return [] + } + visited.add(configPath) + + const ownSignature = await readConfigPathSignature(configPath) + let configText: string + try { + configText = await readFile(configPath, 'utf8') + } catch { + return [ownSignature] + } + + const includedPaths = parseIncludedConfigPaths(configText, dirname(configPath)) + const includedSignatures = await Promise.all( + includedPaths.map((includedPath) => readConfigPathSignatures(includedPath, visited)) + ) + return [ownSignature, ...includedSignatures.flat()] +} + +async function readConfigPathSignature(configPath: string): Promise { + try { + const stats = await stat(configPath) + return `${configPath}\0${stats.mtimeMs}\0${stats.size}` + } catch { + return `${configPath}\0missing` + } +} + +function parseIncludedConfigPaths(configText: string, baseDir: string): string[] { + const includedPaths: string[] = [] + let inIncludeSection = false + for (const rawLine of configText.split(/\r?\n/)) { + const line = rawLine.trim() + if (!line || line.startsWith('#') || line.startsWith(';')) { + continue + } + const sectionName = parseConfigSectionName(line) + if (sectionName) { + inIncludeSection = sectionName === 'include' || sectionName.startsWith('includeif ') + continue + } + if (!inIncludeSection) { + continue + } + const includePath = parseIncludedConfigPath(line) + if (includePath) { + includedPaths.push(resolveIncludedConfigPath(includePath, baseDir)) + } + } + return includedPaths +} + +function parseConfigSectionName(line: string): string | null { + if (!line.startsWith('[')) { + return null + } + let quote: string | null = null + for (let index = 1; index < line.length; index += 1) { + const char = line[index] + if (quote) { + if (char === quote) { + quote = null + } + continue + } + if (char === '"' || char === "'") { + quote = char + continue + } + if (char !== ']') { + continue + } + const trailing = line.slice(index + 1).trim() + if (trailing && !trailing.startsWith('#') && !trailing.startsWith(';')) { + return null + } + return line.slice(1, index).trim().toLowerCase() + } + return null +} + +function parseIncludedConfigPath(line: string): string | null { + const match = line.match(/^path\s*=\s*(.+)$/i) + if (!match) { + return null + } + const rawValue = match[1].trim() + if (!rawValue) { + return null + } + const quotedValue = parseQuotedConfigValue(rawValue) + if (quotedValue !== null) { + return quotedValue + } + const value = stripInlineConfigComment(rawValue).trim() + if (!value) { + return null + } + return value +} + +function parseQuotedConfigValue(rawValue: string): string | null { + const quote = rawValue[0] + if (quote !== '"' && quote !== "'") { + return null + } + const endQuoteIndex = rawValue.indexOf(quote, 1) + if (endQuoteIndex === -1) { + return null + } + const trailing = rawValue.slice(endQuoteIndex + 1).trim() + if (trailing && !trailing.startsWith('#') && !trailing.startsWith(';')) { + return null + } + return rawValue.slice(1, endQuoteIndex) +} + +function stripInlineConfigComment(value: string): string { + if ( + (value.startsWith('"') && value.endsWith('"')) || + (value.startsWith("'") && value.endsWith("'")) + ) { + return value.slice(1, -1) + } + return value.replace(/\s[#;].*$/, '').trim() +} + +function resolveIncludedConfigPath(includePath: string, baseDir: string): string { + if (includePath === '~') { + return homedir() + } + if (includePath.startsWith('~/')) { + return join(homedir(), includePath.slice(2)) + } + if (isAbsolute(includePath)) { + return includePath + } + return resolve(baseDir, includePath) +} + +async function resolveLocalGitConfigPaths(repoPath: string): Promise { + const dotGitPath = join(repoPath, '.git') + try { + const dotGitStats = await stat(dotGitPath) + if (dotGitStats.isDirectory()) { + return { + commonConfigPath: join(dotGitPath, 'config'), + worktreeConfigPath: join(dotGitPath, 'config.worktree') + } + } + if (!dotGitStats.isFile()) { + return null + } + } catch { + return null + } + + try { + const gitFile = await readFile(dotGitPath, 'utf8') + const match = gitFile.match(/^gitdir:\s*(.+?)\s*$/im) + if (!match) { + return null + } + const gitDir = resolve(dirname(dotGitPath), match[1]) + let commonGitDir = gitDir + try { + const commonDir = (await readFile(join(gitDir, 'commondir'), 'utf8')).trim() + if (commonDir) { + commonGitDir = resolve(gitDir, commonDir) + } + } catch { + // Fall back to the linked worktree gitdir below. + } + return { + commonConfigPath: join(commonGitDir, 'config'), + worktreeConfigPath: join(gitDir, 'config.worktree') + } + } catch { + return null + } +} diff --git a/src/main/github/stable-missing-git-remote-error.ts b/src/main/github/stable-missing-git-remote-error.ts new file mode 100644 index 00000000000..b88ded0d00c --- /dev/null +++ b/src/main/github/stable-missing-git-remote-error.ts @@ -0,0 +1,16 @@ +export function isStableMissingGitRemoteError(error: unknown): boolean { + const parts: string[] = [] + if (error instanceof Error) { + parts.push(error.message) + } + if (typeof error === 'object' && error !== null && 'stderr' in error) { + const stderr = (error as { stderr?: unknown }).stderr + if (typeof stderr === 'string') { + parts.push(stderr) + } + } + if (parts.length === 0) { + parts.push(String(error)) + } + return /no such remote/i.test(parts.join('\n')) +} diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 0f043c7c6fe..5722eb2f9e9 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -2037,6 +2037,55 @@ describe('registerWorktreeHandlers', () => { }) }) + it('does not reuse host detected worktree scans for a selected WSL runtime', async () => { + listWorktreesMock + .mockResolvedValueOnce([ + { + path: '/workspace/repo', + head: 'host-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ]) + .mockResolvedValueOnce([ + { + path: '/workspace/repo', + head: 'wsl-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ]) + + const hostResult = (await handlers['worktrees:listDetected'](null, { + repoId: 'repo-1' + })) as { worktrees: Worktree[] } + setPlatform('win32') + store.getProjects.mockReturnValue([ + { + id: 'project-1', + displayName: 'repo', + badgeColor: '#000', + sourceRepoIds: ['repo-1'], + localWindowsRuntimePreference: { kind: 'wsl', distro: 'Ubuntu' }, + createdAt: 0, + updatedAt: 0 + } + ]) + const wslResult = (await handlers['worktrees:listDetected'](null, { + repoId: 'repo-1' + })) as { worktrees: Worktree[] } + + expect(hostResult.worktrees[0].head).toBe('host-head') + expect(wslResult.worktrees[0].head).toBe('wsl-head') + expect(listWorktreesMock).toHaveBeenCalledTimes(2) + expect(listWorktreesMock).toHaveBeenNthCalledWith(1, '/workspace/repo') + expect(listWorktreesMock).toHaveBeenNthCalledWith(2, '/workspace/repo', { + wslDistro: 'Ubuntu' + }) + }) + it('reuses a recent authoritative detected worktree scan', async () => { listWorktreesMock.mockResolvedValue([ { diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index f550d11079a..439e0e479b2 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -407,12 +407,19 @@ const detectedWorktreeScanInFlight = new Map> const detectedWorktreeScanGenerations = new Map() function invalidateDetectedWorktreeScanCache(repoId: string): void { - detectedWorktreeScanCache.delete(repoId) - detectedWorktreeScanInFlight.delete(repoId) - detectedWorktreeScanGenerations.set( - repoId, - (detectedWorktreeScanGenerations.get(repoId) ?? 0) + 1 - ) + const keyPrefix = `${repoId}\0` + for (const key of new Set([ + ...detectedWorktreeScanCache.keys(), + ...detectedWorktreeScanInFlight.keys(), + ...detectedWorktreeScanGenerations.keys() + ])) { + if (!key.startsWith(keyPrefix)) { + continue + } + detectedWorktreeScanCache.delete(key) + detectedWorktreeScanInFlight.delete(key) + detectedWorktreeScanGenerations.set(key, (detectedWorktreeScanGenerations.get(key) ?? 0) + 1) + } } registerWorktreeChangeInvalidator(invalidateDetectedWorktreeScanCache) @@ -427,45 +434,54 @@ async function listDetectedGitWorktrees( store: Store, repo: Repo ): Promise { + const localWorktreeGitOptions = getLocalProjectWorktreeGitOptions(store, repo) if (repo.connectionId || isFolderRepo(repo)) { return { - gitWorktrees: await listRepoWorktrees(repo, getLocalProjectWorktreeGitOptions(store, repo)), + gitWorktrees: await listRepoWorktrees(repo, localWorktreeGitOptions), fresh: true } } - const cached = detectedWorktreeScanCache.get(repo.id) + const cacheKey = getDetectedWorktreeScanCacheKey(repo.id, localWorktreeGitOptions) + const cached = detectedWorktreeScanCache.get(cacheKey) if (cached && cached.expiresAt > Date.now()) { return { gitWorktrees: cached.worktrees, fresh: false } } - const inFlight = detectedWorktreeScanInFlight.get(repo.id) + const inFlight = detectedWorktreeScanInFlight.get(cacheKey) if (inFlight) { return { gitWorktrees: await inFlight, fresh: false } } - const scan = listRepoWorktrees(repo, getLocalProjectWorktreeGitOptions(store, repo)) - const generation = detectedWorktreeScanGenerations.get(repo.id) ?? 0 - detectedWorktreeScanInFlight.set(repo.id, scan) + const scan = listRepoWorktrees(repo, localWorktreeGitOptions) + const generation = detectedWorktreeScanGenerations.get(cacheKey) ?? 0 + detectedWorktreeScanInFlight.set(cacheKey, scan) try { const gitWorktrees = await scan // Why: a create/remove notification can invalidate while the git scan is // still running. Do not let that stale scan repopulate the cache afterward. - const isCurrentGeneration = (detectedWorktreeScanGenerations.get(repo.id) ?? 0) === generation + const isCurrentGeneration = (detectedWorktreeScanGenerations.get(cacheKey) ?? 0) === generation if (isCurrentGeneration) { - detectedWorktreeScanCache.set(repo.id, { + detectedWorktreeScanCache.set(cacheKey, { worktrees: gitWorktrees, expiresAt: Date.now() + DETECTED_WORKTREE_SCAN_CACHE_TTL_MS }) } return { gitWorktrees, fresh: isCurrentGeneration } } finally { - if (detectedWorktreeScanInFlight.get(repo.id) === scan) { - detectedWorktreeScanInFlight.delete(repo.id) + if (detectedWorktreeScanInFlight.get(cacheKey) === scan) { + detectedWorktreeScanInFlight.delete(cacheKey) } } } +function getDetectedWorktreeScanCacheKey( + repoId: string, + localWorktreeGitOptions: { wslDistro?: string } = {} +): string { + return `${repoId}\0${localWorktreeGitOptions.wslDistro ?? 'host'}` +} + function warnOnce(keySet: Set, key: string, message: string, error?: unknown): void { if (keySet.has(key)) { return @@ -892,6 +908,7 @@ export function registerWorktreeHandlers( const results = await mapWithConcurrency(repos, WORKTREE_LIST_ALL_CONCURRENCY, async (repo) => { try { let gitWorktrees + let freshScan = true if (isFolderRepo(repo)) { return listVisibleFolderWorkspaces(store, repo) } else if (repo.connectionId) { @@ -917,13 +934,14 @@ export function registerWorktreeHandlers( return listDisconnectedSshWorktrees(store, repo, sshWorktreeMetaIndex) } } else { - gitWorktrees = await listRepoWorktrees( - repo, - getLocalProjectWorktreeGitOptions(store, repo) - ) + const scan = await listDetectedGitWorktrees(store, repo) + gitWorktrees = scan.gitWorktrees + freshScan = scan.fresh + } + if (freshScan) { + rememberLocalWorktreeRoots(store, repo, gitWorktrees) + pruneLineageForMissingRepoWorktrees(store, repo, gitWorktrees) } - rememberLocalWorktreeRoots(store, repo, gitWorktrees) - pruneLineageForMissingRepoWorktrees(store, repo, gitWorktrees) loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`) return buildDetectedGitWorktrees(store, repo, gitWorktrees) .filter((worktree) => worktree.visible) @@ -960,6 +978,7 @@ export function registerWorktreeHandlers( try { let gitWorktrees + let freshScan = true if (isFolderRepo(repo)) { return listVisibleFolderWorkspaces(store, repo) } else if (repo.connectionId) { @@ -985,10 +1004,14 @@ export function registerWorktreeHandlers( return listDisconnectedSshWorktrees(store, repo, sshWorktreeMetaIndex) } } else { - gitWorktrees = await listRepoWorktrees(repo, getLocalProjectWorktreeGitOptions(store, repo)) + const scan = await listDetectedGitWorktrees(store, repo) + gitWorktrees = scan.gitWorktrees + freshScan = scan.fresh + } + if (freshScan) { + rememberLocalWorktreeRoots(store, repo, gitWorktrees) + pruneLineageForMissingRepoWorktrees(store, repo, gitWorktrees) } - rememberLocalWorktreeRoots(store, repo, gitWorktrees) - pruneLineageForMissingRepoWorktrees(store, repo, gitWorktrees) loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`) return buildDetectedGitWorktrees(store, repo, gitWorktrees) .filter((worktree) => worktree.visible)