diff --git a/AGENTS.md b/AGENTS.md index f8f0f80f5..fba60208e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,6 +30,10 @@ Orca targets macOS, Linux, and Windows. Keep all platform-dependent behavior beh All changes must consider the SSH use case. Don't assume local-only execution. +## Git Provider Compatibility + +Source-control and review changes must consider GitLab and other supported git providers, not only GitHub. Keep provider-specific behavior behind explicit checks, and avoid GitHub-only naming for generic review concepts. + ## GitHub CLI Usage Be mindful of the user's `gh` CLI API rate limit — batch requests where possible and avoid unnecessary calls. All code, commands, and scripts must be compatible with macOS, Linux, and Windows. diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index 6b5e018bd..b6620d4f3 100644 --- a/src/main/github/client.test.ts +++ b/src/main/github/client.test.ts @@ -511,6 +511,78 @@ describe('getPRForBranch', () => { }) }) + it('prefers branch lookup over a fallback PR number', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 43, + title: 'Branch PR wins', + state: 'open', + html_url: 'https://github.com/acme/widgets/pull/43', + updated_at: '2026-03-28T00:00:00Z', + draft: false, + mergeable: true, + head: { ref: 'feature/test', sha: 'branch-head-oid' }, + base: { ref: 'main', sha: 'branch-base-oid' } + } + ]) + }) + + const pr = await getPRForBranch('/repo-root', 'feature/test', null, null, 42) + + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + expect(ghExecFileAsyncMock).toHaveBeenCalledWith( + ['api', 'repos/acme/widgets/pulls?head=acme%3Afeature%2Ftest&state=all&per_page=1'], + { cwd: '/repo-root' } + ) + expect(pr).toMatchObject({ number: 43, title: 'Branch PR wins' }) + }) + + it('uses a fallback PR number only after branch lookup misses', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 42, + title: 'Fallback PR lookup', + state: 'OPEN', + url: 'https://github.com/acme/widgets/pull/42', + statusCheckRollup: [], + updatedAt: '2026-03-28T00:00:00Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'contributor/original', + baseRefOid: 'base-oid', + headRefOid: 'fallback-head-oid' + }) + }) + + const pr = await getPRForBranch('/repo-root', 'feature/test', null, null, 42) + + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 1, + ['api', 'repos/acme/widgets/pulls?head=acme%3Afeature%2Ftest&state=all&per_page=1'], + { cwd: '/repo-root' } + ) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 2, + [ + 'pr', + 'view', + '42', + '--repo', + 'acme/widgets', + '--json', + 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' + ], + { cwd: '/repo-root' } + ) + expect(pr).toMatchObject({ number: 42, title: 'Fallback PR lookup' }) + }) + it('uses linked PR number as the source of truth when provided', async () => { getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) ghExecFileAsyncMock.mockResolvedValueOnce({ @@ -722,6 +794,42 @@ describe('getPRForBranch', () => { expect(execFileAsyncMock).not.toHaveBeenCalled() }) + it('uses fallback PR number for empty branch when detached', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 42, + title: 'Detached fallback lookup', + state: 'OPEN', + url: 'https://github.com/acme/widgets/pull/42', + statusCheckRollup: [], + updatedAt: '2026-03-28T00:00:00Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'feature/test', + baseRefOid: 'base-oid', + headRefOid: 'head-oid' + }) + }) + + const pr = await getPRForBranch('/repo-root', '', null, null, 42) + + expect(ghExecFileAsyncMock).toHaveBeenCalledWith( + [ + 'pr', + 'view', + '42', + '--repo', + 'acme/widgets', + '--json', + 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' + ], + { cwd: '/repo-root' } + ) + expect(pr).toMatchObject({ number: 42, title: 'Detached fallback lookup' }) + }) + it('returns null when pr list returns an empty array', async () => { execFileAsyncMock .mockResolvedValueOnce({ stdout: 'git@github.com:acme/widgets.git\n' }) diff --git a/src/main/github/client.ts b/src/main/github/client.ts index d3418cf4c..5c822e9a1 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -1727,6 +1727,46 @@ async function getPRByNumber( } } +async function lookupPRByNumber(args: { + candidates: OwnerRepo[] + number: number + ghOptions: ReturnType +}): Promise<{ data: PullRequestLookupData | null; dataRepo: OwnerRepo | null }> { + for (const candidate of args.candidates) { + try { + const linkedData = await getPRByNumber(candidate, args.number, args.ghOptions) + if (!linkedData) { + continue + } + return { data: linkedData, dataRepo: candidate } + } catch (err) { + if (shouldStopAfterExactLookupError(err)) { + throw err + } + // Candidate probing is best-effort; another repo may own the PR. + } + } + + if (args.candidates.length > 0) { + return { data: null, dataRepo: null } + } + + try { + const { stdout } = await ghExecFileAsync( + ['pr', 'view', String(args.number), '--json', PR_LOOKUP_JSON_FIELDS], + args.ghOptions + ) + return { data: JSON.parse(stdout), dataRepo: null } + } catch (err) { + if (isNoPullRequestError(err)) { + // Why: stale cached fallback numbers should not turn every poll into an + // error when the PR was deleted or belonged to a different repo. + return { data: null, dataRepo: null } + } + throw err + } +} + function isNotFoundGhError(err: unknown): boolean { const stderr = err instanceof Error ? err.message : String(err) return classifyGhError(stderr).type === 'not_found' @@ -1746,14 +1786,23 @@ function shouldStopAfterExactLookupError(err: unknown): boolean { * "create from PR" worktrees whose local branch differs from the PR head ref, * and prevents a coalesced linked-PR refresh from fanning out an unrelated * branch lookup result to sibling aliases. + * `fallbackPRNumber` is weaker: branch lookup still wins, and exact lookup is + * used only after branch lookup misses. */ export async function getPRForBranch( repoPath: string, branch: string, linkedPRNumber?: number | null, - connectionId?: string | null + connectionId?: string | null, + fallbackPRNumber?: number | null ): Promise { - const outcome = await getPRForBranchOutcome(repoPath, branch, linkedPRNumber, connectionId) + const outcome = await getPRForBranchOutcome( + repoPath, + branch, + linkedPRNumber, + connectionId, + fallbackPRNumber + ) return outcome.kind === 'found' ? outcome.pr : null } @@ -1761,11 +1810,14 @@ export async function getPRForBranchOutcome( repoPath: string, branch: string, linkedPRNumber?: number | null, - connectionId?: string | null + connectionId?: string | null, + fallbackPRNumber?: number | null ): Promise { // Strip refs/heads/ prefix if present const branchName = branch.replace(/^refs\/heads\//, '') - if (!branchName && typeof linkedPRNumber !== 'number') { + // Why: detached HEAD cannot use branch lookup, but an exact linked/fallback + // PR number remains safe to query and keeps review state visible. + if (!branchName && typeof linkedPRNumber !== 'number' && typeof fallbackPRNumber !== 'number') { return { kind: 'no-pr', fetchedAt: Date.now() } } const context = githubRepoContext(repoPath, connectionId) @@ -1778,39 +1830,13 @@ export async function getPRForBranchOutcome( let dataRepo: OwnerRepo | null = null if (typeof linkedPRNumber === 'number') { - for (const candidate of candidates) { - try { - const linkedData = await getPRByNumber(candidate, linkedPRNumber, ghOptions) - if (!linkedData) { - continue - } - data = linkedData - dataRepo = candidate - break - } catch (err) { - if (shouldStopAfterExactLookupError(err)) { - throw err - } - // Candidate probing is best-effort; another repo may own the PR. - } - } - - if (!data && candidates.length === 0) { - const args = ['pr', 'view', String(linkedPRNumber), '--json', PR_LOOKUP_JSON_FIELDS] - try { - const { stdout } = await ghExecFileAsync(args, ghOptions) - data = JSON.parse(stdout) - } catch (err) { - if (!isNoPullRequestError(err)) { - return prRefreshUpstreamError(err) - } - // Why: a stale linkedPRNumber (PR deleted, wrong repo, ...) makes - // `gh pr view ` reject. Treat that as the no-PR case so - // callers see the historical `null` semantics instead of a thrown - // error every poll cycle. - data = null - } - } + const exactLookup = await lookupPRByNumber({ + candidates, + number: linkedPRNumber, + ghOptions + }) + data = exactLookup.data + dataRepo = exactLookup.dataRepo } else if (branchName) { // During a rebase the worktree is in detached HEAD and branch is empty. // An empty --head filter causes gh to return an arbitrary PR. @@ -1852,6 +1878,15 @@ export async function getPRForBranchOutcome( } } } + if (!data && typeof linkedPRNumber !== 'number' && typeof fallbackPRNumber === 'number') { + const fallbackLookup = await lookupPRByNumber({ + candidates, + number: fallbackPRNumber, + ghOptions + }) + data = fallbackLookup.data + dataRepo = fallbackLookup.dataRepo + } if (!data) { return { kind: 'no-pr', fetchedAt: Date.now() } } diff --git a/src/main/github/pr-refresh-coordinator.ts b/src/main/github/pr-refresh-coordinator.ts index cb7a02728..5de13ed79 100644 --- a/src/main/github/pr-refresh-coordinator.ts +++ b/src/main/github/pr-refresh-coordinator.ts @@ -432,7 +432,8 @@ async function drainQueue(): Promise { next.candidate.repoPath, next.candidate.branch, next.candidate.linkedPRNumber ?? null, - next.candidate.connectionId ?? null + next.candidate.connectionId ?? null, + next.candidate.linkedPRNumber == null ? (next.candidate.fallbackPRNumber ?? null) : null ) outcomeObserver?.(next.candidate, outcome) broadcast({ aliases, reason: next.reason, outcome, requestStartedAt }, requestSequence) @@ -575,7 +576,8 @@ export async function refreshPRNow(candidate: GitHubPRRefreshCandidate): Promise candidate.repoPath, candidate.branch, candidate.linkedPRNumber ?? null, - candidate.connectionId ?? null + candidate.connectionId ?? null, + candidate.linkedPRNumber == null ? (candidate.fallbackPRNumber ?? null) : null ) outcomeObserver?.(candidate, outcome) broadcast({ aliases, reason: 'manual', outcome, requestStartedAt }, requestSequence) diff --git a/src/main/ipc/github.test.ts b/src/main/ipc/github.test.ts index 6d340e8f7..bcbb5f2e8 100644 --- a/src/main/ipc/github.test.ts +++ b/src/main/ipc/github.test.ts @@ -94,7 +94,13 @@ describe('registerGitHubHandlers', () => { branch: 'feature/test' }) - expect(getPRForBranchMock).toHaveBeenCalledWith('/workspace/repo', 'feature/test', null, null) + expect(getPRForBranchMock).toHaveBeenCalledWith( + '/workspace/repo', + 'feature/test', + null, + null, + null + ) }) it('rejects unknown repository paths', async () => { diff --git a/src/main/ipc/github.ts b/src/main/ipc/github.ts index a2397255d..9f322f1a6 100644 --- a/src/main/ipc/github.ts +++ b/src/main/ipc/github.ts @@ -166,13 +166,22 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi ipcMain.handle( 'gh:prForBranch', - async (_event, args: { repoPath: string; branch: string; linkedPRNumber?: number | null }) => { + async ( + _event, + args: { + repoPath: string + branch: string + linkedPRNumber?: number | null + fallbackPRNumber?: number | null + } + ) => { const repo = assertRegisteredRepo(args, store) const pr = await getPRForBranch( repo.path, args.branch, args.linkedPRNumber ?? null, - repoConnectionId(repo) + repoConnectionId(repo), + args.linkedPRNumber == null ? (args.fallbackPRNumber ?? null) : null ) // Emit pr_created when a PR is first detected for a branch. // Why here: the renderer polls gh:prForBranch to check PR status per worktree. diff --git a/src/main/ipc/hosted-review.ts b/src/main/ipc/hosted-review.ts index d972334bf..3b7cb60fd 100644 --- a/src/main/ipc/hosted-review.ts +++ b/src/main/ipc/hosted-review.ts @@ -56,6 +56,7 @@ export function registerHostedReviewHandlers(store: Store, stats: StatsCollector connectionId: repo.connectionId, branch: args.branch, linkedGitHubPR: args.linkedGitHubPR ?? null, + fallbackGitHubPR: args.linkedGitHubPR == null ? (args.fallbackGitHubPR ?? null) : null, linkedGitLabMR: args.linkedGitLabMR ?? null, linkedBitbucketPR: args.linkedBitbucketPR ?? null, linkedAzureDevOpsPR: args.linkedAzureDevOpsPR ?? null, diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 944924f72..985448b67 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -5167,17 +5167,25 @@ export class OrcaRuntimeService { async getRepoPRForBranch( repoSelector: string, branch: string, - linkedPRNumber?: number | null + linkedPRNumber?: number | null, + fallbackPRNumber?: number | null ): Promise>> { const repo = await this.resolveRepoSelector(repoSelector) this.assertHostIntegrationRepoIsLocal(repo, 'repo_pr') - return getPRForBranch(repo.path, branch, linkedPRNumber ?? null) + return getPRForBranch( + repo.path, + branch, + linkedPRNumber ?? null, + null, + linkedPRNumber == null ? (fallbackPRNumber ?? null) : null + ) } async getHostedReviewForBranch(args: { repoSelector: string branch: string linkedGitHubPR?: number | null + fallbackGitHubPR?: number | null linkedGitLabMR?: number | null linkedBitbucketPR?: number | null linkedAzureDevOpsPR?: number | null @@ -5189,6 +5197,7 @@ export class OrcaRuntimeService { repoPath: repo.path, branch: args.branch, linkedGitHubPR: args.linkedGitHubPR ?? null, + fallbackGitHubPR: args.linkedGitHubPR == null ? (args.fallbackGitHubPR ?? null) : null, linkedGitLabMR: args.linkedGitLabMR ?? null, linkedBitbucketPR: args.linkedBitbucketPR ?? null, linkedAzureDevOpsPR: args.linkedAzureDevOpsPR ?? null, @@ -5222,6 +5231,7 @@ export class OrcaRuntimeService { ahead: args.ahead, behind: args.behind, linkedGitHubPR: args.linkedGitHubPR ?? null, + fallbackGitHubPR: args.linkedGitHubPR == null ? (args.fallbackGitHubPR ?? null) : null, linkedGitLabMR: args.linkedGitLabMR ?? null, linkedBitbucketPR: args.linkedBitbucketPR ?? null, linkedAzureDevOpsPR: args.linkedAzureDevOpsPR ?? null, diff --git a/src/main/runtime/rpc/methods/github.ts b/src/main/runtime/rpc/methods/github.ts index b33d4e66e..78ef65b03 100644 --- a/src/main/runtime/rpc/methods/github.ts +++ b/src/main/runtime/rpc/methods/github.ts @@ -46,7 +46,8 @@ const SlugAssignableUsers = SlugRepo.extend({ const PrForBranch = RepoSelector.extend({ branch: requiredString('Missing branch'), - linkedPRNumber: z.number().int().positive().nullable().optional() + linkedPRNumber: z.number().int().positive().nullable().optional(), + fallbackPRNumber: z.number().int().positive().nullable().optional() }) const Issue = RepoSelector.extend({ @@ -313,7 +314,12 @@ export const GITHUB_METHODS: RpcMethod[] = [ name: 'github.prForBranch', params: PrForBranch, handler: async (params, { runtime }) => - runtime.getRepoPRForBranch(params.repo, params.branch, params.linkedPRNumber) + runtime.getRepoPRForBranch( + params.repo, + params.branch, + params.linkedPRNumber, + params.fallbackPRNumber + ) }), defineMethod({ name: 'github.issue', diff --git a/src/main/runtime/rpc/methods/hosted-review.ts b/src/main/runtime/rpc/methods/hosted-review.ts index cd2ccf455..f03f5193d 100644 --- a/src/main/runtime/rpc/methods/hosted-review.ts +++ b/src/main/runtime/rpc/methods/hosted-review.ts @@ -6,6 +6,7 @@ const HostedReviewForBranch = z.object({ repo: requiredString('Missing repo selector'), branch: requiredString('Missing branch'), linkedGitHubPR: z.number().int().positive().nullable().optional(), + fallbackGitHubPR: z.number().int().positive().nullable().optional(), linkedGitLabMR: z.number().int().positive().nullable().optional(), linkedBitbucketPR: z.number().int().positive().nullable().optional(), linkedAzureDevOpsPR: z.number().int().positive().nullable().optional(), @@ -22,6 +23,7 @@ const HostedReviewCreationEligibility = z.object({ ahead: z.number().int().nonnegative().optional(), behind: z.number().int().nonnegative().optional(), linkedGitHubPR: z.number().int().positive().nullable().optional(), + fallbackGitHubPR: z.number().int().positive().nullable().optional(), linkedGitLabMR: z.number().int().positive().nullable().optional(), linkedBitbucketPR: z.number().int().positive().nullable().optional(), linkedAzureDevOpsPR: z.number().int().positive().nullable().optional(), @@ -43,22 +45,28 @@ export const HOSTED_REVIEW_METHODS: RpcMethod[] = [ defineMethod({ name: 'hostedReview.forBranch', params: HostedReviewForBranch, - handler: async (params, { runtime }) => - runtime.getHostedReviewForBranch({ + handler: async (params, { runtime }) => { + const fallbackGitHubPR = + params.linkedGitHubPR == null ? (params.fallbackGitHubPR ?? null) : null + return runtime.getHostedReviewForBranch({ repoSelector: params.repo, branch: params.branch, linkedGitHubPR: params.linkedGitHubPR ?? null, + ...(fallbackGitHubPR !== null ? { fallbackGitHubPR } : {}), linkedGitLabMR: params.linkedGitLabMR ?? null, linkedBitbucketPR: params.linkedBitbucketPR ?? null, linkedAzureDevOpsPR: params.linkedAzureDevOpsPR ?? null, linkedGiteaPR: params.linkedGiteaPR ?? null }) + } }), defineMethod({ name: 'hostedReview.getCreationEligibility', params: HostedReviewCreationEligibility, - handler: async (params, { runtime }) => - runtime.getHostedReviewCreationEligibility({ + handler: async (params, { runtime }) => { + const fallbackGitHubPR = + params.linkedGitHubPR == null ? (params.fallbackGitHubPR ?? null) : null + return runtime.getHostedReviewCreationEligibility({ repoSelector: params.repo, worktreeSelector: params.worktree, branch: params.branch, @@ -68,11 +76,13 @@ export const HOSTED_REVIEW_METHODS: RpcMethod[] = [ ahead: params.ahead, behind: params.behind, linkedGitHubPR: params.linkedGitHubPR ?? null, + ...(fallbackGitHubPR !== null ? { fallbackGitHubPR } : {}), linkedGitLabMR: params.linkedGitLabMR ?? null, linkedBitbucketPR: params.linkedBitbucketPR ?? null, linkedAzureDevOpsPR: params.linkedAzureDevOpsPR ?? null, linkedGiteaPR: params.linkedGiteaPR ?? null }) + } }), defineMethod({ name: 'hostedReview.create', diff --git a/src/main/source-control/hosted-review-creation.ts b/src/main/source-control/hosted-review-creation.ts index 6b69fd0e8..e5d12015b 100644 --- a/src/main/source-control/hosted-review-creation.ts +++ b/src/main/source-control/hosted-review-creation.ts @@ -242,6 +242,7 @@ export async function getHostedReviewCreationEligibility( repoPath: args.repoPath, branch, linkedGitHubPR: args.linkedGitHubPR ?? null, + fallbackGitHubPR: args.linkedGitHubPR == null ? (args.fallbackGitHubPR ?? null) : null, linkedGitLabMR: args.linkedGitLabMR ?? null, linkedBitbucketPR: args.linkedBitbucketPR ?? null, linkedAzureDevOpsPR: args.linkedAzureDevOpsPR ?? null, diff --git a/src/main/source-control/hosted-review.test.ts b/src/main/source-control/hosted-review.test.ts index 6514ce2ac..3b62c0f41 100644 --- a/src/main/source-control/hosted-review.test.ts +++ b/src/main/source-control/hosted-review.test.ts @@ -123,6 +123,33 @@ describe('getHostedReviewForBranch', () => { expect(getPRForBranchMock).toHaveBeenCalledWith('/repo', 'feature', 3, undefined) }) + it('uses fallback GitHub PR when branch is empty', async () => { + getProjectSlugMock.mockResolvedValue(null) + getRepoSlugMock.mockResolvedValue({ owner: 'o', repo: 'r' }) + getPRForBranchMock.mockResolvedValue({ + number: 42, + title: 'Detached GitHub branch', + state: 'open', + url: 'https://github.com/o/r/pull/42', + checksStatus: 'success', + updatedAt: '2026-05-10T00:00:00.000Z', + mergeable: 'MERGEABLE' + }) + + await expect( + getHostedReviewForBranch({ + repoPath: '/repo', + branch: '', + fallbackGitHubPR: 42 + }) + ).resolves.toMatchObject({ + provider: 'github', + number: 42, + status: 'success' + }) + expect(getPRForBranchMock).toHaveBeenCalledWith('/repo', '', null, undefined, 42) + }) + it('falls through to Bitbucket when origin is not GitLab or GitHub', async () => { getProjectSlugMock.mockResolvedValue(null) getRepoSlugMock.mockResolvedValue(null) diff --git a/src/main/source-control/hosted-review.ts b/src/main/source-control/hosted-review.ts index 55ad105bf..d2da4f887 100644 --- a/src/main/source-control/hosted-review.ts +++ b/src/main/source-control/hosted-review.ts @@ -95,15 +95,19 @@ export async function getHostedReviewForBranch(input: { connectionId?: string | null branch: string linkedGitHubPR?: number | null + fallbackGitHubPR?: number | null linkedGitLabMR?: number | null linkedBitbucketPR?: number | null linkedAzureDevOpsPR?: number | null linkedGiteaPR?: number | null }): Promise { const branchName = input.branch.replace(/^refs\/heads\//, '') + // Why: detached HEAD cannot use branch lookup, but provider-specific exact + // ids can still resolve the review without probing an empty branch name. if ( !branchName && input.linkedGitHubPR == null && + input.fallbackGitHubPR == null && input.linkedGitLabMR == null && input.linkedBitbucketPR == null && input.linkedAzureDevOpsPR == null && @@ -125,12 +129,22 @@ export async function getHostedReviewForBranch(input: { const githubRepo = await getRepoSlug(input.repoPath, input.connectionId) if (githubRepo) { - const pr = await getPRForBranch( - input.repoPath, - branchName, - input.linkedGitHubPR ?? null, - input.connectionId - ) + const fallbackGitHubPR = input.linkedGitHubPR == null ? (input.fallbackGitHubPR ?? null) : null + const pr = + fallbackGitHubPR !== null + ? await getPRForBranch( + input.repoPath, + branchName, + input.linkedGitHubPR ?? null, + input.connectionId, + fallbackGitHubPR + ) + : await getPRForBranch( + input.repoPath, + branchName, + input.linkedGitHubPR ?? null, + input.connectionId + ) return pr ? mapGitHubReview(pr) : null } diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index ecfeb2e52..ae78cc3f6 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -816,6 +816,7 @@ export type PreloadApi = { repoId?: string branch: string linkedPRNumber?: number | null + fallbackPRNumber?: number | null }) => Promise refreshPRNow: (args: { candidate: GitHubPRRefreshCandidate }) => Promise enqueuePRRefresh: (args: { diff --git a/src/preload/index.ts b/src/preload/index.ts index 8ccca0c8e..15b66240f 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -782,6 +782,7 @@ const api = { repoId?: string branch: string linkedPRNumber?: number | null + fallbackPRNumber?: number | null }): Promise => ipcRenderer.invoke('gh:prForBranch', args), refreshPRNow: (args: { candidate: GitHubPRRefreshCandidate }): Promise => diff --git a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx index 958cb9ca3..ec4895d2c 100644 --- a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx @@ -58,6 +58,7 @@ export default function ChecksPanel(): React.JSX.Element { const fetchUpstreamStatus = useAppStore((s) => s.fetchUpstreamStatus) const setRightSidebarOpen = useAppStore((s) => s.setRightSidebarOpen) const setRightSidebarTab = useAppStore((s) => s.setRightSidebarTab) + const updateWorktreeMeta = useAppStore((s) => s.updateWorktreeMeta) // Why: the sidebar stays mounted when closed (for performance). Gate // polling on visibility so we don't fetch checks/comments in the background @@ -176,9 +177,10 @@ export default function ChecksPanel(): React.JSX.Element { ) // Fetch PR data when the active worktree/branch changes. - // Why: pass linkedPR so worktrees created from a PR (whose new local branch - // differs from the PR's head ref) resolve via the number-based fallback. + // Why: branch lookup is lossy for fork/deleted-head PRs; reuse a known PR + // number from metadata or the visible cache whenever we have one. const linkedPR = activeWorktree?.linkedPR ?? null + const fallbackGitHubPRNumber = linkedPR == null ? (pr?.number ?? null) : null const linkedGitLabMR = activeWorktree?.linkedGitLabMR ?? null const activeWorktreePath = activeWorktree?.path ?? null const stateRequestKey = @@ -215,7 +217,12 @@ export default function ChecksPanel(): React.JSX.Element { hasUpstream: remoteStatus?.hasUpstream, ahead: remoteStatus?.ahead, behind: remoteStatus?.behind, - linkedGitHubPR: linkedPR + linkedGitHubPR: linkedPR, + fallbackGitHubPR: fallbackGitHubPRNumber, + linkedGitLabMR, + linkedBitbucketPR: null, + linkedAzureDevOpsPR: null, + linkedGiteaPR: null }) .then((result) => { if (!stale) { @@ -238,6 +245,8 @@ export default function ChecksPanel(): React.JSX.Element { isFolder, isPanelVisible, linkedPR, + fallbackGitHubPRNumber, + linkedGitLabMR, remoteStatus?.ahead, remoteStatus?.behind, remoteStatus?.hasUpstream, @@ -272,7 +281,8 @@ export default function ChecksPanel(): React.JSX.Element { void fetchPRForBranch(repo.path, branch, { force: true, repoId: repo.id, - linkedPRNumber: linkedPR + linkedPRNumber: linkedPR, + fallbackPRNumber: fallbackGitHubPRNumber ?? pr.number }).finally(() => { // Why: fetchPRForBranch updates the PR cache before resolving, which // can rerun this effect. Only the current refresh key may clear the @@ -281,7 +291,17 @@ export default function ChecksPanel(): React.JSX.Element { setConflictDetailsRefreshing(false) } }) - }, [repo, isFolder, branch, pr, prCacheKey, activeWorktreeId, linkedPR, fetchPRForBranch]) + }, [ + repo, + isFolder, + branch, + pr, + prCacheKey, + activeWorktreeId, + linkedPR, + fallbackGitHubPRNumber, + fetchPRForBranch + ]) // Fetch checks via cached store method const fetchChecks = useCallback( @@ -505,7 +525,8 @@ export default function ChecksPanel(): React.JSX.Element { const refreshedPR = await fetchPRForBranch(repo.path, branch, { force: true, repoId: repo.id, - linkedPRNumber: linkedPR + linkedPRNumber: linkedPR, + fallbackPRNumber: fallbackGitHubPRNumber }) if (!isCurrentRequest()) { return @@ -514,7 +535,8 @@ export default function ChecksPanel(): React.JSX.Element { repoPath: repo.path, repoId: repo.id, branch, - linkedGitHubPR: refreshedPR?.number ?? linkedPR, + linkedGitHubPR: linkedPR, + fallbackGitHubPR: refreshedPR?.number ?? fallbackGitHubPRNumber, linkedGitLabMR }) if (!isCurrentRequest()) { @@ -618,6 +640,7 @@ export default function ChecksPanel(): React.JSX.Element { pr?.prRepo, prCacheKey, linkedPR, + fallbackGitHubPRNumber, linkedGitLabMR, fetchPRForBranch, fetchPRChecks, @@ -727,14 +750,15 @@ export default function ChecksPanel(): React.JSX.Element { await fetchPRForBranch(repo.path, branch, { force: true, repoId: repo.id, - linkedPRNumber: linkedPR + linkedPRNumber: linkedPR, + fallbackPRNumber: fallbackGitHubPRNumber ?? pr.number }) } } finally { setTitleSaving(false) setEditingTitle(false) } - }, [repo, pr, titleDraft, branch, linkedPR, fetchPRForBranch]) + }, [repo, pr, titleDraft, branch, linkedPR, fallbackGitHubPRNumber, fetchPRForBranch]) const handleTitleKeyDown = useCallback( (e: React.KeyboardEvent) => { @@ -875,17 +899,27 @@ export default function ChecksPanel(): React.JSX.Element { const refreshedPR = await fetchPRForBranch(repo.path, branch, { force: true, repoId: repo.id, - linkedPRNumber: linkedPR + linkedPRNumber: linkedPR, + fallbackPRNumber: fallbackGitHubPRNumber }) await refreshHostedReviewCard(fetchHostedReviewForBranch, { repoPath: repo.path, repoId: repo.id, branch, - linkedGitHubPR: refreshedPR?.number ?? linkedPR, + linkedGitHubPR: linkedPR, + fallbackGitHubPR: refreshedPR?.number ?? fallbackGitHubPRNumber, linkedGitLabMR }) } - }, [repo, branch, linkedPR, linkedGitLabMR, fetchPRForBranch, fetchHostedReviewForBranch]) + }, [ + repo, + branch, + linkedPR, + fallbackGitHubPRNumber, + linkedGitLabMR, + fetchPRForBranch, + fetchHostedReviewForBranch + ]) // Open PR in browser const handleOpenPR = useCallback(() => { @@ -940,6 +974,9 @@ export default function ChecksPanel(): React.JSX.Element { setRightSidebarOpen(true) setRightSidebarTab('checks') try { + if (activeWorktreeId) { + await updateWorktreeMeta(activeWorktreeId, { linkedPR: result.number }) + } const refreshedPR = await fetchPRForBranch(repo.path, branch, { force: true, repoId: repo.id, @@ -1009,7 +1046,9 @@ export default function ChecksPanel(): React.JSX.Element { pr?.prRepo, repo, setRightSidebarOpen, - setRightSidebarTab + setRightSidebarTab, + activeWorktreeId, + updateWorktreeMeta ] ) diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index e5a7caa47..241405fab 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -779,6 +779,7 @@ function SourceControlInner(): React.JSX.Element { (s) => s.getHostedReviewCreationEligibility ) const createHostedReview = useAppStore((s) => s.createHostedReview) + const updateWorktreeMeta = useAppStore((s) => s.updateWorktreeMeta) const fetchPRForBranch = useAppStore((s) => s.fetchPRForBranch) const prCache = useAppStore((s) => s.prCache) const enqueueGitHubPRRefresh = useAppStore((s) => s.enqueueGitHubPRRefresh) @@ -1200,6 +1201,7 @@ function SourceControlInner(): React.JSX.Element { : null const linkedGitHubPR = activeWorktree?.linkedPR ?? null + const fallbackGitHubPRNumber = linkedGitHubPR == null ? (activePrFromQueue?.number ?? null) : null const linkedGitLabMR = activeWorktree?.linkedGitLabMR ?? null // Why: when activeRepo.connectionId is truthy, neither the SourceControl // effect below nor WorktreeCard.tsx fetches hostedReview for this branch, @@ -1209,7 +1211,7 @@ function SourceControlInner(): React.JSX.Element { // gate doesn't latch. const isHostedReviewStateLoading = !activeRepo?.connectionId && - (linkedGitHubPR !== null || linkedGitLabMR !== null) && + ((linkedGitHubPR ?? fallbackGitHubPRNumber) !== null || linkedGitLabMR !== null) && hostedReviewEntry === undefined useEffect(() => { if ( @@ -1223,13 +1225,13 @@ function SourceControlInner(): React.JSX.Element { return } // Why: the Source Control panel renders branch review status directly. - // When a terminal checkout moves this worktree onto a new branch, we need - // to fetch that branch's PR/MR immediately instead of waiting for the user - // to reselect the worktree. The linked ids handle create-from-review - // worktrees whose local branch differs from the remote head branch. + // When a terminal checkout moves this worktree onto a new branch, fetch + // immediately; carry a known PR number because branch lookup is lossy for + // fork/deleted-head PRs. void fetchHostedReviewForBranch(activeRepo.path, branchName, { repoId: activeRepo.id, linkedGitHubPR, + fallbackGitHubPR: fallbackGitHubPRNumber, linkedGitLabMR, staleWhileRevalidate: true }) @@ -1245,6 +1247,7 @@ function SourceControlInner(): React.JSX.Element { isBranchVisible, isFolder, linkedGitHubPR, + fallbackGitHubPRNumber, linkedGitLabMR ]) @@ -1802,6 +1805,9 @@ function SourceControlInner(): React.JSX.Element { setRightSidebarOpen(true) setRightSidebarTab('checks') try { + if (activeWorktreeId) { + await updateWorktreeMeta(activeWorktreeId, { linkedPR: result.number }) + } await Promise.all([ fetchHostedReviewForBranch(activeRepo.path, branchName, { force: true, @@ -1831,7 +1837,9 @@ function SourceControlInner(): React.JSX.Element { fetchPRForBranch, linkedGitLabMR, setRightSidebarOpen, - setRightSidebarTab + setRightSidebarTab, + activeWorktreeId, + updateWorktreeMeta ] ) @@ -2164,6 +2172,7 @@ function SourceControlInner(): React.JSX.Element { ahead: remoteStatus?.ahead, behind: remoteStatus?.behind, linkedGitHubPR, + fallbackGitHubPR: fallbackGitHubPRNumber, linkedGitLabMR }) .then((result) => { @@ -2195,6 +2204,7 @@ function SourceControlInner(): React.JSX.Element { isCreatingPr, isFolder, linkedGitHubPR, + fallbackGitHubPRNumber, linkedGitLabMR, prGenerating, remoteStatus?.ahead, diff --git a/src/renderer/src/components/sidebar/WorktreeCard.tsx b/src/renderer/src/components/sidebar/WorktreeCard.tsx index 6199d7a38..6336c7135 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.tsx @@ -220,6 +220,8 @@ const WorktreeCard = React.memo(function WorktreeCard({ const hostedReview: HostedReviewInfo | null | undefined = hostedReviewEntry !== undefined ? hostedReviewEntry.data : undefined + const fallbackGitHubPRNumber = + worktree.linkedPR == null && hostedReview?.provider === 'github' ? hostedReview.number : null const prDisplay = getWorktreeCardPrDisplay(hostedReview, worktree.linkedPR) const issue: IssueInfo | null | undefined = worktree.linkedIssue ? issueEntry !== undefined @@ -281,12 +283,12 @@ const WorktreeCard = React.memo(function WorktreeCard({ return } if (repo && !isFolder && !worktree.isBare && hostedReviewCacheKey && showPR) { - // Why: pass linkedPR so worktrees created from a PR (whose new local - // branch differs from the remote head ref) still resolve their PR/MR via - // a number-based fallback in the main process. + // Why: branch lookup is lossy for fork/deleted-head PRs; reuse a known PR + // number from metadata or the visible cache whenever we have one. fetchHostedReviewForBranch(repo.path, branch, { repoId: repo.id, linkedGitHubPR: worktree.linkedPR ?? null, + fallbackGitHubPR: fallbackGitHubPRNumber, linkedGitLabMR: worktree.linkedGitLabMR ?? null, staleWhileRevalidate: true }) @@ -296,6 +298,7 @@ const WorktreeCard = React.memo(function WorktreeCard({ isFolder, worktree.isBare, worktree.linkedPR, + fallbackGitHubPRNumber, worktree.linkedGitLabMR, fetchHostedReviewForBranch, branch, diff --git a/src/renderer/src/store/slices/github.test.ts b/src/renderer/src/store/slices/github.test.ts index 7a241f530..1c089dd0a 100644 --- a/src/renderer/src/store/slices/github.test.ts +++ b/src/renderer/src/store/slices/github.test.ts @@ -1880,6 +1880,53 @@ describe('createGitHubSlice.refreshGitHubForWorktreeIfStale', () => { }) expect(store.getState().prCache[`repo-1::${branch}`]).toBeUndefined() }) + + it('uses the cached PR number as a fallback refresh hint when worktree metadata is not linked yet', () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/cached-pr' + const worktreeId = 'wt-cached-pr' + + store.setState({ + repos: [{ id: repoId, path: repoPath, name: 'repo', kind: 'git' }], + groupBy: 'pr-status', + worktreesByRepo: { + [repoId]: [ + { + id: worktreeId, + repoId, + path: '/repo/worktrees/cached-pr', + branch, + displayName: 'cached-pr', + isMainWorktree: false, + isBare: false, + isArchived: false, + linkedPR: null + } + ] + }, + prCache: { + [`${repoId}::${branch}`]: { + data: makePR({ number: 42 }), + fetchedAt: 1 + } + } + } as unknown as Partial) + + store.getState().refreshGitHubForWorktreeIfStale(worktreeId) + + expect(mockApi.gh.enqueuePRRefresh).toHaveBeenCalledWith({ + candidate: expect.objectContaining({ + repoPath, + branch, + linkedPRNumber: null, + fallbackPRNumber: 42 + }), + reason: 'active', + priority: 80 + }) + }) }) describe('createGitHubSlice.refreshAllGitHub', () => { diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index fa9702a32..516e1697c 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -339,7 +339,7 @@ const ERROR_TOAST_DURATION = 60_000 const inflightPRRequests = new Map< string, - { promise: Promise; force: boolean; generation: number } + { promise: Promise; force: boolean; generation: number; lookupHintKey: string } >() const inflightIssueRequests = new Map>() const inflightChecksRequests = new Map>() @@ -514,6 +514,8 @@ function buildPRRefreshCandidate( state.settings, repo.connectionId ) + const fallbackPRNumber = + worktree.linkedPR == null ? (state.prCache[cacheKey]?.data?.number ?? null) : null const sshStatus = repo.connectionId ? state.sshConnectionStates.get(repo.connectionId)?.status : null @@ -524,7 +526,10 @@ function buildPRRefreshCandidate( branch, cacheKey, worktreeId: worktree.id, + // Why: persisted linked PR metadata is exact, while PR cache numbers are + // only fallback hints after branch lookup misses. linkedPRNumber: worktree.linkedPR ?? null, + fallbackPRNumber, isBare: worktree.isBare, isArchived: worktree.isArchived, connectionId: repo.connectionId ?? null, @@ -558,6 +563,13 @@ function isGitHubLinkedReviewHintKey(hintKey: string | undefined): boolean { return hintKey?.split('|').some((key) => key.startsWith('github:')) ?? false } +function prLookupHintKey(linkedPRNumber: number | null, fallbackPRNumber: number | null): string { + if (linkedPRNumber !== null) { + return `linked:${linkedPRNumber}` + } + return fallbackPRNumber !== null ? `fallback:${fallbackPRNumber}` : '' +} + function linkedReviewHintKeyForNoGitHubPR( entry: AppState['hostedReviewCache'][string] | undefined ): string | undefined { @@ -814,7 +826,10 @@ export type GitHubSlice = { fetchPRForBranch: ( repoPath: string, branch: string, - options?: RepoScopedFetchOptions & { linkedPRNumber?: number | null } + options?: RepoScopedFetchOptions & { + linkedPRNumber?: number | null + fallbackPRNumber?: number | null + } ) => Promise fetchIssue: ( repoPath: string, @@ -1693,13 +1708,22 @@ export const createGitHubSlice: StateCreator = (s // the worktree-card lookup (which has a linked PR fallback) would otherwise // return null forever. Refetch when the cached miss could now resolve via // the linkedPR path. - const linkedRefetch = cached?.data === null && (options?.linkedPRNumber ?? null) !== null + const linkedPRNumber = options?.linkedPRNumber ?? null + const fallbackPRNumber = linkedPRNumber == null ? (options?.fallbackPRNumber ?? null) : null + const lookupHintKey = prLookupHintKey(linkedPRNumber, fallbackPRNumber) + const linkedRefetch = + cached?.data === null && (linkedPRNumber !== null || fallbackPRNumber !== null) if (!options?.force && !linkedRefetch && isFresh(cached)) { return cached.data } const inflightRequest = inflightPRRequests.get(cacheKey) - if (inflightRequest && (!options?.force || inflightRequest.force) && !linkedRefetch) { + if ( + inflightRequest && + (!options?.force || inflightRequest.force) && + inflightRequest.lookupHintKey === lookupHintKey && + !linkedRefetch + ) { return inflightRequest.promise } @@ -1708,7 +1732,6 @@ export const createGitHubSlice: StateCreator = (s const requestStartedHostedReviewEntry = get().hostedReviewCache[hostedReviewCacheKey] prRequestGenerations.set(cacheKey, generation) - const linkedPRNumber = options?.linkedPRNumber ?? null const request = (async () => { try { const runtimeRepo = getRuntimeRepoTarget(get(), repoPath, requestSettings) @@ -1716,7 +1739,12 @@ export const createGitHubSlice: StateCreator = (s ? await callRuntimeRpc( runtimeRepo.target, 'github.prForBranch', - { repo: runtimeRepo.repo.id, branch, linkedPRNumber }, + { + repo: runtimeRepo.repo.id, + branch, + linkedPRNumber, + ...(fallbackPRNumber !== null ? { fallbackPRNumber } : {}) + }, { timeoutMs: 30_000 } ).then((pr) => pr @@ -1731,13 +1759,14 @@ export const createGitHubSlice: StateCreator = (s branch, cacheKey, linkedPRNumber, + fallbackPRNumber, connectionId: repo?.connectionId ?? null, cachedFetchedAt: cached?.fetchedAt ?? null } return window.api.gh.refreshPRNow ? await window.api.gh.refreshPRNow({ candidate }) : await window.api.gh - .prForBranch({ repoPath, repoId, branch, linkedPRNumber }) + .prForBranch({ repoPath, repoId, branch, linkedPRNumber, fallbackPRNumber }) .then((pr) => pr ? ({ kind: 'found', pr, fetchedAt: Date.now() } as const) @@ -1781,7 +1810,8 @@ export const createGitHubSlice: StateCreator = (s inflightPRRequests.set(cacheKey, { promise: request, force: Boolean(options?.force), - generation + generation, + lookupHintKey }) return request }, @@ -2075,7 +2105,8 @@ export const createGitHubSlice: StateCreator = (s void get().fetchPRForBranch(candidate.repoPath, candidate.branch, { force: bypassesGitHubPRRefreshFreshness(reason), repoId: candidate.repoId, - linkedPRNumber: candidate.linkedPRNumber ?? null + linkedPRNumber: candidate.linkedPRNumber ?? null, + fallbackPRNumber: candidate.fallbackPRNumber ?? null }) return } @@ -2087,7 +2118,8 @@ export const createGitHubSlice: StateCreator = (s return get().fetchPRForBranch(candidate.repoPath, candidate.branch, { force: bypassesGitHubPRRefreshFreshness(reason), repoId: candidate.repoId, - linkedPRNumber: candidate.linkedPRNumber ?? null + linkedPRNumber: candidate.linkedPRNumber ?? null, + fallbackPRNumber: candidate.fallbackPRNumber ?? null }) } return null @@ -2110,7 +2142,8 @@ export const createGitHubSlice: StateCreator = (s for (const candidate of candidates) { void get().fetchPRForBranch(candidate.repoPath, candidate.branch, { repoId: candidate.repoId, - linkedPRNumber: candidate.linkedPRNumber ?? null + linkedPRNumber: candidate.linkedPRNumber ?? null, + fallbackPRNumber: candidate.fallbackPRNumber ?? null }) } return @@ -2332,7 +2365,8 @@ export const createGitHubSlice: StateCreator = (s if (getRuntimeRepoTarget(state, candidate.repoPath)) { void get().fetchPRForBranch(candidate.repoPath, candidate.branch, { repoId: candidate.repoId, - linkedPRNumber: candidate.linkedPRNumber ?? null + linkedPRNumber: candidate.linkedPRNumber ?? null, + fallbackPRNumber: candidate.fallbackPRNumber ?? null }) } else { void window.api.gh.enqueuePRRefresh?.({ candidate, reason: 'swr', priority: 10 }) @@ -2387,7 +2421,8 @@ export const createGitHubSlice: StateCreator = (s void get().fetchPRForBranch(candidate.repoPath, candidate.branch, { force: true, repoId: candidate.repoId, - linkedPRNumber: candidate.linkedPRNumber ?? null + linkedPRNumber: candidate.linkedPRNumber ?? null, + fallbackPRNumber: candidate.fallbackPRNumber ?? null }) } else { void window.api.gh.enqueuePRRefresh?.({ candidate, reason: 'post-push', priority: 100 }) @@ -2567,7 +2602,8 @@ export const createGitHubSlice: StateCreator = (s void get().fetchPRForBranch(candidate.repoPath, candidate.branch, { force: true, repoId: candidate.repoId, - linkedPRNumber: candidate.linkedPRNumber ?? null + linkedPRNumber: candidate.linkedPRNumber ?? null, + fallbackPRNumber: candidate.fallbackPRNumber ?? null }) } else { void window.api.gh.enqueuePRRefresh?.({ candidate, reason: 'active', priority: 80 }) diff --git a/src/renderer/src/store/slices/hosted-review-cache-identity.ts b/src/renderer/src/store/slices/hosted-review-cache-identity.ts index 0ffad6175..c8e9a3db9 100644 --- a/src/renderer/src/store/slices/hosted-review-cache-identity.ts +++ b/src/renderer/src/store/slices/hosted-review-cache-identity.ts @@ -2,6 +2,7 @@ import type { GlobalSettings } from '../../../../shared/types' export type LinkedReviewHints = { linkedGitHubPR?: number | null + fallbackGitHubPR?: number | null linkedGitLabMR?: number | null linkedBitbucketPR?: number | null linkedAzureDevOpsPR?: number | null @@ -29,7 +30,7 @@ export function getHostedReviewCacheKey( // linked review number. Track that distinction without changing the cache key. export function linkedReviewHintKey(options?: LinkedReviewHints): string { const hints = [ - ['github', options?.linkedGitHubPR ?? null], + ['github', options?.linkedGitHubPR ?? options?.fallbackGitHubPR ?? null], ['gitlab', options?.linkedGitLabMR ?? null], ['bitbucket', options?.linkedBitbucketPR ?? null], ['azure-devops', options?.linkedAzureDevOpsPR ?? null], diff --git a/src/renderer/src/store/slices/hosted-review.ts b/src/renderer/src/store/slices/hosted-review.ts index ad133db21..fb70642f0 100644 --- a/src/renderer/src/store/slices/hosted-review.ts +++ b/src/renderer/src/store/slices/hosted-review.ts @@ -104,6 +104,7 @@ type RefreshHostedReviewCardArgs = { repoId: string branch: string linkedGitHubPR?: number | null + fallbackGitHubPR?: number | null linkedGitLabMR?: number | null linkedBitbucketPR?: number | null linkedAzureDevOpsPR?: number | null @@ -114,10 +115,12 @@ export function refreshHostedReviewCard( fetchHostedReviewForBranch: HostedReviewSlice['fetchHostedReviewForBranch'], args: RefreshHostedReviewCardArgs ): Promise { + const fallbackGitHubPR = args.linkedGitHubPR == null ? (args.fallbackGitHubPR ?? null) : null return fetchHostedReviewForBranch(args.repoPath, args.branch, { force: true, repoId: args.repoId, linkedGitHubPR: args.linkedGitHubPR ?? null, + ...(fallbackGitHubPR !== null ? { fallbackGitHubPR } : {}), linkedGitLabMR: args.linkedGitLabMR ?? null, linkedBitbucketPR: args.linkedBitbucketPR ?? null, linkedAzureDevOpsPR: args.linkedAzureDevOpsPR ?? null, @@ -218,10 +221,13 @@ export const createHostedReviewSlice: StateCreator { try { + const fallbackGitHubPR = + options?.linkedGitHubPR == null ? (options?.fallbackGitHubPR ?? null) : null const args = { branch, ...(options?.repoId !== undefined ? { repoId: options.repoId } : {}), linkedGitHubPR: options?.linkedGitHubPR ?? null, + ...(fallbackGitHubPR !== null ? { fallbackGitHubPR } : {}), linkedGitLabMR: options?.linkedGitLabMR ?? null, linkedBitbucketPR: options?.linkedBitbucketPR ?? null, linkedAzureDevOpsPR: options?.linkedAzureDevOpsPR ?? null, diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 82b3bac0c..a6983c004 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -40,6 +40,7 @@ globalThis.window = { api: mockApi } import { createWorktreeSlice } from './worktrees' import { getHostedReviewCacheKey } from './hosted-review' +import { getGitHubPRCacheKey, getLegacyGitHubPRCacheKey } from './github-cache-key' import { registerPersistentWebview, unregisterPersistentWebview @@ -1490,6 +1491,18 @@ describe('worktree remote runtime mutations', () => { }) const fetchHostedReviewForBranch = vi.fn().mockResolvedValue(null) const cacheKey = getHostedReviewCacheKey('/repo1', 'pr-branch', undefined, 'repo1') + const prCacheKey = getGitHubPRCacheKey('/repo1', 'repo1', 'pr-branch') + const legacyRepoPRCacheKey = getLegacyGitHubPRCacheKey('/repo1', 'repo1', 'pr-branch') + const legacyPathPRCacheKey = getLegacyGitHubPRCacheKey('/repo1', undefined, 'pr-branch') + const prData = { + number: 456, + title: 'Linked PR', + state: 'open' as const, + url: 'https://github.com/acme/repo/pull/456', + checksStatus: 'success' as const, + updatedAt: '2026-05-15T00:00:00.000Z', + mergeable: 'MERGEABLE' as const + } store.setState({ repos: [ { id: 'repo1', path: '/repo1', displayName: 'Repo 1', badgeColor: '#000', addedAt: 0 } @@ -1510,6 +1523,20 @@ describe('worktree remote runtime mutations', () => { fetchedAt: Date.now() } }, + prCache: { + [prCacheKey]: { + data: prData, + fetchedAt: Date.now() + }, + [legacyRepoPRCacheKey]: { + data: { ...prData, title: 'Legacy repo-scoped PR' }, + fetchedAt: Date.now() + }, + [legacyPathPRCacheKey]: { + data: { ...prData, title: 'Legacy path-scoped PR' }, + fetchedAt: Date.now() + } + }, fetchHostedReviewForBranch } as Partial) @@ -1517,6 +1544,9 @@ describe('worktree remote runtime mutations', () => { expect(store.getState().worktreesByRepo.repo1[0]?.linkedPR).toBeNull() expect(store.getState().hostedReviewCache[cacheKey]).toBeUndefined() + expect(store.getState().prCache[prCacheKey]).toBeUndefined() + expect(store.getState().prCache[legacyRepoPRCacheKey]).toBeUndefined() + expect(store.getState().prCache[legacyPathPRCacheKey]).toBeUndefined() expect(fetchHostedReviewForBranch).toHaveBeenCalledWith('/repo1', 'pr-branch', { repoId: 'repo1', linkedGitHubPR: null, diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index d3f763568..454808846 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -17,6 +17,7 @@ import { ensureHooksConfirmed } from '@/lib/ensure-hooks-confirmed' import { tabHasLivePty } from '@/lib/tab-has-live-pty' import { callRuntimeRpc, getActiveRuntimeTarget } from '../../runtime/runtime-rpc-client' import { getHostedReviewCacheKey } from './hosted-review' +import { getGitHubPRCacheKey, getLegacyGitHubPRCacheKey } from './github-cache-key' import { moveFocusToRendererBeforeFocusedWebviewHidden } from './browser-webview-cleanup' export type { WorktreeSlice, WorktreeDeleteState } from './worktree-helpers' @@ -1030,8 +1031,27 @@ export const createWorktreeSlice: StateCreator reviewRepo.connectionId ) : null + const prCacheKey = + reviewRepo && reviewBranch + ? getGitHubPRCacheKey( + reviewRepo.path, + reviewRepo.id, + reviewBranch, + s.settings, + reviewRepo.connectionId + ) + : null + const prCacheKeys = + reviewRepo && reviewBranch + ? [ + prCacheKey, + getLegacyGitHubPRCacheKey(reviewRepo.path, reviewRepo.id, reviewBranch), + getLegacyGitHubPRCacheKey(reviewRepo.path, undefined, reviewBranch) + ].filter((key): key is string => Boolean(key)) + : [] const hostedReviewCache = s.hostedReviewCache ?? {} - if (nextWorktrees === s.worktreesByRepo && !cacheKey) { + const prCache = s.prCache ?? {} + if (nextWorktrees === s.worktreesByRepo && !cacheKey && !prCacheKey) { return {} } @@ -1043,6 +1063,15 @@ export const createWorktreeSlice: StateCreator return next })() : hostedReviewCache + const nextPRCache = prCacheKeys.some((key) => prCache[key]) + ? (() => { + const next = { ...prCache } + for (const key of prCacheKeys) { + delete next[key] + } + return next + })() + : prCache return { ...(nextWorktrees !== s.worktreesByRepo @@ -1050,7 +1079,8 @@ export const createWorktreeSlice: StateCreator : {}), ...(nextHostedReviewCache !== hostedReviewCache ? { hostedReviewCache: nextHostedReviewCache } - : {}) + : {}), + ...(nextPRCache !== prCache ? { prCache: nextPRCache } : {}) } }) diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 34cc19ce5..759f64788 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -724,7 +724,8 @@ function createGitHubApi(): NonNullable['gh']> { repo: candidate.repoId || candidate.repoPath, repoPath: candidate.repoPath, branch: candidate.branch, - linkedPRNumber: candidate.linkedPRNumber ?? null + linkedPRNumber: candidate.linkedPRNumber ?? null, + fallbackPRNumber: candidate.fallbackPRNumber ?? null }) return pr ? { kind: 'found', pr, fetchedAt: Date.now() } diff --git a/src/shared/hosted-review.ts b/src/shared/hosted-review.ts index 3424b405d..d75131a56 100644 --- a/src/shared/hosted-review.ts +++ b/src/shared/hosted-review.ts @@ -28,6 +28,7 @@ export type HostedReviewForBranchArgs = { repoId?: string branch: string linkedGitHubPR?: number | null + fallbackGitHubPR?: number | null linkedGitLabMR?: number | null linkedBitbucketPR?: number | null linkedAzureDevOpsPR?: number | null @@ -118,6 +119,7 @@ export type HostedReviewCreationEligibilityArgs = { ahead?: number behind?: number linkedGitHubPR?: number | null + fallbackGitHubPR?: number | null linkedGitLabMR?: number | null linkedBitbucketPR?: number | null linkedAzureDevOpsPR?: number | null diff --git a/src/shared/types.ts b/src/shared/types.ts index 39b0fb6f5..ff23181a1 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -646,6 +646,7 @@ export type GitHubPRRefreshAlias = { export type GitHubPRRefreshCandidate = GitHubPRRefreshAlias & { linkedPRNumber?: number | null + fallbackPRNumber?: number | null repoKind: RepoKind repoId: string isBare?: boolean