diff --git a/packages/server/src/services/gitea-service.test.ts b/packages/server/src/services/gitea-service.test.ts index 06cf32098d..8feae65572 100644 --- a/packages/server/src/services/gitea-service.test.ts +++ b/packages/server/src/services/gitea-service.test.ts @@ -2097,6 +2097,30 @@ describe("createGiteaService", () => { ]); }); + it("discovers the repository from tea when origin and a head path are unavailable", async () => { + const pr = currentPullRequestApi({ + number: 8, + state: "closed", + headRef: "topic", + headSha: "1".repeat(40), + merged: true, + }); + const { service } = makeService( + (args) => { + if (args[0] === "api" && args[1] === "repos/{owner}/{repo}") + return ok(JSON.stringify({ full_name: "example-user/sample-repo" })); + if (args[0] === "pr" && args[1] === "list") return ok("[]"); + if (args[0] === "api" && args[1].includes("pulls?state=all")) + return ok(JSON.stringify([pr])); + throw new Error(`unexpected call: ${args.join(" ")}`); + }, + { resolveRemoteUrl: async () => null }, + ); + expect( + (await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "topic" }))?.number, + ).toBe(8); + }); + it("shows a merged PR when the checkout head differs", async () => { const stale = currentPullRequestApi({ number: 8, diff --git a/packages/server/src/services/gitea-service.ts b/packages/server/src/services/gitea-service.ts index 3b449fb2ca..3699ab9468 100644 --- a/packages/server/src/services/gitea-service.ts +++ b/packages/server/src/services/gitea-service.ts @@ -1638,6 +1638,14 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For (input.headRepositoryOwner ? null : repoIdentity && `${repoIdentity.owner}/${repoIdentity.name}`); + if (!expectedPath && !input.headRepositoryOwner && !repoIdentity) { + const repository = await runJson( + ["api", "repos/{owner}/{repo}"], + { cwd: input.cwd }, + GiteaPullRequestRepoSchema, + ); + expectedPath = repository.full_name ?? null; + } if (!expectedPath && input.headRepositoryOwner && input.changeRequestNumber) { const hint = await runJson( ["api", `${repoPath}/pulls/${input.changeRequestNumber}`], diff --git a/packages/server/src/services/github-service.test.ts b/packages/server/src/services/github-service.test.ts index bae1ec4844..fafac7f286 100644 --- a/packages/server/src/services/github-service.test.ts +++ b/packages/server/src/services/github-service.test.ts @@ -18,6 +18,7 @@ import { } from "./github-service.js"; import { isPlatform } from "../test-utils/platform.js"; import { CheckoutPrStatusResponseSchema } from "@getpaseo/protocol/messages"; +import { ForgeDiscoveryIncompleteError } from "./forge-service.js"; const EXPECTED_GITHUB_FAST_POLL_MS = 20_000; const EXPECTED_GITHUB_SLOW_POLL_MS = 120_000; @@ -1874,6 +1875,82 @@ describe("ForgeService", () => { service.dispose?.(); }); + it("uses canonical repository identity for batched polls after a transfer", async () => { + const node = batchPollPrNodeJson({ + state: "OPEN", + mergedAt: null, + headRepositoryOwner: { login: "new-owner" }, + headRepository: { name: "repo" }, + }); + const runner = createScriptedRunner([ + batchPollStatusJson({ + t0: batchPollRepositoryJson([node], { owner: { login: "new-owner" }, name: "repo" }), + }), + batchPollStatusJson({ t0: batchPollChecksAliasJson([]) }), + ]); + const service = createGitHubService({ + ttlMs: 0, + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + resolveRepoHost: async () => null, + resolveRepoSlug: async () => "old-owner/repo", + }); + const statuses: Array = []; + const subscription = service.retainCurrentPullRequestStatusPoll?.({ + cwd: "/ws-a", + headRef: "feat-a", + headRepositoryPath: "old-owner/repo", + onStatus: (status) => statuses.push(status), + }); + await vi.advanceTimersByTimeAsync(0); + await flushMicrotasks(); + expect(statuses).toEqual([expect.objectContaining({ number: 42, state: "open" })]); + subscription?.unsubscribe(); + service.dispose?.(); + }); + + it("resolves a transferred fork during batched polling", async () => { + const node = batchPollPrNodeJson({ + state: "OPEN", + mergedAt: null, + headRepositoryOwner: { login: "new-fork" }, + headRepository: { name: "repo" }, + }); + const pr = currentPullRequestJson({ + url: "https://github.com/base/repo/pull/42", + headRefName: "feat-a", + headRepositoryOwner: { login: "new-fork" }, + headRepository: { name: "repo" }, + }); + const runner = createScriptedRunner([ + batchPollStatusJson({ t0: batchPollRepositoryJson([node]) }), + pr, + JSON.stringify([JSON.parse(pr)]), + JSON.stringify({ full_name: "new-fork/repo" }), + currentPullRequestGithubFactsJson(), + ]); + const service = createGitHubService({ + ttlMs: 0, + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + resolveRepoHost: async () => null, + resolveRepoSlug: async () => "base/repo", + }); + const statuses: Array = []; + const subscription = service.retainCurrentPullRequestStatusPoll?.({ + cwd: "/ws-a", + headRef: "feat-a", + headRepositoryOwner: "old-fork", + headRepositoryPath: "old-fork/repo", + onStatus: (status) => statuses.push(status), + }); + await vi.advanceTimersByTimeAsync(0); + await flushMicrotasks(); + expect(statuses).toEqual([expect.objectContaining({ number: 42, state: "open" })]); + subscription?.unsubscribe(); + service.dispose?.(); + }); + it("recovers healthy aliases from a partially failed batch response", async () => { let now = 0; const partialBody = batchPollStatusJson({ @@ -3747,12 +3824,62 @@ describe("ForgeService", () => { runner: runner.runner, resolveGhPath: async () => "/usr/bin/gh", }); - expect( - await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "feature/fork" }), - ).toBeNull(); + await expect( + service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "feature/fork" }), + ).rejects.toBeInstanceOf(ForgeDiscoveryIncompleteError); expect(runner.calls.filter((call) => call.args[0] === "api")).toHaveLength(1); }); + it("matches a repository transferred while the checkout retains its old origin URL", async () => { + const pr = currentPullRequestJson({ + url: "https://github.com/new-owner/repo/pull/42", + headRepositoryOwner: { login: "new-owner" }, + headRepository: { name: "repo" }, + }); + const runner = createScriptedRunner([ + pr, + JSON.stringify({ owner: { login: "new-owner" }, name: "repo", parent: null }), + JSON.stringify([JSON.parse(pr)]), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + resolveRepoSlug: async () => "old-owner/repo", + }); + expect( + await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headRepositoryPath: "old-owner/repo", + }), + ).toMatchObject({ number: 42 }); + }); + + it("matches a transferred fork using its canonical repository identity", async () => { + const pr = currentPullRequestJson({ + url: "https://github.com/base/repo/pull/42", + headRepositoryOwner: { login: "new-fork" }, + headRepository: { name: "repo" }, + }); + const runner = createScriptedRunner([ + pr, + JSON.stringify([JSON.parse(pr)]), + JSON.stringify({ full_name: "new-fork/repo" }), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + expect( + await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headRepositoryOwner: "old-fork", + headRepositoryPath: "old-fork/repo", + }), + ).toMatchObject({ number: 42 }); + }); + it("propagates a failed fork PR lookup instead of reporting no pull request", async () => { const rateLimitError = new GitHubCommandError({ args: ["api", "repos/parentOwner/parentRepo/pulls"], diff --git a/packages/server/src/services/github-service.ts b/packages/server/src/services/github-service.ts index 4090e61da0..a74ff31898 100644 --- a/packages/server/src/services/github-service.ts +++ b/packages/server/src/services/github-service.ts @@ -29,6 +29,7 @@ import { normalizeForgeSearchKinds, parseOptionalTime, matchesHeadRepositoryEndpoint, + ForgeDiscoveryIncompleteError, } from "./forge-service.js"; import type { CheckAnnotation, @@ -666,6 +667,7 @@ const BatchPollPrNodeSchema = z.object({ const BatchPollRepositorySchema = z.object({ owner: z.object({ login: z.string().optional() }).nullable().optional().catch(null), + name: z.string().optional(), isFork: z.boolean().optional().catch(false), parent: z .object({ owner: z.object({ login: z.string() }), name: z.string() }) @@ -797,6 +799,7 @@ function buildBatchPullRequestStatusQuery( owner { login } + name isFork parent { owner { @@ -1108,6 +1111,7 @@ interface GitHubBatchPollEntry { owner: string; name: string; headRepositoryOwner?: string; + canonicalHeadRepositoryPath?: string; } interface PendingBatchPullRequestChecks { @@ -1179,7 +1183,12 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G const frozenPullRequestChecksCache = new Map(); const pollRepositoryRedirects = new Map< string, - { owner: string; name: string; headRepositoryOwner: string } + { + owner: string; + name: string; + headRepositoryOwner: string; + canonicalHeadRepositoryPath: string; + } >(); const pollPausedUntilByHost = new Map(); const rateLimitResetLoads = new Map>(); @@ -1754,6 +1763,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G owner: repository.data.parent.owner.login, name: repository.data.parent.name, headRepositoryOwner: forkOwner, + canonicalHeadRepositoryPath: `${forkOwner}/${repository.data.name ?? entry.originName}`, }); distribution.redirected.push(entry); continue; @@ -1764,8 +1774,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G } const node = selectBatchPollNode(entry, repository.data); if (!node) { - // A fork's PR can live in the parent repository. - if (repository.data.isFork) { + if (shouldRetryUnmatchedBatch(entry, repository.data)) { distribution.legacyFallback.push(entry.target); } else { finalizeBatchPollTarget({ @@ -1778,10 +1787,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G } continue; } - const frozenKey = - node.state !== "OPEN" && node.headRefOid.length > 0 - ? `${entry.owner}/${entry.name}#${node.number}@${node.headRefOid}` - : null; + const frozenKey = frozenPullRequestKey(entry, node); const cachedRollup = frozenKey ? frozenPullRequestChecksCache.get(frozenKey) : undefined; if (frozenKey && cachedRollup) { rememberFrozenPullRequestChecks(frozenKey, cachedRollup); @@ -1804,6 +1810,24 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G return distribution; } + function shouldRetryUnmatchedBatch( + entry: GitHubBatchPollEntry, + repository: BatchPollRepository, + ): boolean { + return Boolean( + repository.isFork || + (entry.target.headRepositoryOwner && + entry.target.headRepositoryPath && + repository.pullRequests.nodes.length > 0), + ); + } + + function frozenPullRequestKey(entry: GitHubBatchPollEntry, node: BatchPollPrNode): string | null { + return node.state !== "OPEN" && node.headRefOid.length > 0 + ? `${entry.owner}/${entry.name}#${node.number}@${node.headRefOid}` + : null; + } + function addressGitHubBatchEntry(entry: GitHubBatchPollEntry): GitHubBatchPollEntry { const redirect = pollRepositoryRedirects.get(batchRepositoryRedirectKey(entry)); return redirect @@ -1812,6 +1836,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G owner: redirect.owner, name: redirect.name, headRepositoryOwner: redirect.headRepositoryOwner, + canonicalHeadRepositoryPath: redirect.canonicalHeadRepositoryPath, } : entry; } @@ -1857,7 +1882,12 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G headRef: entry.target.headRef, headRepositoryOwner, headRepositoryPath: - entry.target.headRepositoryPath ?? `${entry.originOwner}/${entry.originName}`, + !entry.target.headRepositoryOwner && + entry.target.headRepositoryPath?.toLowerCase() === + `${entry.originOwner}/${entry.originName}`.toLowerCase() + ? (entry.canonicalHeadRepositoryPath ?? + `${repository.owner?.login ?? entry.originOwner}/${repository.name ?? entry.originName}`) + : (entry.target.headRepositoryPath ?? `${entry.originOwner}/${entry.originName}`), }); return match ? (nodeByCandidate.get(match) ?? null) : null; } @@ -2220,6 +2250,10 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G headSha: input.headSha, headRepositoryOwner: input.headRepositoryOwner, headRepositoryPath: input.headRepositoryPath, + originRepositoryPath: + input.headRepositoryPath && !input.headRepositoryOwner + ? await resolveRepoSlugCached(input.cwd) + : null, changeRequestNumber: input.changeRequestNumber, run, }); @@ -3035,6 +3069,7 @@ interface CurrentPullRequestLookup { headSha?: string; headRepositoryOwner?: string; headRepositoryPath?: string; + originRepositoryPath?: string | null; changeRequestNumber?: number; run: (args: string[], options: GitHubCommandRunnerOptions) => Promise; } @@ -3087,7 +3122,11 @@ async function resolvePullRequestWithoutOwner( const parentOwner = repo?.parent?.owner?.login; const parentName = repo?.parent?.name; if (!forkOwner || !forkName) return null; - const headRepositoryPath = options.headRepositoryPath ?? `${forkOwner}/${forkName}`; + const headRepositoryPath = + options.headRepositoryPath && + options.headRepositoryPath.toLowerCase() !== options.originRepositoryPath?.toLowerCase() + ? options.headRepositoryPath + : `${forkOwner}/${forkName}`; options = { ...options, headRepositoryPath }; const expectedOwner = headRepositoryPath.split("/")[0]; if (!expectedOwner) return null; @@ -3113,12 +3152,37 @@ async function resolveListedPullRequest( if (candidates.length >= BATCH_PR_CANDIDATE_LIMIT && options.headRepositoryPath) { return resolveCrowdedPullRequests(options, options.headRepositoryPath); } - const match = pickPullRequestCandidate({ + let match = pickPullRequestCandidate({ candidates: viewMatch ? [viewMatch, ...candidates] : candidates, headRef: options.headRef, headRepositoryOwner, headRepositoryPath: options.headRepositoryPath, }); + if ( + !match && + options.headRepositoryOwner && + options.headRepositoryPath && + candidates.some((candidate) => isCandidateForHeadRef(candidate, options.headRef)) + ) { + const args = ["api", `repos/${options.headRepositoryPath}`]; + const stdout = await options.run(args, { cwd: options.cwd }); + const repository = parseGitHubJsonOutput( + stdout, + z.object({ full_name: z.string().optional() }), + { args, cwd: options.cwd, emptyFallback: "{}" }, + ); + if ( + repository.full_name && + repository.full_name.toLowerCase() !== options.headRepositoryPath.toLowerCase() + ) { + match = pickPullRequestCandidate({ + candidates: viewMatch ? [viewMatch, ...candidates] : candidates, + headRef: options.headRef, + headRepositoryOwner: repository.full_name.split("/")[0], + headRepositoryPath: repository.full_name, + }); + } + } return match?.status ?? null; } @@ -3221,7 +3285,10 @@ async function listForkPullRequestNumbers(options: { parseOptionalTime(b.updated_at) - parseOptionalTime(a.updated_at) || b.number - a.number, ); - if (refs.length >= FORK_PR_PAGE_SIZE && !matching.some((ref) => ref.state === "open")) return []; + if (refs.length >= FORK_PR_PAGE_SIZE && !matching.some((ref) => ref.state === "open")) + throw new ForgeDiscoveryIncompleteError( + "GitHub pull request discovery exceeded its candidate window", + ); return matching.slice(0, FORK_PR_VIEW_LIMIT).map((ref) => ref.number); } diff --git a/packages/server/src/services/gitlab-service.test.ts b/packages/server/src/services/gitlab-service.test.ts index 5d81bbf55f..1f86da3c3e 100644 --- a/packages/server/src/services/gitlab-service.test.ts +++ b/packages/server/src/services/gitlab-service.test.ts @@ -262,6 +262,36 @@ const DISCUSSIONS = [ ]; describe("createGitLabService", () => { + it.each([true, false])( + "discovers the project without an origin remote (head path supplied: %s)", + async (hasHeadPath) => { + const mr = { ...OPEN_MR, source_branch: "topic" }; + const { service } = makeService( + (args) => { + if (args[1] === "list") return ok(JSON.stringify([mr])); + if (args[1] === "view") return ok(JSON.stringify(mr)); + if ( + args[1] === "projects/:fullpath" || + args[1] === "projects/example-group%2Fexample-project" + ) + return ok( + JSON.stringify({ id: 101, path_with_namespace: "example-group/example-project" }), + ); + return ok("{}"); + }, + { resolveRemoteUrl: async () => null }, + ); + expect( + ( + await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "topic", + ...(hasHeadPath ? { headRepositoryPath: "example-group/example-project" } : {}), + }) + )?.number, + ).toBe(14); + }, + ); it("requires native clone URL proof for mixed transport ports and isolates cached endpoints", async () => { const mr = { ...OPEN_MR, source_branch: "topic" }; let cloneUrl: string | undefined = diff --git a/packages/server/src/services/gitlab-service.ts b/packages/server/src/services/gitlab-service.ts index 31b7b4e8cc..5e13b8cc60 100644 --- a/packages/server/src/services/gitlab-service.ts +++ b/packages/server/src/services/gitlab-service.ts @@ -988,22 +988,24 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F } async function resolveExpectedHeadProject( - cwd: string, - headRepositoryPath?: string, - headRepositoryOwner?: string, - changeRequestNumber?: number, - force = false, + input: Parameters[0], ): Promise { - const remote = await resolveRemoteUrl(cwd); - const remotePath = remote ? parseGitRemoteLocation(remote)?.path : null; - if (!remotePath) return null; - if (headRepositoryPath) return headRepositoryPath; - if (!headRepositoryOwner) return remotePath; - if (!changeRequestNumber) return null; - const hint = await viewMergeRequest(cwd, String(changeRequestNumber)); + if (input.headRepositoryPath) return input.headRepositoryPath; + if (!input.headRepositoryOwner) { + const remote = await resolveRemoteUrl(input.cwd); + return ( + (remote ? parseGitRemoteLocation(remote)?.path : null) ?? + (await readProject(input.cwd, ":fullpath", input.force === true)).path_with_namespace + ); + } + if (!input.changeRequestNumber) return null; + const hint = await viewMergeRequest(input.cwd, String(input.changeRequestNumber)); if (hint.source_project_id == null) return null; - const path = (await readProject(cwd, hint.source_project_id, force)).path_with_namespace; - return path.split("/")[0]?.toLowerCase() === headRepositoryOwner.toLowerCase() ? path : null; + const path = (await readProject(input.cwd, hint.source_project_id, input.force === true)) + .path_with_namespace; + return path.split("/")[0]?.toLowerCase() === input.headRepositoryOwner.toLowerCase() + ? path + : null; } async function readProject(cwd: string, projectId: number | string, force = false) { @@ -1011,7 +1013,7 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F const cached = force ? undefined : projectPaths.get(key); if (cached) return cached; const source = await runJson( - ["api", `projects/${encodeURIComponent(projectId)}`], + ["api", `projects/${projectId === ":fullpath" ? projectId : encodeURIComponent(projectId)}`], { cwd }, z.object({ id: z.number().optional(), @@ -1034,7 +1036,7 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F const remote = await resolveRemoteUrl(input.cwd); const originPath = remote ? parseGitRemoteLocation(remote)?.path : null; const targetId = - input.expectedPath.toLowerCase() === originPath?.toLowerCase() + !originPath || input.expectedPath.toLowerCase() === originPath.toLowerCase() ? input.mergeRequests.find( (mr) => extractProjectPath(mr.references?.full)?.toLowerCase() === @@ -1054,23 +1056,13 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F } async function resolveCurrentMergeRequest( - cwd: string, - headRef: string, - headRepositoryPath?: string, - headRepositoryOwner?: string, - changeRequestNumber?: number, - force = false, - headRepositoryEndpoint?: import("./forge-service.js").HeadRepositoryEndpoint, + input: Parameters[0], ): Promise { + const { cwd, headRef, headRepositoryEndpoint } = input; + const force = input.force === true; const mergeRequests = await listMergeRequestsBySourceBranch(cwd, headRef); if (mergeRequests.length === 0) return null; - const expectedPath = await resolveExpectedHeadProject( - cwd, - headRepositoryPath, - headRepositoryOwner, - changeRequestNumber, - force, - ); + const expectedPath = await resolveExpectedHeadProject(input); if (!expectedPath) return null; const expectedProjectId = await resolveExpectedProjectId({ cwd, @@ -1266,15 +1258,7 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F async getCurrentPullRequestStatus(input): Promise { try { - const mr = await resolveCurrentMergeRequest( - input.cwd, - input.headRef, - input.headRepositoryPath, - input.headRepositoryOwner, - input.changeRequestNumber, - input.force === true, - input.headRepositoryEndpoint, - ); + const mr = await resolveCurrentMergeRequest(input); if (!mr) { return null; }