diff --git a/docs/forge-providers.md b/docs/forge-providers.md index f69c7eb64a..374e0fd42b 100644 --- a/docs/forge-providers.md +++ b/docs/forge-providers.md @@ -97,19 +97,28 @@ Register the adapter in `defaultForgeRegistry` with: - `matchesHost` from manifest `cloudHosts` - `probeHost` when self-hosted/Enterprise detection is supported -Current change-request lookup uses two identities deliberately: +Match a change request using the head repository and branch. Within the bounded +candidate window, prefer an open request, otherwise select the most recently +updated terminal request, breaking ties by number. A full inconclusive window is +a failed discovery, so a poll retains its previous status. Gitea does not expose +a source-branch filter: after its bounded open window it also examines one recent +all-state page. An older open request outside both windows cannot take precedence +over a recent terminal request on a cold lookup. When a workspace already shows +an open request, revalidate it before replacing it with another terminal request. +A reused branch can display a historical request +until a new one opens. Display does not depend on checkout HEAD: the checkout can +be behind the remote PR after its branch is updated during merge. -- An open PR/MR belongs to the checkout when its head branch and head repository - match. Its remote head SHA may differ because the checkout can be ahead, - behind, or contain commits that have not been pushed yet. -- A merged or closed PR/MR belongs to the checkout only when its recorded head - SHA exactly matches the checkout's current `HEAD`. Branch names are reusable; - selecting the newest terminal request by branch alone can silently attach an - old promotion or feature request to new work. +For mixed SSH and HTTPS ports on one host, require the source repository's native +clone URL to match the normalized checkout endpoint. Different HTTP ports remain +distinct forge instances. Unknown clone URLs do not establish identity. -Thread the checkout head SHA through adapter cache and poll identities as well -as the lookup itself. Otherwise a commit made on the same branch can inherit the -previous commit's cached terminal status until the cache expires. +Auto-archive has a narrower contract. It requires a fresh, observed open-to-merged +transition and an exact match between the checkout HEAD and the forge's recorded +PR head, a successful upstream safety read, and the workspace safety checks. A +successful read reporting no upstream is distinct from a failed read. Keep +checkout HEAD in cache and poll identities so a checkout change refreshes those +safety facts. Cloud hosts in the manifest are a bounded public-host list, not a self-host allowlist. Self-hosted detection is a trust gate: Paseo only talks to a forge diff --git a/packages/app/e2e/support/helpers/e2e-worker.ts b/packages/app/e2e/support/helpers/e2e-worker.ts index 1f1cf783a2..c3f567c79b 100644 --- a/packages/app/e2e/support/helpers/e2e-worker.ts +++ b/packages/app/e2e/support/helpers/e2e-worker.ts @@ -94,6 +94,9 @@ if (origin === fixtureRemote) { labels: [], baseRefName: "main", headRefName: isFork ? "pr-branch-2" : "pr-branch-1", + headRefOid: spawnSync("git", ["rev-parse", "HEAD"], { encoding: "utf8" }).stdout?.trim() || "", + headRepositoryOwner: { login: isFork ? "fork-owner" : "paseo-e2e" }, + headRepository: { name: "local-fixture" }, updatedAt: "2026-01-01T00:00:00Z" }; process.stdout.write(JSON.stringify(command === "pr list" ? [pr] : pr)); diff --git a/packages/server/src/server/auto-archive-on-merge/archive-if-safe.test.ts b/packages/server/src/server/auto-archive-on-merge/archive-if-safe.test.ts index f050237620..4540114a95 100644 --- a/packages/server/src/server/auto-archive-on-merge/archive-if-safe.test.ts +++ b/packages/server/src/server/auto-archive-on-merge/archive-if-safe.test.ts @@ -14,6 +14,8 @@ import { import type { ArchiveResult, ActiveWorkspaceRef } from "../workspace-archive-service.js"; import type { WorkspaceGitRuntimeSnapshot } from "../workspace-git-service.js"; import { createWorktree, type WorktreeConfig } from "../../utils/worktree.js"; +import { createGitLabService } from "../../services/gitlab-service.js"; +import { createGiteaService } from "../../services/gitea-service.js"; import type { ForgeService } from "../../../services/forge-service.js"; import type { StoredAgentRecord } from "../agent/agent-storage.js"; @@ -30,6 +32,7 @@ function createPullRequest( state: "open", baseRefName: "main", headRefName: "feature", + headSha: "same-head", isMerged: true, ...overrides, }; @@ -46,6 +49,8 @@ function createSnapshot(overrides?: { repoRoot: "/tmp/repo", mainRepoRoot: "/tmp/repo", currentBranch: "feature", + upstreamKnown: true, + headSha: "same-head", remoteUrl: "https://github.com/acme/repo.git", isPaseoOwnedWorktree: true, isDirty: false, @@ -346,6 +351,89 @@ afterEach(() => { }); describe("archiveIfSafe", () => { + test("archives an exact-head GitLab merge using the adapter status", async () => { + const mr = { + iid: 123, + title: "Merge me", + web_url: "https://gitlab.example.com/acme/repo/-/merge_requests/123", + state: "merged", + source_branch: "feature", + target_branch: "main", + source_project_id: 1, + target_project_id: 1, + sha: "same-head", + references: { full: "acme/repo!123" }, + }; + const forge = createGitLabService({ + resolveGlabPath: async () => "/test/glab", + resolveRemoteUrl: async () => "https://gitlab.example.com/acme/repo.git", + runner: async (args) => ({ + stdout: JSON.stringify(args[1] === "list" ? [mr] : mr), + stderr: "", + }), + }); + const status = await forge.getCurrentPullRequestStatus({ cwd: CWD, headRef: "feature" }); + expect(status).toMatchObject({ isMerged: true, headSha: "same-head" }); + const harness = createHarness({ snapshot: createSnapshot({ pullRequest: status }) }); + await runArchiveIfSafe(harness); + expect(harness.deps.archiveByScope).toHaveBeenCalledTimes(1); + }); + + test("archives a Gitea merge when optional checks fail but selected head is known", async () => { + const pr = { + number: 123, + html_url: "https://gitea.com/acme/repo/pulls/123", + title: "Merge me", + state: "closed", + merged: true, + head: { + ref: "feature", + sha: "same-head", + repo: { id: 1, full_name: "acme/repo", owner: { login: "acme" } }, + }, + base: { ref: "main", repo: { id: 1, owner: { login: "acme" } } }, + }; + const forge = createGiteaService({ + resolveTeaPath: async () => "/test/tea", + resolveRemoteUrl: async () => "https://gitea.com/acme/repo.git", + runner: async (args) => { + if (args[0] === "pr" && args[1] === "list") return { stdout: "[]", stderr: "" }; + if (args[0] === "api" && args[1].includes("/pulls?")) + return { stdout: JSON.stringify([pr]), stderr: "" }; + throw new Error("checks unavailable"); + }, + }); + const status = await forge.getCurrentPullRequestStatus({ cwd: CWD, headRef: "feature" }); + expect(status).toMatchObject({ isMerged: true, headSha: "same-head" }); + const harness = createHarness({ snapshot: createSnapshot({ pullRequest: status }) }); + await runArchiveIfSafe(harness); + expect(harness.deps.archiveByScope).toHaveBeenCalledTimes(1); + }); + test("does not archive when the checkout and merged PR heads differ", async () => { + const harness = createHarness({ + snapshot: createSnapshot({ + git: { headSha: "checkout-head" } as Partial, + pullRequest: createPullRequest({ headSha: "remote-head" } as Partial< + NonNullable + >), + }), + }); + + await runArchiveIfSafe(harness); + + expect(harness.deps.archiveByScope).not.toHaveBeenCalled(); + }); + test("does not archive when either head or the worktree state is unavailable", async () => { + for (const snapshot of [ + createSnapshot({ git: { headSha: undefined } }), + createSnapshot({ pullRequest: createPullRequest({ headSha: undefined }) }), + createSnapshot({ git: { isDirty: null } }), + ]) { + const harness = createHarness({ snapshot }); + await runArchiveIfSafe(harness); + expect(harness.deps.archiveByScope).not.toHaveBeenCalled(); + } + }); beforeEach(() => { vi.clearAllMocks(); }); @@ -381,6 +469,26 @@ describe("archiveIfSafe", () => { expect(harness.deps.archiveByScope).not.toHaveBeenCalled(); }); + test("does not archive when the upstream lookup failed", async () => { + const harness = createHarness({ + snapshot: createSnapshot({ + git: { aheadOfOrigin: null, upstreamKnown: false } as Partial< + WorkspaceGitRuntimeSnapshot["git"] + >, + }), + }); + await runArchiveIfSafe(harness); + expect(harness.deps.archiveByScope).not.toHaveBeenCalled(); + }); + + test("does not archive when upstream safety provenance is missing", async () => { + const harness = createHarness({ + snapshot: createSnapshot({ git: { aheadOfOrigin: 0, upstreamKnown: undefined } }), + }); + await runArchiveIfSafe(harness); + expect(harness.deps.archiveByScope).not.toHaveBeenCalled(); + }); + test("archives when the PR is merged and the upstream branch was deleted", async () => { const harness = createHarness({ snapshot: createSnapshot({ git: { aheadOfOrigin: null, behindOfOrigin: null } }), diff --git a/packages/server/src/server/auto-archive-on-merge/archive-if-safe.ts b/packages/server/src/server/auto-archive-on-merge/archive-if-safe.ts index be8a6f4cea..ae75cbcc1e 100644 --- a/packages/server/src/server/auto-archive-on-merge/archive-if-safe.ts +++ b/packages/server/src/server/auto-archive-on-merge/archive-if-safe.ts @@ -47,6 +47,19 @@ const defaultDependencies: ArchiveIfSafeDependencies = { killTerminalsForWorkspace, }; +function isEligibleForMergeArchive(snapshot: WorkspaceGitRuntimeSnapshot): boolean { + const pullRequest = snapshot.forge.pullRequest; + return Boolean( + pullRequest?.isMerged && + !snapshot.forge.error && + snapshot.git.headSha && + snapshot.git.headSha === pullRequest.headSha && + snapshot.git.isDirty === false && + snapshot.git.upstreamKnown === true && + (snapshot.git.aheadOfOrigin === 0 || snapshot.git.aheadOfOrigin === null), + ); +} + export async function archiveIfSafe(input: { workspaceId: string; snapshot: WorkspaceGitRuntimeSnapshot; @@ -59,13 +72,7 @@ export async function archiveIfSafe(input: { const cwd = snapshot.cwd; const pullRequest = snapshot.forge.pullRequest; - if (!pullRequest?.isMerged) { - return; - } - if (snapshot.git.isDirty === true) { - return; - } - if (typeof snapshot.git.aheadOfOrigin === "number" && snapshot.git.aheadOfOrigin > 0) { + if (!pullRequest || !isEligibleForMergeArchive(snapshot)) { return; } diff --git a/packages/server/src/server/auto-archive-on-merge/behavior.test.ts b/packages/server/src/server/auto-archive-on-merge/behavior.test.ts index be952b8efa..f046066e6f 100644 --- a/packages/server/src/server/auto-archive-on-merge/behavior.test.ts +++ b/packages/server/src/server/auto-archive-on-merge/behavior.test.ts @@ -30,8 +30,8 @@ interface PullRequestObservation { const cleanupPaths: string[] = []; const cleanupServices: WorkspaceGitServiceImpl[] = []; -function run(cwd: string, args: string[]): void { - execFileSync("git", args, { cwd, stdio: "pipe" }); +function run(cwd: string, args: string[]): string { + return execFileSync("git", args, { cwd, encoding: "utf8", stdio: "pipe" }); } async function settleObservation(): Promise { @@ -63,6 +63,7 @@ async function createWorkspaceFiles() { async function createWorkspaceJourney() { const { repoDir, worktree } = await createWorkspaceFiles(); + const remoteHeadSha = run(worktree.worktreePath, ["rev-parse", "HEAD"]).trim(); const workspaceId = "workspace-under-test"; let active = true; let currentPullRequest: PullRequestObservation | null = null; @@ -81,6 +82,7 @@ async function createWorkspaceJourney() { repoRoot: worktree.worktreePath, mainRepoRoot: repoDir, currentBranch: branch, + headSha: run(worktree.worktreePath, ["rev-parse", "HEAD"]).trim(), remoteUrl: "https://github.com/acme/repo.git", isPaseoOwnedWorktree: true, isDirty: false, @@ -102,6 +104,7 @@ async function createWorkspaceJourney() { state: currentPullRequest.state, baseRefName: "main", headRefName: currentPullRequest.headRefName, + headSha: remoteHeadSha, isMerged: currentPullRequest.state === "merged", } : null, @@ -161,9 +164,13 @@ async function createWorkspaceJourney() { }; } -async function createUnobservedWorkspace(archiveBarrier?: Promise) { +async function createUnobservedWorkspace( + archiveBarrier?: Promise, + changeBeforeArchiveRead = false, +) { const { repoDir, paseoHome, worktree } = await createWorkspaceFiles(); const cwd = worktree.worktreePath; + let remoteHeadSha = run(cwd, ["rev-parse", "HEAD"]).trim(); run(repoDir, ["remote", "add", "origin", "https://github.com/acme/repo.git"]); run(repoDir, ["update-ref", "refs/remotes/origin/workspace", "HEAD"]); run(cwd, ["branch", "--set-upstream-to=origin/workspace"]); @@ -176,38 +183,58 @@ async function createUnobservedWorkspace(archiveBarrier?: Promise) { const reads: Array = []; const archives: string[] = []; let active = true; - const service = new WorkspaceGitServiceImpl({ - logger, - paseoHome, - deps: { - forgeOverrides: { github }, - getPullRequestStatus: async (_cwd, _forge, options) => { - reads.push(options?.reason); - return { - status: observation && { - ...observation, - title: observation.headRefName, - baseRefName: "main", - isMerged: observation.state === "merged", - }, - authState: "authenticated", - featuresEnabled: true, - githubFeaturesEnabled: true, - }; + function createService() { + const service = new WorkspaceGitServiceImpl({ + logger, + paseoHome, + deps: { + forgeOverrides: { github }, + getPullRequestStatus: async (_cwd, _forge, options) => { + reads.push(options?.reason); + return { + status: observation && { + ...observation, + headSha: remoteHeadSha, + title: observation.headRefName, + baseRefName: "main", + isMerged: observation.state === "merged", + }, + authState: "authenticated", + featuresEnabled: true, + githubFeaturesEnabled: true, + }; + }, }, - }, - }); - cleanupServices.push(service); + }); + cleanupServices.push(service); + return service; + } + let service = createService(); const options = { logger, paseoHome, daemonConfigStore: { get: () => ({ autoArchiveAfterMerge: true }) }, - workspaceGitService: service, + workspaceGitService: { + onSnapshotUpdated: (listener: (snapshot: WorkspaceGitRuntimeSnapshot) => void) => + service.onSnapshotUpdated(listener), + getSnapshot: async ( + snapshotCwd: string, + readOptions?: { reason?: string; force?: boolean }, + ) => { + if (changeBeforeArchiveRead && readOptions?.reason === "auto-archive-on-merge") { + changeBeforeArchiveRead = false; + writeFileSync(path.join(cwd, "README.md"), "changed after notification\n"); + run(cwd, ["add", "README.md"]); + run(cwd, ["-c", "commit.gpgsign=false", "commit", "-m", "after notification"]); + } + return service.getSnapshot(snapshotCwd, readOptions); + }, + }, getAutoArchivedChangeRequestUrl: async () => null, listActiveWorkspaces: async () => active ? [{ workspaceId: "workspace-under-test", cwd, kind: "worktree" }] : [], } as unknown as AutoArchiveOnMergeOptions; - setupAutoArchiveOnMerge(options, { + const archiveDependencies: AutoArchiveOnMergeDependencies = { resolvePath: path.resolve, archiveIfSafe: async (input) => archiveIfSafe({ @@ -228,12 +255,22 @@ async function createUnobservedWorkspace(archiveBarrier?: Promise) { }, }, }), - }); + }; + let subscription = setupAutoArchiveOnMerge(options, archiveDependencies); return { archives, reads, metrics: () => service.getMetrics(), isActive: () => active, + restartProducer: async () => { + subscription.unsubscribe(); + await service.dispose(); + service = createService(); + subscription = setupAutoArchiveOnMerge(options, archiveDependencies); + }, + advanceRemoteHead: () => { + remoteHeadSha = "a".repeat(40); + }, makeDirty: () => writeFileSync(path.join(cwd, "README.md"), "uncommitted work\n"), commitUnpushed: () => { writeFileSync(path.join(cwd, "README.md"), "unpushed work\n"); @@ -270,6 +307,24 @@ afterEach(async () => { }); describe("workspace auto-archive behavior", () => { + test("rechecks a checkout changed after the merged notification", async () => { + const workspace = await createUnobservedWorkspace(undefined, true); + await workspace.observe("open"); + await workspace.observe("merged"); + expect(workspace.reads).toContain("auto-archive-on-merge"); + expect(workspace.archives).toEqual([]); + expect(workspace.isActive()).toBe(true); + }); + test("keeps a merged badge and an active workspace when the remote head advanced", async () => { + const workspace = await createUnobservedWorkspace(); + await workspace.observe("open"); + workspace.advanceRemoteHead(); + const merged = await workspace.observe("merged"); + expect(merged.forge.pullRequest).toMatchObject({ state: "merged", headSha: "a".repeat(40) }); + expect(merged.git.headSha).not.toBe(merged.forge.pullRequest?.headSha); + expect(workspace.isActive()).toBe(true); + expect(workspace.archives).toEqual([]); + }); test("archives a clean owned workspace on an unobserved open-to-merged transition only once", async () => { let releaseArchive!: () => void; const archiveBarrier = new Promise((resolve) => { @@ -292,10 +347,11 @@ describe("workspace auto-archive behavior", () => { expect(workspace.metrics().workspaceListenerCount).toBe(0); expect(workspace.archives).toEqual(["workspace-under-test"]); - // Confirmation reads the completed snapshot cache; it does not shell out again. + // The archive decision rereads the checkout and forge after the transition. expect(workspace.reads).toEqual([ "workspace-journey", "workspace-journey", + "auto-archive-on-merge", "workspace-journey", ]); } finally { @@ -306,11 +362,14 @@ describe("workspace auto-archive behavior", () => { test("keeps an unobserved workspace active when its first snapshot is already merged", async () => { const workspace = await createUnobservedWorkspace(); - await workspace.observe("merged"); + workspace.advanceRemoteHead(); + expect((await workspace.observe("merged")).forge.pullRequest?.state).toBe("merged"); + await workspace.restartProducer(); + expect((await workspace.observe("merged")).forge.pullRequest?.state).toBe("merged"); expect(workspace.metrics().workspaceListenerCount).toBe(0); expect(workspace.archives).toEqual([]); - expect(workspace.reads).toEqual(["workspace-journey"]); + expect(workspace.reads).toEqual(["workspace-journey", "workspace-journey"]); }); test("keeps dirty work when an unobserved workspace PR merges", async () => { diff --git a/packages/server/src/server/auto-archive-on-merge/index.test.ts b/packages/server/src/server/auto-archive-on-merge/index.test.ts index 885d8250e8..8956407493 100644 --- a/packages/server/src/server/auto-archive-on-merge/index.test.ts +++ b/packages/server/src/server/auto-archive-on-merge/index.test.ts @@ -90,6 +90,11 @@ test("fans one fresh observation out to every workspace attached to its exact cw await finished; expect(getSnapshot).toHaveBeenCalledTimes(1); + expect(getSnapshot).toHaveBeenCalledWith("/repo/worktree/.", { + force: true, + afterInFlight: true, + reason: "auto-archive-on-merge", + }); expect(calls).toEqual([ { workspaceId: "workspace-a", pullRequest: freshSnapshot.forge.pullRequest }, { workspaceId: "workspace-b", pullRequest: freshSnapshot.forge.pullRequest }, diff --git a/packages/server/src/server/auto-archive-on-merge/index.ts b/packages/server/src/server/auto-archive-on-merge/index.ts index be950bf367..5e2cc5c269 100644 --- a/packages/server/src/server/auto-archive-on-merge/index.ts +++ b/packages/server/src/server/auto-archive-on-merge/index.ts @@ -63,6 +63,8 @@ export function setupAutoArchiveOnMerge( let freshSnapshot: WorkspaceGitRuntimeSnapshot | null; try { freshSnapshot = await options.workspaceGitService.getSnapshot(snapshot.cwd, { + force: true, + afterInFlight: true, reason: "auto-archive-on-merge", }); } catch (error) { diff --git a/packages/server/src/server/workspace-git-service.primitive.test.ts b/packages/server/src/server/workspace-git-service.primitive.test.ts index 1b43026c7d..dedeb2672d 100644 --- a/packages/server/src/server/workspace-git-service.primitive.test.ts +++ b/packages/server/src/server/workspace-git-service.primitive.test.ts @@ -150,6 +150,8 @@ function currentPullRequestJson(overrides: Record = {}): string isDraft: false, baseRefName: "main", headRefName: "feature", + headRepositoryOwner: { login: "acme" }, + headRepository: { name: "repo" }, mergedAt: null, statusCheckRollup: [], reviewDecision: "REVIEW_REQUIRED", @@ -802,6 +804,28 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => { service.dispose(); }); + test("an after-in-flight read refreshes again after an earlier forced read", async () => { + const forcedRefresh = createDeferred(); + const getCheckoutStatus = vi + .fn<() => Promise>() + .mockImplementationOnce(async () => createCheckoutStatus(REPO_CWD)) + .mockImplementationOnce(async () => forcedRefresh.promise) + .mockImplementationOnce(async () => createCheckoutStatus(REPO_CWD)); + const service = createService({ getCheckoutStatus }); + await service.getSnapshot(REPO_CWD); + const first = service.getSnapshot(REPO_CWD, { force: true, reason: "earlier" }); + await flushPromises(); + const second = service.getSnapshot(REPO_CWD, { + force: true, + afterInFlight: true, + reason: "archive", + }); + forcedRefresh.resolve(createCheckoutStatus(REPO_CWD)); + await Promise.all([first, second]); + expect(getCheckoutStatus).toHaveBeenCalledTimes(3); + service.dispose(); + }); + test("a forced GitHub-inclusive call during an in-flight forced git refresh queues a GitHub refresh", async () => { const forcedGitRefresh = createDeferred(); const getCheckoutStatus = vi @@ -852,7 +876,7 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => { expect(getPullRequestStatus).toHaveBeenCalledWith( REPO_CWD, expect.anything(), - { force: true, reason: "merge-pr-validation" }, + { force: true, reason: "merge-pr-validation", previousPullRequest: null }, expect.anything(), ); @@ -942,7 +966,7 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => { expect(getPullRequestStatus).toHaveBeenCalledWith( REPO_CWD, expect.anything(), - { force: false, reason: "initial" }, + { force: false, reason: "initial", previousPullRequest: null }, expect.anything(), ); @@ -1274,6 +1298,14 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => { const service = createService({ getCheckoutSnapshotFacts, getCheckoutStatus, + getPullRequestStatus: vi.fn(async () => ({ + ...createPullRequestStatusResult(), + status: { + ...createPullRequestStatusResult().status!, + number: 2, + url: "https://forge-self-heal.test/acme/repo/pulls/2", + }, + })), }); const listener = vi.fn(); @@ -1288,6 +1320,11 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => { cwd: REPO_CWD, headRef: "feature", headSha: "1111111111111111111111111111111111111111", + previousPullRequest: { + number: 2, + url: "https://forge-self-heal.test/acme/repo/pulls/2", + state: "open", + }, reason: "self-heal-forge-pr-status", }); expect(listener).toHaveBeenCalledWith( @@ -1308,6 +1345,176 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => { } }); + test.each(["poll then refresh", "refresh then poll"] as const)( + "uses the current workspace association across %s", + async (direction) => { + const merged = createCurrentPullRequestStatus({ + number: 1, + url: "https://forge-owner.test/acme/repo/pulls/1", + state: "merged", + isMerged: true, + }); + const open = createCurrentPullRequestStatus({ + number: 2, + url: "https://forge-owner.test/acme/repo/pulls/2", + state: "open", + }); + let refreshFindsOpen = false; + const forge = { + ...createGitHubServiceStub(), + retainCurrentPullRequestStatusPoll: undefined, + getCurrentPullRequestStatus: vi.fn(async (input) => + direction === "poll then refresh" || input.previousPullRequest?.number === 2 + ? open + : merged, + ), + }; + const unregister = defaultForgeRegistry.register("forge-owner-test", { + createService: () => forge, + matchesHost: (host) => host === "forge-owner.test", + }); + const getPullRequestStatus = vi.fn(async (_cwd, _forge, options) => ({ + ...createPullRequestStatusResult(), + status: refreshFindsOpen || options.previousPullRequest?.number === 2 ? open : merged, + })); + const service = createService({ + getCheckoutSnapshotFacts: vi.fn(async (cwd) => + createCheckoutFacts(cwd, { + currentBranch: "feature", + remoteUrl: "https://forge-owner.test/acme/repo.git", + pullRequestLookupTarget: { headRef: "feature" }, + }), + ), + getCheckoutStatus: vi.fn(async (cwd) => + createCheckoutStatus(cwd, { + currentBranch: "feature", + remoteUrl: "https://forge-owner.test/acme/repo.git", + }), + ), + getPullRequestStatus, + }); + try { + expect((await service.getSnapshot(REPO_CWD)).forge.pullRequest?.number).toBe(1); + const subscription = service.registerWorkspace({ cwd: REPO_CWD }, vi.fn()); + await flushPromises(); + if (direction === "poll then refresh") { + await vi.advanceTimersByTimeAsync(120_000); + await flushPromises(); + expect((await service.getSnapshot(REPO_CWD)).forge.pullRequest?.number).toBe(2); + expect( + (await service.getSnapshot(REPO_CWD, { force: true, reason: "test" })).forge.pullRequest + ?.number, + ).toBe(2); + } else { + refreshFindsOpen = true; + expect( + (await service.getSnapshot(REPO_CWD, { force: true, reason: "test" })).forge.pullRequest + ?.number, + ).toBe(2); + refreshFindsOpen = false; + await vi.advanceTimersByTimeAsync(120_000); + await flushPromises(); + expect((await service.getSnapshot(REPO_CWD)).forge.pullRequest?.number).toBe(2); + } + subscription.unsubscribe(); + } finally { + service.dispose(); + unregister(); + } + }, + ); + + test.each(["older poll", "older refresh"] as const)( + "does not publish an %s after a newer observation", + async (older) => { + const merged = createCurrentPullRequestStatus({ + number: 1, + url: "https://forge-race.test/acme/repo/pulls/1", + state: "merged", + isMerged: true, + }); + const open = createCurrentPullRequestStatus({ + number: 2, + url: "https://forge-race.test/acme/repo/pulls/2", + state: "open", + }); + const delayedPoll = createDeferred(); + const delayedRefresh = createDeferred(); + let delayRefresh = false; + const forge = { + ...createGitHubServiceStub(), + retainCurrentPullRequestStatusPoll: undefined, + getCurrentPullRequestStatus: vi.fn(async () => + older === "older poll" ? delayedPoll.promise : open, + ), + }; + const unregister = defaultForgeRegistry.register("forge-race-test", { + createService: () => forge, + matchesHost: (host) => host === "forge-race.test", + }); + let first = true; + const getPullRequestStatus = vi.fn(async (_cwd, _forge, options) => { + if (first) { + first = false; + return { ...createPullRequestStatusResult(), status: merged }; + } + if (delayRefresh) { + delayRefresh = false; + return delayedRefresh.promise; + } + return { + ...createPullRequestStatusResult(), + status: + older === "older poll" || options.previousPullRequest?.number === 2 ? open : merged, + }; + }); + const service = createService({ + getCheckoutSnapshotFacts: vi.fn(async (cwd) => + createCheckoutFacts(cwd, { + currentBranch: "feature", + remoteUrl: "https://forge-race.test/acme/repo.git", + pullRequestLookupTarget: { headRef: "feature" }, + }), + ), + getCheckoutStatus: vi.fn(async (cwd) => + createCheckoutStatus(cwd, { + currentBranch: "feature", + remoteUrl: "https://forge-race.test/acme/repo.git", + }), + ), + getPullRequestStatus, + }); + try { + await service.getSnapshot(REPO_CWD); + const subscription = service.registerWorkspace({ cwd: REPO_CWD }, vi.fn()); + await flushPromises(); + if (older === "older poll") { + await vi.advanceTimersByTimeAsync(120_000); + await flushPromises(); + expect( + (await service.getSnapshot(REPO_CWD, { force: true, reason: "test" })).forge.pullRequest + ?.number, + ).toBe(2); + delayedPoll.resolve(merged); + await flushPromises(); + } else { + delayRefresh = true; + const refresh = service.getSnapshot(REPO_CWD, { force: true, reason: "test" }); + await flushPromises(); + await vi.advanceTimersByTimeAsync(120_000); + await flushPromises(); + delayedRefresh.resolve({ ...createPullRequestStatusResult(), status: merged }); + expect((await refresh).forge.pullRequest?.number).toBe(2); + } + expect((await service.getSnapshot(REPO_CWD)).forge.pullRequest?.number).toBe(2); + subscription.unsubscribe(); + } finally { + service.dispose(); + unregister(); + } + }, + ); + test("generic forge self-heal uses the fast poll window while checks are pending", async () => { const forge = { ...createGitHubServiceStub(), diff --git a/packages/server/src/server/workspace-git-service.ts b/packages/server/src/server/workspace-git-service.ts index 26a7f16419..8452cac873 100644 --- a/packages/server/src/server/workspace-git-service.ts +++ b/packages/server/src/server/workspace-git-service.ts @@ -23,6 +23,7 @@ import { getPullRequestStatus, forgeAuthStateFromError, hasOriginRemote, + isPullRequestTargetOnForge, listBranchSuggestions, resolveRepositoryDefaultBranch, resolveBranchCheckout, @@ -151,6 +152,8 @@ export const getWorkspaceGitSelfHealPhaseMs = getWorkspaceGitObservationReensure export interface WorkspaceGitRuntimeSnapshot { cwd: string; git: { + headSha?: string; + upstreamKnown?: boolean; isGit: boolean; repoRoot: string | null; mainRepoRoot: string | null; @@ -176,6 +179,7 @@ export interface WorkspaceGitRuntimeSnapshot { */ forge?: string; pullRequest: { + headSha?: string; number?: number; repoOwner?: string; repoName?: string; @@ -321,6 +325,7 @@ export type WorkspaceGitSnapshotOptions = force: true; includeForge?: boolean; reason: string; + afterInFlight?: boolean; }; interface WorkspaceGitRefreshRequest { @@ -550,6 +555,10 @@ interface WorkspaceForgePrStatusPollTarget { headRef: string; headSha?: string; headRepositoryOwner?: string; + headRepositoryPath?: string; + headRepositoryHost?: string; + headRepositoryEndpoint?: import("../services/forge-service.js").HeadRepositoryEndpoint; + changeRequestNumber?: number; } function buildDefaultWorkspaceGitServiceDeps( @@ -2970,6 +2979,15 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { ...(pollTarget.headRepositoryOwner ? { headRepositoryOwner: pollTarget.headRepositoryOwner } : {}), + ...(pollTarget.headRepositoryPath + ? { headRepositoryPath: pollTarget.headRepositoryPath } + : {}), + ...(pollTarget.headRepositoryEndpoint + ? { headRepositoryEndpoint: pollTarget.headRepositoryEndpoint } + : {}), + ...(pollTarget.changeRequestNumber + ? { changeRequestNumber: pollTarget.changeRequestNumber } + : {}), onStatus: (status) => { if (!this.isActiveObservedWorkspaceTarget(target)) { return; @@ -3020,8 +3038,6 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { }): { unsubscribe: () => void } { let closed = false; let timer: NodeJS.Timeout | null = null; - let latestStatus: WorkspaceGitRuntimeSnapshot["forge"]["pullRequest"] = - target.latestForge?.pullRequest ?? null; let consecutiveErrors = 0; const schedule = (delayMs: number) => { @@ -3038,6 +3054,8 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { if (closed || !this.isActiveObservedWorkspaceTarget(target)) { return; } + const previousForge = target.latestForge; + const previousStatus = previousForge?.pullRequest; try { const status = await service.getCurrentPullRequestStatus({ cwd: target.cwd, @@ -3046,10 +3064,31 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { ...(pollTarget.headRepositoryOwner ? { headRepositoryOwner: pollTarget.headRepositoryOwner } : {}), + ...(pollTarget.headRepositoryPath + ? { headRepositoryPath: pollTarget.headRepositoryPath } + : {}), + ...(pollTarget.headRepositoryEndpoint + ? { headRepositoryEndpoint: pollTarget.headRepositoryEndpoint } + : {}), + ...(pollTarget.changeRequestNumber + ? { changeRequestNumber: pollTarget.changeRequestNumber } + : {}), + ...(previousStatus?.state === "open" + ? { + previousPullRequest: { + number: previousStatus.number, + url: previousStatus.url, + state: previousStatus.state, + }, + } + : {}), reason: "self-heal-forge-pr-status", }); - if (!closed && this.isActiveObservedWorkspaceTarget(target)) { - latestStatus = status; + if ( + !closed && + this.isActiveObservedWorkspaceTarget(target) && + target.latestForge === previousForge + ) { consecutiveErrors = 0; this.rememberForgePrStatusSnapshot(target, buildForgeSnapshotFromStatus(status, forge)); } @@ -3067,7 +3106,12 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { "Failed to run forge PR status self-heal refresh", ); } finally { - schedule(computeGenericForgeNextInterval(latestStatus, consecutiveErrors)); + schedule( + computeGenericForgeNextInterval( + target.latestForge?.pullRequest ?? null, + consecutiveErrors, + ), + ); } }; @@ -3075,7 +3119,12 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { // changes. Revalidate that new identity immediately instead of leaving the // PR panel empty for the full stable polling interval. schedule( - pollImmediately ? 0 : computeGenericForgeNextInterval(latestStatus, consecutiveErrors), + pollImmediately + ? 0 + : computeGenericForgeNextInterval( + target.latestForge?.pullRequest ?? null, + consecutiveErrors, + ), ); return { unsubscribe: () => { @@ -3101,10 +3150,12 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { ? target.latestFacts.pullRequestLookupTarget : null; if (target.latestFacts?.isGit && target.latestFacts.paseoWorktree.isPaseoOwnedWorktree) { - return lookupTarget; + return lookupTarget && isPullRequestTargetOnForge(git.remoteUrl, lookupTarget) + ? lookupTarget + : null; } if (lookupTarget) { - return lookupTarget; + return isPullRequestTargetOnForge(git.remoteUrl, lookupTarget) ? lookupTarget : null; } return { headRef: git.currentBranch }; @@ -3242,7 +3293,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { refreshWorktree: true, includeForge: options?.includeForge ?? true, reason: options?.reason ?? defaultReason, - queueIfBusy: false, + queueIfBusy: options?.force === true && options.afterInFlight === true, movedRemoteRefs: new Set(), }; } @@ -3459,12 +3510,17 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { runGitCommand: runRefreshGitCommand, }, ); - target.latestFacts = { ...facts, upstreamStatus: derived.upstreamStatus }; + target.latestFacts = { + ...facts, + upstreamStatus: derived.upstreamStatus, + upstreamKnown: derived.upstreamStatus !== null, + }; target.latestGit = { ...latestGit, aheadBehind: derived.aheadBehind, diffStat: derived.diffStat, upstreamRef: derived.upstreamStatus?.ref ?? null, + upstreamKnown: derived.upstreamStatus !== null, aheadOfOrigin: derived.upstreamStatus?.aheadBehind.ahead ?? null, behindOfOrigin: derived.upstreamStatus?.aheadBehind.behind ?? null, }; @@ -3539,6 +3595,8 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { target.latestGit = { isGit: true, + headSha: facts.isGit ? facts.pullRequestLookupTarget?.headSha : undefined, + upstreamKnown: facts.isGit ? facts.upstreamKnown : false, repoRoot: checkoutStatus.repoRoot, mainRepoRoot: checkoutStatus.mainRepoRoot, currentBranch: checkoutStatus.currentBranch, @@ -3588,20 +3646,28 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { forgeService.invalidate({ cwd: target.cwd }); } - const forgeSnapshot = await loadForgeSnapshot({ - cwd: target.cwd, - forgeService, - now: this.deps.now(), - deps: this.deps, - force: forceForge, - reason: request.reason, - facts, - runGitCommand: runRefreshGitCommand, - }); - // Carry the resolved forge (probe-aware) so the wire projection labels - // self-managed GitLab hosts correctly instead of falling back to "github". - target.latestForge = { ...forgeSnapshot, forge: resolution.forge }; - target.latestForgeLoadedAtMs = this.deps.now().getTime(); + for (let attempt = 0; attempt < 2; attempt += 1) { + const previousForge = target.latestForge; + const forgeSnapshot = await loadForgeSnapshot({ + cwd: target.cwd, + forgeService, + now: this.deps.now(), + deps: this.deps, + force: forceForge, + reason: request.reason, + facts, + runGitCommand: runRefreshGitCommand, + previousForge, + }); + if (target.latestForge !== previousForge) { + if (forceForge) continue; + return; + } + target.latestForge = { ...forgeSnapshot, forge: resolution.forge }; + target.latestForgeLoadedAtMs = this.deps.now().getTime(); + return; + } + throw new Error("Forge status changed during the forced workspace refresh"); } private combineSnapshot(target: WorkspaceGitTarget): WorkspaceGitRuntimeSnapshot { @@ -3971,6 +4037,7 @@ async function loadForgeSnapshot(options: { reason?: string; facts?: CheckoutSnapshotFacts; runGitCommand: RunGitCommand; + previousForge: WorkspaceGitRuntimeSnapshot["forge"] | null; }): Promise { const forgeService = options.forgeService; if (!forgeService) { @@ -3996,6 +4063,7 @@ async function loadForgeSnapshot(options: { { force: options.force, reason: options.reason, + previousPullRequest: options.previousForge?.pullRequest ?? null, }, { facts: options.facts, runGitCommand: options.runGitCommand }, ); @@ -4003,7 +4071,7 @@ async function loadForgeSnapshot(options: { } catch (error) { // The auth probe succeeded, so a failure here is a command error, not an // auth problem — surface it as an error while keeping features enabled. - return buildForgeSnapshot("authenticated", null, { + return buildForgeSnapshot("authenticated", options.previousForge?.pullRequest ?? null, { message: error instanceof Error ? error.message : String(error), }); } @@ -4127,6 +4195,10 @@ function buildWorkspaceForgePrStatusPollKey({ target.headRef, target.headSha ?? null, target.headRepositoryOwner ?? null, + target.headRepositoryPath ?? null, + target.headRepositoryHost ?? null, + target.headRepositoryEndpoint ?? null, + target.changeRequestNumber ?? null, ]); } diff --git a/packages/server/src/services/forge-service.ts b/packages/server/src/services/forge-service.ts index 8499cc8756..0a6af79733 100644 --- a/packages/server/src/services/forge-service.ts +++ b/packages/server/src/services/forge-service.ts @@ -1,4 +1,29 @@ import type { ForgeSearchKind } from "@getpaseo/protocol/messages"; +import { parseGitRemoteLocation, type GitRemoteLocation } from "@getpaseo/protocol/git-remote"; + +/** A checkout endpoint that must be proven against the candidate's native clone URL. */ +export type HeadRepositoryEndpoint = GitRemoteLocation; + +/** The bounded discovery window could not establish a result. */ +export class ForgeDiscoveryIncompleteError extends Error {} + +export function matchesHeadRepositoryEndpoint( + endpoint: HeadRepositoryEndpoint | undefined, + urls: (string | null | undefined)[], +): boolean { + if (!endpoint) return true; + return urls.some((url) => { + const candidate = url ? parseGitRemoteLocation(url) : null; + if (!candidate) return false; + const transport = (value: GitRemoteLocation["transport"]) => (value === "scp" ? "ssh" : value); + return ( + transport(candidate.transport) === transport(endpoint.transport) && + candidate.host === endpoint.host && + candidate.port === endpoint.port && + candidate.path.toLowerCase() === endpoint.path.toLowerCase() + ); + }); +} export type ForgeSearchRequestKind = ForgeSearchKind | "github-issue" | "github-pr" | "pr"; @@ -114,6 +139,8 @@ export type ForgeAuthState = export type ForgeSpecificStatusFacts = { forge: string } & Record; export interface CurrentPullRequestStatus { + /** Commit recorded as the change request's head by the forge. */ + headSha?: string; number?: number; repoOwner?: string; repoName?: string; @@ -472,6 +499,11 @@ export interface ForgeService { headRef: string; headSha?: string; headRepositoryOwner?: string; + headRepositoryPath?: string; + headRepositoryHost?: string; + headRepositoryEndpoint?: HeadRepositoryEndpoint; + previousPullRequest?: { number?: number; url: string; state: string } | null; + changeRequestNumber?: number; } & ForgeReadOptions, ): Promise; getPullRequestTimeline(options: GetPullRequestTimelineOptions): Promise; @@ -513,6 +545,9 @@ export interface ForgeService { headRef: string; headSha?: string; headRepositoryOwner?: string; + headRepositoryPath?: string; + headRepositoryEndpoint?: HeadRepositoryEndpoint; + changeRequestNumber?: number; onStatus?: (status: CurrentPullRequestStatus | null) => void; onError?: (error: unknown) => void; }): { unsubscribe: () => void }; diff --git a/packages/server/src/services/gitea-service.test.ts b/packages/server/src/services/gitea-service.test.ts index 9e3a3ecc62..06cf32098d 100644 --- a/packages/server/src/services/gitea-service.test.ts +++ b/packages/server/src/services/gitea-service.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import type { PullRequestCommandStatus, PullRequestTimelineItem } from "./forge-service.js"; +import { ForgeDiscoveryIncompleteError } from "./forge-service.js"; import { type CreateGiteaServiceOptions, createGiteaService, @@ -19,9 +20,38 @@ function ok(stdout: string): TeaCommandResult { function makeService(responder: Responder, overrides: Partial = {}) { const calls: string[][] = []; + const listed = new Map(); + const listedDetail = (args: string[]): TeaCommandResult | null => { + const match = args[0] === "api" ? args[1]?.match(/\/pulls\/(\d+)$/) : null; + const item = match ? listed.get(match[1]) : undefined; + if (!item) return null; + const detail = currentPullRequestApi({ + number: Number(item.index), + state: "open", + headRef: item.head, + headSha: STATUS_PR_VIEW.headSha, + }); + return ok(JSON.stringify({ ...detail, html_url: item.url, title: item.title })); + }; const runner: TeaCommandRunner = async (args) => { calls.push(args); - return responder(args); + try { + const result = await responder(args); + if (args[0] === "pr" && args[1] === "list") { + const items = JSON.parse(result.stdout); + if (Array.isArray(items)) for (const item of items) listed.set(item.index, item); + } + if (args[0] === "api" && /\/pulls\/\d+$/.test(args[1] ?? "")) { + const data = JSON.parse(result.stdout); + if (!data?.number) return listedDetail(args) ?? result; + } + return result; + } catch (error) { + if (error instanceof Error && error.message.startsWith("unexpected call:")) { + return listedDetail(args) ?? Promise.reject(error); + } + throw error; + } }; const service = createGiteaService({ runner, @@ -94,7 +124,7 @@ function currentPullRequestApi(input: { head: { ref: input.headRef, sha: input.headSha, - repo: { id: 1, owner: { login: "example-user" } }, + repo: { id: 1, full_name: "example-user/sample-repo", owner: { login: "example-user" } }, }, base: { ref: "main", @@ -485,8 +515,9 @@ describe("createGiteaService", () => { expect(calls[0]).toContain("--fields"); expect(calls[0]).toContain("-o"); expect(calls[0]).toContain("json"); - expect(calls[1]).toEqual(["pr", "5", "-o", "json"]); - expect(calls[2]).toEqual([ + expect(calls[1]).toEqual(["api", "repos/example-user/sample-repo/pulls/5"]); + expect(calls[2]).toEqual(["pr", "5", "-o", "json"]); + expect(calls[3]).toEqual([ "api", "repos/example-user/sample-repo/commits/3333333333333333333333333333333333333333/status", ]); @@ -1627,6 +1658,7 @@ describe("createGiteaService", () => { "--page", "1", ], + ["api", "repos/example-user/sample-repo/pulls/5"], ["pr", "5", "-o", "json"], ["api", `repos/example-user/sample-repo/commits/${STATUS_PR_VIEW.headSha}/status`], ]); @@ -2055,6 +2087,7 @@ describe("createGiteaService", () => { cwd: "/repo", headRef: "feat/recently-closed", headSha, + headRepositoryPath: "example-user/sample-repo", }); expect(status).toMatchObject({ number: 8, headRefName: "feat/recently-closed" }); @@ -2064,7 +2097,7 @@ describe("createGiteaService", () => { ]); }); - it("does not attach a stale Gitea-family PR after a same-name branch advances", async () => { + it("shows a merged PR when the checkout head differs", async () => { const stale = currentPullRequestApi({ number: 8, state: "closed", @@ -2086,9 +2119,338 @@ describe("createGiteaService", () => { headRef: "dev", headSha: "2222222222222222222222222222222222222222", }), + ).resolves.toMatchObject({ number: 8, state: "merged", headSha: stale.head.sha }); + }); + + it("does not scan beyond the bounded terminal window", async () => { + const unrelated = Array.from({ length: 50 }, (_, i) => + currentPullRequestApi({ + number: i + 100, + state: "closed", + headRef: `other-${i}`, + headSha: "1".repeat(40), + }), + ); + const wanted = currentPullRequestApi({ + number: 8, + state: "closed", + headRef: "topic", + headSha: "2".repeat(40), + merged: true, + }); + const { service, calls } = makeService((args) => { + if (args[0] === "pr" && args[1] === "list") return ok("[]"); + if (args[0] === "api" && args[1].includes("/pulls?state=all")) + return ok(JSON.stringify(args[1].includes("page=2") ? [wanted] : unrelated)); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + await expect( + service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "topic" }), + ).rejects.toBeInstanceOf(ForgeDiscoveryIncompleteError); + expect(calls.filter((args) => args[1]?.includes("/pulls?state=all"))).toHaveLength(1); + }); + + it("does not associate a different repository owned by the same account", async () => { + const foreign = currentPullRequestApi({ + number: 8, + state: "closed", + headRef: "dev", + headSha: "1".repeat(40), + merged: true, + }); + foreign.head.repo.id = 2; + foreign.head.repo.full_name = "example-user/other"; + const { service } = makeService((args) => { + if (args[0] === "pr" && args[1] === "list") return ok("[]"); + if (args[0] === "api" && args[1].includes("/pulls?state=all")) + return ok(JSON.stringify([foreign])); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + await expect( + service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "dev" }), ).resolves.toBeNull(); }); + it.each(["merged", "open"] as const)( + "finds a recent %s candidate after 200 unrelated open requests", + async (state) => { + const unrelated = Array.from({ length: 50 }, (_, i) => ({ + ...OPEN_PR, + index: String(i + 100), + head: `other-${i}`, + })); + const wanted = currentPullRequestApi({ + number: 9, + state: state === "merged" ? "closed" : "open", + headRef: "topic", + headSha: "1".repeat(40), + merged: state === "merged", + }); + const newerMerged = currentPullRequestApi({ + number: 10, + state: "closed", + headRef: "topic", + headSha: "2".repeat(40), + merged: true, + }); + const { service, calls } = makeService((args) => { + if (args[0] === "pr" && args[1] === "list") return ok(JSON.stringify(unrelated)); + if (args[0] === "api" && args[1].includes("pulls?state=all")) + return ok( + JSON.stringify( + state === "open" + ? [ + { ...newerMerged, updated_at: "2026-10-01T00:00:00Z" }, + { ...wanted, updated_at: "2026-01-01T00:00:00Z" }, + ] + : [wanted], + ), + ); + if (args[0] === "pr" && args[1] === "9") return ok(JSON.stringify(STATUS_PR_VIEW)); + if (args[0] === "api" && args[1].includes("/commits/")) + return ok(JSON.stringify({ state: "", statuses: [] })); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + expect( + (await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "topic" }))?.number, + ).toBe(9); + expect(calls.filter((args) => args[0] === "pr" && args[1] === "list")).toHaveLength(4); + expect(calls.filter((args) => args[1]?.includes("pulls?state=all"))).toHaveLength(1); + }, + ); + + it("revalidates a known open PR before replacing it with a crowded-window terminal candidate", async () => { + const open = currentPullRequestApi({ + number: 2, + state: "open", + headRef: "topic", + headSha: "1".repeat(40), + }); + const oldMerged = currentPullRequestApi({ + number: 1, + state: "closed", + headRef: "topic", + headSha: "2".repeat(40), + merged: true, + }); + const unrelated = Array.from({ length: 50 }, (_, i) => ({ + ...OPEN_PR, + index: String(i + 100), + head: `other-${i}`, + })); + const fullRecentItems = Array.from({ length: 50 }, (_, i) => ({ + ...oldMerged, + number: i + 100, + head: { ...oldMerged.head, ref: `other-${i}` }, + })); + let crowded = false; + let fullRecent = false; + let prior = open; + const { service, calls } = makeService((args) => { + if (args[0] === "pr" && args[1] === "list") + return ok( + JSON.stringify( + crowded ? unrelated : [{ ...OPEN_PR, index: "2", url: open.html_url, head: "topic" }], + ), + ); + if (args[0] === "api" && args[1].endsWith("/pulls/2")) return ok(JSON.stringify(prior)); + if (args[0] === "api" && args[1].includes("pulls?state=all")) + return ok(JSON.stringify(fullRecent ? fullRecentItems : [oldMerged])); + if (args[0] === "pr" && args[1] === "2") return ok(JSON.stringify(STATUS_PR_VIEW)); + if (args[0] === "api" && args[1].includes("/commits/")) + return ok(JSON.stringify({ state: "", statuses: [] })); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + const input = { cwd: "/repo", headRef: "topic" }; + const observed = await service.getCurrentPullRequestStatus(input); + expect(observed).toMatchObject({ number: 2, state: "open" }); + crowded = true; + expect( + await service.getCurrentPullRequestStatus({ ...input, previousPullRequest: observed! }), + ).toMatchObject({ number: 2, state: "open" }); + prior = { ...open, state: "closed", merged: true }; + expect( + await service.getCurrentPullRequestStatus({ ...input, previousPullRequest: observed! }), + ).toMatchObject({ number: 2, state: "merged" }); + prior = { ...open, state: "closed", merged: false }; + expect( + await service.getCurrentPullRequestStatus({ ...input, previousPullRequest: observed! }), + ).toMatchObject({ number: 2, state: "closed" }); + fullRecent = true; + expect( + await service.getCurrentPullRequestStatus({ ...input, previousPullRequest: observed! }), + ).toMatchObject({ number: 2, state: "closed" }); + expect(calls.filter((args) => args[1]?.endsWith("/pulls/2"))).toHaveLength(5); + }); + + it("requires candidate clone URL proof for mixed ports and isolates cached identity", async () => { + const pr = currentPullRequestApi({ + number: 5, + state: "open", + headRef: "topic", + headSha: "1".repeat(40), + }); + let cloneUrl: string | undefined = "ssh://git@forge.example:2222/example-user/sample-repo.git"; + const { service, calls } = makeService((args) => { + if (args[0] === "pr" && args[1] === "list") + return ok(JSON.stringify([{ ...OPEN_PR, head: "topic" }])); + if (args[0] === "api" && args[1].endsWith("/pulls/5")) + return ok( + JSON.stringify({ + ...pr, + head: { ...pr.head, repo: { ...pr.head.repo, ssh_url: cloneUrl } }, + }), + ); + if (args[0] === "api" && args[1].includes("pulls?state=all")) return ok("[]"); + if (args[0] === "pr" && args[1] === "5") return ok(JSON.stringify(STATUS_PR_VIEW)); + if (args[0] === "api" && args[1].includes("/commits/")) + return ok(JSON.stringify({ state: "", statuses: [] })); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + const endpoint = { + transport: "ssh" as const, + host: "forge.example", + port: "2222", + path: "example-user/sample-repo", + }; + const input = { cwd: "/repo", headRef: "topic", headRepositoryEndpoint: endpoint }; + expect((await service.getCurrentPullRequestStatus(input))?.number).toBe(5); + expect( + await service.getCurrentPullRequestStatus({ + ...input, + headRepositoryEndpoint: { ...endpoint, port: "2223" }, + }), + ).toBeNull(); + expect( + (await service.getCurrentPullRequestStatus({ ...input, force: true, reason: "archive" })) + ?.number, + ).toBe(5); + cloneUrl = undefined; + expect( + await service.getCurrentPullRequestStatus({ ...input, force: true, reason: "archive" }), + ).toBeNull(); + expect(calls.filter((args) => args[1]?.endsWith("/pulls/5"))).toHaveLength(4); + }); + + it("ignores an open PR from another repository of the same owner", async () => { + const foreign = currentPullRequestApi({ + number: 5, + state: "open", + headRef: "dev", + headSha: "1".repeat(40), + }); + foreign.head.repo.id = 2; + foreign.head.repo.full_name = "example-user/other"; + const { service } = makeService((args) => { + if (args[0] === "pr" && args[1] === "list") + return ok(JSON.stringify([{ ...OPEN_PR, head: "dev" }])); + if (args[0] === "api" && args[1].endsWith("/pulls/5")) return ok(JSON.stringify(foreign)); + if (args[0] === "api" && args[1].includes("/pulls?state=all")) return ok("[]"); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + await expect( + service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "dev", + headRepositoryPath: "example-user/sample-repo", + }), + ).resolves.toBeNull(); + }); + + it("uses a checkout hint to discover a renamed fork before finding the current PR", async () => { + const forkPr = currentPullRequestApi({ + number: 42, + state: "open", + headRef: "topic", + headSha: "1".repeat(40), + }); + forkPr.head.repo = { id: 2, full_name: "contributor/renamed", owner: { login: "contributor" } }; + const { service } = makeService((args) => { + if (args[0] === "api" && args[1].endsWith("/pulls/41")) + return ok(JSON.stringify({ ...forkPr, number: 41 })); + if (args[0] === "pr" && args[1] === "list") return ok("[]"); + if (args[0] === "api" && args[1].includes("/pulls?state=all")) + return ok(JSON.stringify([forkPr])); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "topic", + headRepositoryOwner: "contributor", + changeRequestNumber: 41, + }); + expect(status?.number).toBe(42); + }); + + it("prefers an open PR before looking for terminal requests", async () => { + const open = { ...OPEN_PR, head: "topic" }; + const { service, calls } = makeService((args) => { + if (args[0] === "pr" && args[1] === "list") return ok(JSON.stringify([open])); + if (args[0] === "pr" && args[1] === "5") return ok(JSON.stringify(STATUS_PR_VIEW)); + if (args[0] === "api" && args[1].includes("/commits/")) + return ok(JSON.stringify({ state: "", statuses: [] })); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + expect( + await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "topic" }), + ).toMatchObject({ number: 5, state: "open" }); + expect(calls.some((args) => args[1]?.includes("pulls?state=all"))).toBe(false); + }); + + it("bounds repository identity reads when a branch is crowded", async () => { + const candidates = Array.from({ length: 4 }, (_, i) => ({ + ...OPEN_PR, + index: String(i + 20), + head: "topic", + })); + const { service, calls } = makeService((args) => { + if (args[0] === "pr" && args[1] === "list") return ok(JSON.stringify(candidates)); + if (args[0] === "api" && args[1].includes("pulls?state=all")) return ok("[]"); + if (args[0] === "api" && /\/pulls\/\d+$/.test(args[1])) { + const pr = currentPullRequestApi({ + number: Number(args[1].split("/").at(-1)), + state: "open", + headRef: "topic", + headSha: "1".repeat(40), + }); + pr.head.repo.full_name = "example-user/other"; + return ok(JSON.stringify(pr)); + } + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + await expect( + service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "topic" }), + ).rejects.toBeInstanceOf(ForgeDiscoveryIncompleteError); + expect(calls.filter((args) => args[0] === "api" && /\/pulls\/\d+$/.test(args[1]))).toHaveLength( + 3, + ); + }); + + it("reuses the head repository identity on subsequent open PR polls", async () => { + const open = { ...OPEN_PR, head: "topic" }; + const { service, calls } = makeService((args) => { + if (args[0] === "pr" && args[1] === "list") return ok(JSON.stringify([open])); + if (args[0] === "pr" && args[1] === "5") return ok(JSON.stringify(STATUS_PR_VIEW)); + if (args[0] === "api" && args[1].includes("/commits/")) + return ok(JSON.stringify({ state: "", statuses: [] })); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + const input = { cwd: "/repo", headRef: "topic" }; + expect((await service.getCurrentPullRequestStatus(input))?.headSha).toBe( + STATUS_PR_VIEW.headSha, + ); + expect((await service.getCurrentPullRequestStatus(input))?.headSha).toBe( + STATUS_PR_VIEW.headSha, + ); + expect( + calls.filter((args) => args[1] === "repos/example-user/sample-repo/pulls/5"), + ).toHaveLength(1); + await service.getCurrentPullRequestStatus({ ...input, force: true, reason: "archive" }); + expect( + calls.filter((args) => args[1] === "repos/example-user/sample-repo/pulls/5"), + ).toHaveLength(2); + }); + it("returns null when no PR matches the current branch", async () => { const { service, calls } = makeService((args) => { if (args[0] === "pr" && args[1] === "list") return ok(JSON.stringify([])); @@ -2110,11 +2472,7 @@ describe("createGiteaService", () => { ]); }); - it("resolves an open cross-fork PR by head sha despite the owner gate", async () => { - // The PR lives in the base repo but its head is on a fork - // (contributor:feat/fork-change), so the checkout-owner gate excludes it. - // The head sha uniquely identifies it, so resolution must accept it — this - // mirrors GitLab, which resolves fork MRs from the base checkout. + it("resolves an open cross-fork PR for the specified head owner", async () => { const forkHeadSha = "c4ce035f493f2c50f656e3e5bdba29b321a74042"; const forkPr = { number: 6, @@ -2129,7 +2487,7 @@ describe("createGiteaService", () => { head: { ref: "feat/fork-change", sha: forkHeadSha, - repo: { id: 2, owner: { login: "contributor" } }, + repo: { id: 2, full_name: "contributor/renamed", owner: { login: "contributor" } }, }, base: { ref: "main", @@ -2157,6 +2515,8 @@ describe("createGiteaService", () => { cwd: "/repo", headRef: "feat/fork-change", headSha: forkHeadSha, + headRepositoryOwner: "contributor", + headRepositoryPath: "contributor/renamed", }); expect(status?.number).toBe(6); diff --git a/packages/server/src/services/gitea-service.ts b/packages/server/src/services/gitea-service.ts index 892f9cc33a..3b449fb2ca 100644 --- a/packages/server/src/services/gitea-service.ts +++ b/packages/server/src/services/gitea-service.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { LRUCache } from "lru-cache"; import { CHECK_TRAIT_ACTION_REQUIRED, CHECK_TRAIT_WARNING } from "@getpaseo/protocol/check-traits"; import { mapGiteaCommitState } from "@getpaseo/protocol/gitea-status"; import pLimit from "p-limit"; @@ -27,6 +28,8 @@ import { createUnavailableSearchResult, normalizeForgeSearchKinds, parseOptionalTime, + matchesHeadRepositoryEndpoint, + ForgeDiscoveryIncompleteError, } from "./forge-service.js"; import type { CheckDetails, @@ -95,6 +98,7 @@ const ISSUE_LIST_FIELDS = "index,state,author,url,title,body,labels,comments,cre const CURRENT_PR_LOOKUP_PAGE_SIZE = 50; const CURRENT_PR_OPEN_LOOKUP_MAX_PAGES = 4; +const CURRENT_PR_IDENTITY_READ_LIMIT = 3; const TIMELINE_PAGE_SIZE = 50; const TIMELINE_MAX_PAGES_PER_BUCKET = 4; @@ -160,6 +164,7 @@ const GiteaPrListItemSchema = z head: z.string().optional(), created: z.string().optional(), updated: z.string().optional(), + headSha: z.string().optional(), labels: z.string().optional(), ci: z.string().optional(), }) @@ -223,7 +228,9 @@ const GiteaPullRequestViewSchema = z const GiteaPullRequestRepoSchema = z .object({ id: z.number().optional(), + full_name: z.string().optional(), ssh_url: z.string().nullable().optional(), + clone_url: z.string().nullable().optional(), html_url: z.string().nullable().optional(), owner: z.object({ login: z.string().optional() }).passthrough().nullable().optional(), }) @@ -1010,6 +1017,7 @@ function currentPullRequestApiToListItem(item: GiteaCurrentPullRequestApi): Gite ...(item.mergeable != null ? { mergeable: String(item.mergeable) } : {}), base: item.base.ref, head, + headSha: item.head.sha, updated: item.updated_at, labels: item.labels.map((label) => label.name).join(","), }; @@ -1082,6 +1090,7 @@ function toCurrentPullRequestStatus(item: GiteaPrListItem): CurrentPullRequestSt state, baseRefName: item.base ?? "", headRefName: stripHeadOwner(item.head), + ...(item.headSha ? { headSha: item.headSha } : {}), isMerged: state === "merged", mergeable: mapGiteaMergeable(item), checks: [], @@ -1394,6 +1403,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For const resolveCurrentBranch = options.resolveCurrentBranch ?? defaultResolveCurrentBranch; const now = options.now ?? Date.now; const currentPrHeadShaCache = new Map(); + const pullRequestIdentityCache = new LRUCache({ max: 512, ttl: 120_000 }); async function run(args: string[], runOptions: TeaCommandRunnerOptions): Promise { const teaPath = await resolveTea(); @@ -1578,6 +1588,12 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For cwd: string; headRef: string; headSha?: string; + headRepositoryOwner?: string; + headRepositoryPath?: string; + headRepositoryEndpoint?: import("./forge-service.js").HeadRepositoryEndpoint; + changeRequestNumber?: number; + force?: boolean; + previousPullRequest?: { number?: number; url: string; state: string } | null; }): Promise { const match = await findCurrentPullRequestForHeadRef(input); if (!match) { @@ -1606,21 +1622,154 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For cwd: string; headRef: string; headSha?: string; + headRepositoryOwner?: string; + headRepositoryPath?: string; + headRepositoryEndpoint?: import("./forge-service.js").HeadRepositoryEndpoint; + changeRequestNumber?: number; + force?: boolean; + previousPullRequest?: { number?: number; url: string; state: string } | null; }): Promise { const repoIdentity = await resolveCurrentRepoIdentity(input.cwd); - const expectedHeadOwner = repoIdentity?.owner ?? null; - const openMatch = await findOpenPullRequestForHeadRef(input, expectedHeadOwner); - if (openMatch) { - return openMatch; - } - - // tea discovers repository context from any configured git remote, while - // the default remote resolver reads origin only. Preserve that broader - // discovery for repositories whose primary remote has another name. const repoPath = repoIdentity ? `repos/${encodeURIComponent(repoIdentity.owner)}/${encodeURIComponent(repoIdentity.name)}` : "repos/{owner}/{repo}"; - const recentItems = await runJsonArray( + let expectedPath = + input.headRepositoryPath ?? + (input.headRepositoryOwner + ? null + : repoIdentity && `${repoIdentity.owner}/${repoIdentity.name}`); + if (!expectedPath && input.headRepositoryOwner && input.changeRequestNumber) { + const hint = await runJson( + ["api", `${repoPath}/pulls/${input.changeRequestNumber}`], + { cwd: input.cwd }, + GiteaCurrentPullRequestApiSchema, + ); + const path = hint.head.repo?.full_name; + if (path?.split("/")[0]?.toLowerCase() === input.headRepositoryOwner.toLowerCase()) + expectedPath = path; + } + if (!expectedPath) return null; + const expectedHeadOwner = expectedPath.split("/")[0] ?? null; + const matchesRepository = (item: GiteaCurrentPullRequestApi): boolean => { + const headPath = + item.head.repo?.full_name ?? + (item.head.repo?.id !== undefined && + item.head.repo.id === item.base.repo?.id && + repoIdentity + ? `${repoIdentity.owner}/${repoIdentity.name}` + : null); + return ( + headPath?.toLowerCase() === expectedPath.toLowerCase() && + matchesHeadRepositoryEndpoint(input.headRepositoryEndpoint, [ + item.head.repo?.ssh_url, + item.head.repo?.clone_url, + item.head.repo?.html_url, + ]) + ); + }; + const open = await findOpenPullRequestForHeadRef( + input, + expectedHeadOwner, + repoPath, + matchesRepository, + expectedPath, + input.headRepositoryEndpoint, + ); + if (open.match) return open.match; + // A full open window does not make the recent all-state window useless: + // Gitea does not expose a source-branch filter for this endpoint. + return resolveRecentPullRequest({ + ...input, + expectedHeadOwner, + repoPath, + matchesRepository, + exhausted: open.exhausted, + }); + } + + async function resolveRecentPullRequest(input: { + cwd: string; + headRef: string; + previousPullRequest?: { number?: number; url: string; state: string } | null; + expectedHeadOwner: string | null; + repoPath: string; + matchesRepository: (item: GiteaCurrentPullRequestApi) => boolean; + exhausted: boolean; + }): Promise { + let recent: GiteaPrListItem | null; + try { + recent = await findRecentPullRequestForHeadRef( + input, + input.expectedHeadOwner, + input.repoPath, + input.matchesRepository, + ); + } catch (error) { + if (!(error instanceof ForgeDiscoveryIncompleteError) || !input.exhausted) throw error; + const prior = await revalidatePreviousPullRequest({ + ...input, + recent: null, + }); + if (prior) return prior; + throw error; + } + if (input.exhausted) { + const prior = await revalidatePreviousPullRequest({ + ...input, + recent, + }); + if (prior) return prior; + } + if (!recent && input.exhausted) + throw new ForgeDiscoveryIncompleteError( + "Gitea pull request discovery exceeded its open candidate window", + ); + return recent; + } + + async function revalidatePreviousPullRequest(input: { + cwd: string; + headRef: string; + repoPath: string; + expectedHeadOwner: string | null; + matchesRepository: (item: GiteaCurrentPullRequestApi) => boolean; + previousPullRequest?: { number?: number; url: string; state: string } | null; + recent: GiteaPrListItem | null; + }): Promise { + const previous = input.previousPullRequest; + if (previous?.state !== "open") return null; + if ( + input.recent && + (mapGiteaState(input.recent.state) === "open" || input.recent.url === previous.url) + ) + return null; + if (!previous.number) + throw new ForgeDiscoveryIncompleteError( + "Cannot revalidate the previously observed open pull request", + ); + const current = await runJson( + ["api", `${input.repoPath}/pulls/${previous.number}`], + { cwd: input.cwd }, + GiteaCurrentPullRequestApiSchema, + ); + const candidate = currentPullRequestApiToListItem(current); + return candidate.url === previous.url && + matchesCurrentHeadRef(candidate, input.headRef, input.expectedHeadOwner) && + input.matchesRepository(current) + ? candidate + : null; + } + + async function findRecentPullRequestForHeadRef( + input: { cwd: string; headRef: string }, + expectedHeadOwner: string | null, + repoPath: string, + matchesRepository: (item: GiteaCurrentPullRequestApi) => boolean, + ): Promise { + // tea discovers repository context from any configured git remote, while + // the default remote resolver reads origin only. Preserve that broader + // discovery for repositories whose primary remote has another name. + const items = await runJsonArray( [ "api", `${repoPath}/pulls?state=all&sort=recentupdate&page=1&limit=${CURRENT_PR_LOOKUP_PAGE_SIZE}`, @@ -1628,32 +1777,31 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For { cwd: input.cwd }, GiteaCurrentPullRequestApiSchema, ); - const candidates = recentItems.filter((item) => { - if ( + const candidates = items.filter( + (item) => matchesCurrentHeadRef( currentPullRequestApiToListItem(item), input.headRef, expectedHeadOwner, - ) - ) { - return true; - } - // A cross-fork PR carries the fork owner in its head, so the checkout-owner - // gate excludes it even though the PR lives in this base repo. The head sha - // is an unambiguous identity, so accept a match on it regardless of owner — - // this mirrors the GitLab adapter, which resolves fork MRs without an - // owner gate. - return input.headSha !== undefined && item.head.sha === input.headSha; - }); - const match = - candidates.find((item) => mapGiteaState(item.state) === "open") ?? - candidates.find( - (item) => - mapGiteaState(item.state) !== "open" && - input.headSha !== undefined && - item.head.sha === input.headSha, - ) ?? - null; + ) && matchesRepository(item), + ); + candidates.sort( + (a, b) => + Number(b.state === "open") - Number(a.state === "open") || + parseOptionalTime(b.updated_at) - parseOptionalTime(a.updated_at) || + b.number - a.number, + ); + const match = candidates[0] ?? null; + const last = items.at(-1); + if ( + items.length >= CURRENT_PR_LOOKUP_PAGE_SIZE && + (!match?.updated_at || + !last?.updated_at || + parseOptionalTime(last.updated_at) >= parseOptionalTime(match.updated_at)) + ) + throw new ForgeDiscoveryIncompleteError( + "Gitea pull request discovery exceeded its recent candidate window", + ); return match ? currentPullRequestApiToListItem(match) : null; } @@ -1662,9 +1810,15 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For cwd: string; headRef: string; headSha?: string; + force?: boolean; }, expectedHeadOwner: string | null, - ): Promise { + repoPath: string, + matchesRepository: (item: GiteaCurrentPullRequestApi) => boolean, + expectedPath: string, + endpoint?: import("./forge-service.js").HeadRepositoryEndpoint, + ): Promise<{ match: GiteaPrListItem | null; exhausted: boolean }> { + let identityReads = 0; for (let page = 1; page <= CURRENT_PR_OPEN_LOOKUP_MAX_PAGES; page += 1) { const items = await listPullRequestItems({ cwd: input.cwd, @@ -1672,17 +1826,35 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For limit: CURRENT_PR_LOOKUP_PAGE_SIZE, page, }); - const match = items.find((item) => - matchesCurrentHeadRef(item, input.headRef, expectedHeadOwner), - ); - if (match) { - return match; + for (const item of items) { + if (!matchesCurrentHeadRef(item, input.headRef, expectedHeadOwner)) continue; + const identityKey = JSON.stringify([ + input.cwd, + repoPath, + item.index, + expectedPath.toLowerCase(), + endpoint ?? null, + ]); + const cached = input.force ? undefined : pullRequestIdentityCache.get(identityKey); + if (cached === true) return { match: item, exhausted: false }; + if (cached === false) continue; + if (identityReads >= CURRENT_PR_IDENTITY_READ_LIMIT) + return { match: null, exhausted: true }; + identityReads += 1; + const detail = await runJson( + ["api", `${repoPath}/pulls/${item.index}`], + { cwd: input.cwd }, + GiteaCurrentPullRequestApiSchema, + ); + const matches = matchesRepository(detail); + pullRequestIdentityCache.set(identityKey, matches); + if (matches) return { match: { ...item, headSha: detail.head.sha }, exhausted: false }; } if (items.length < CURRENT_PR_LOOKUP_PAGE_SIZE) { - return null; + return { match: null, exhausted: false }; } } - return null; + return { match: null, exhausted: true }; } async function loadCurrentPullRequestChecks( @@ -1718,7 +1890,12 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For combined, actionTasks, ); - return applyGiteaChecks(status, combined, actionTasks, actionRequiredRunIds); + return applyGiteaChecks( + status.headSha ? status : { ...status, headSha: pr.headSha }, + combined, + actionTasks, + actionRequiredRunIds, + ); } catch { return status; } @@ -2003,6 +2180,12 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For cwd: input.cwd, headRef: input.headRef, headSha: input.headSha, + headRepositoryOwner: input.headRepositoryOwner, + headRepositoryPath: input.headRepositoryPath, + headRepositoryEndpoint: input.headRepositoryEndpoint, + previousPullRequest: input.previousPullRequest, + changeRequestNumber: input.changeRequestNumber, + force: input.force, }); }, @@ -2325,6 +2508,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For }, invalidate(input: { cwd: string }): void { + pullRequestIdentityCache.clear(); const prefix = `${input.cwd}:`; for (const key of currentPrHeadShaCache.keys()) { if (key.startsWith(prefix)) { diff --git a/packages/server/src/services/github-service.test.ts b/packages/server/src/services/github-service.test.ts index 3109bcc63c..bae1ec4844 100644 --- a/packages/server/src/services/github-service.test.ts +++ b/packages/server/src/services/github-service.test.ts @@ -23,9 +23,10 @@ const EXPECTED_GITHUB_FAST_POLL_MS = 20_000; const EXPECTED_GITHUB_SLOW_POLL_MS = 120_000; const EXPECTED_GITHUB_ERROR_BACKOFF_CAP_MS = 300_000; const CURRENT_PR_STATUS_BASE_FIELDS = - "number,url,title,state,isDraft,baseRefName,headRefName,headRefOid,mergedAt,reviewDecision,mergeable,headRepositoryOwner"; + "number,url,title,state,isDraft,baseRefName,headRefName,headRefOid,updatedAt,mergedAt,reviewDecision,mergeable,headRepositoryOwner,headRepository"; const CURRENT_PR_STATUS_FIELDS = `${CURRENT_PR_STATUS_BASE_FIELDS},statusCheckRollup`; -const FORK_PR_QUERY = "state=all&per_page=100&head=forkOwner%3Afeature%2Ffork"; +const FORK_PR_QUERY = + "state=all&per_page=100&head=forkOwner%3Afeature%2Ffork&sort=updated&direction=desc"; interface RunnerCall { args: string[]; @@ -202,6 +203,8 @@ function readFakeGitHubCliLog(logPath: string): string[] { } function currentPullRequestJson(overrides: Record = {}): string { + const repoUrl = String(overrides.url ?? "https://github.com/parentOwner/parentRepo/pull/42"); + const [, owner, name] = new URL(repoUrl).pathname.split("/"); return JSON.stringify({ number: 42, url: "https://github.com/parentOwner/parentRepo/pull/42", @@ -211,6 +214,8 @@ function currentPullRequestJson(overrides: Record = {}): string baseRefName: "main", headRefName: "feature/fork", headRefOid: "1111111111111111111111111111111111111111", + headRepositoryOwner: { login: owner }, + headRepository: { name }, mergedAt: null, statusCheckRollup: [], reviewDecision: "REVIEW_REQUIRED", @@ -244,6 +249,8 @@ function currentPullRequestGithubFactsJson(overrides: Record = } function batchPollPrNodeJson(overrides: Record = {}): Record { + const repoUrl = String(overrides.url ?? "https://github.com/acme/widgets/pull/42"); + const [, owner, name] = new URL(repoUrl).pathname.split("/"); return { number: 42, url: "https://github.com/acme/widgets/pull/42", @@ -256,7 +263,8 @@ function batchPollPrNodeJson(overrides: Record = {}): Record { cwd: "/repo", headRef: "open-button-targets-active-file", headRepositoryOwner: "fork-owner", + headRepositoryPath: "fork-owner/parentRepo", }); await vi.advanceTimersByTimeAsync(0); await flushMicrotasks(); @@ -907,6 +916,50 @@ describe("ForgeService", () => { service.dispose?.(); }); + it("keeps mixed-port endpoint proof in retained polls", async () => { + const endpoint = { + transport: "ssh" as const, + host: "forge.example", + port: "2222", + path: "parentOwner/parentRepo", + }; + const calls: string[][] = []; + const service = createGitHubService({ + ttlMs: 0, + resolveGhPath: async () => "/usr/bin/gh", + resolveRepoHost: async () => null, + resolveRepoSlug: async () => null, + runner: async (args) => { + calls.push(args); + if (args[0] === "pr") return { stdout: currentPullRequestJson(), stderr: "" }; + if (args[1] === "repos/parentOwner/parentRepo") + return { + stdout: JSON.stringify({ + ssh_url: "ssh://git@forge.example:2222/parentOwner/parentRepo.git", + }), + stderr: "", + }; + return { stdout: currentPullRequestGithubFactsJson(), stderr: "" }; + }, + }); + const reads = recordCurrentPullRequestStatusReads(service); + const statuses: Array = []; + const subscription = service.retainCurrentPullRequestStatusPoll?.({ + cwd: "/repo", + headRef: "feature/fork", + headRepositoryPath: endpoint.path, + headRepositoryEndpoint: endpoint, + onStatus: (status) => statuses.push(status), + }); + await vi.advanceTimersByTimeAsync(0); + await flushMicrotasks(); + expect(reads).toEqual([expect.objectContaining({ headRepositoryEndpoint: endpoint })]); + expect(statuses).toEqual([expect.objectContaining({ number: 42 })]); + expect(calls.filter((args) => args[1] === "repos/parentOwner/parentRepo")).toHaveLength(1); + subscription?.unsubscribe(); + service.dispose?.(); + }); + it("polls PR status at slow cadence after stable checks", async () => { let now = 0; const runner = createRunner([ @@ -1711,10 +1764,10 @@ describe("ForgeService", () => { batchPollPrNodeJson({ number: 100 + index, url: `https://github.com/acme/widgets/pull/${100 + index}`, - state: "OPEN", - mergedAt: null, + state: index === 0 ? "MERGED" : "OPEN", + mergedAt: index === 0 ? "2026-08-01T00:00:00Z" : null, headRefName: "feat-a", - headRepositoryOwner: { login: "stranger" }, + headRepositoryOwner: { login: index === 0 ? "acme" : "stranger" }, }), ); const runner = createScriptedRunner([ @@ -1740,8 +1793,7 @@ describe("ForgeService", () => { await vi.advanceTimersByTimeAsync(0); await flushMicrotasks(); - // Ten same-named fork PRs filled the page, so absence of a self-owned PR - // is inconclusive and the target resolves through the legacy path. + // A full page can contain a terminal match while an older open PR is omitted. expect(currentPullRequestStatusCalls(runner.calls)).toHaveLength(1); expect(statuses).toEqual([expect.objectContaining({ number: 42, state: "open" })]); @@ -1790,6 +1842,38 @@ describe("ForgeService", () => { service.dispose?.(); }); + it("uses a full fork path in batched polls without a separate owner hint", async () => { + const node = batchPollPrNodeJson({ + state: "OPEN", + mergedAt: null, + headRepositoryOwner: { login: "fork" }, + headRepository: { name: "renamed" }, + }); + const runner = createScriptedRunner([ + batchPollStatusJson({ t0: batchPollRepositoryJson([node]) }), + batchPollStatusJson({ t0: batchPollChecksAliasJson([]) }), + ]); + const service = createGitHubService({ + ttlMs: 0, + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + resolveRepoHost: async () => null, + resolveRepoSlug: async () => "acme/widgets", + }); + const statuses: Array = []; + const subscription = service.retainCurrentPullRequestStatusPoll?.({ + cwd: "/ws-a", + headRef: "feat-a", + headRepositoryPath: "fork/renamed", + 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({ @@ -2718,6 +2802,8 @@ describe("ForgeService", () => { isDraft: true, baseRefName: "main", headRefName: "feature/pr-pane", + headRepositoryOwner: { login: "acme" }, + headRepository: { name: "repo" }, mergedAt: null, reviewDecision: "REVIEW_REQUIRED", statusCheckRollup: [ @@ -3219,14 +3305,18 @@ describe("ForgeService", () => { }); }); - it("keeps a merged PR only when headRefOid matches the checkout HEAD", async () => { + it("keeps a merged PR when headRefOid matches the checkout HEAD", async () => { const headSha = "2222222222222222222222222222222222222222"; - const runner = createRunner([ - currentPullRequestJson({ - state: "MERGED", - mergedAt: "2026-07-17T12:00:00Z", - headRefOid: headSha, - }), + const merged = currentPullRequestJson({ + state: "MERGED", + mergedAt: "2026-07-17T12:00:00Z", + headRefOid: headSha, + headRepositoryOwner: { login: "parentOwner" }, + }); + const runner = createScriptedRunner([ + merged, + JSON.stringify({ owner: { login: "parentOwner" }, name: "parentRepo", parent: null }), + JSON.stringify([JSON.parse(merged)]), ]); const service = createGitHubService({ runner: runner.runner, @@ -3243,11 +3333,12 @@ describe("ForgeService", () => { expect(status?.state).toBe("merged"); }); - it("does not attach a stale merged PR after a same-name branch advances", async () => { + it("shows a merged PR after a same-name branch advances", async () => { const stale = currentPullRequestJson({ state: "MERGED", mergedAt: "2026-07-17T12:00:00Z", headRefOid: "1111111111111111111111111111111111111111", + headRepositoryOwner: { login: "parentOwner" }, }); const runner = createScriptedRunner([ stale, @@ -3265,12 +3356,182 @@ describe("ForgeService", () => { headRef: "feature/fork", headSha: "2222222222222222222222222222222222222222", }), - ).resolves.toBeNull(); + ).resolves.toMatchObject({ + number: 42, + state: "merged", + headSha: "1111111111111111111111111111111111111111", + }); expect(runner.calls.some((call) => call.args[0] === "pr" && call.args[1] === "list")).toBe( true, ); }); + it.each(["OPEN", "MERGED"])( + "ignores a %s PR from another repository of the same owner", + async (state) => { + const foreign = JSON.parse( + currentPullRequestJson({ + state, + headRepositoryOwner: { login: "same" }, + headRepository: { name: "other" }, + }), + ); + const wanted = JSON.parse( + currentPullRequestJson({ + number: 43, + state, + headRepositoryOwner: { login: "same" }, + headRepository: { name: "wanted" }, + }), + ); + const runner = createScriptedRunner([ + JSON.stringify(foreign), + JSON.stringify([foreign, wanted]), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headRepositoryOwner: "same", + headRepositoryPath: "same/wanted", + }); + expect(status?.number).toBe(43); + }, + ); + + it("uses the checkout hint only to discover its fork repository", async () => { + const hint = currentPullRequestJson({ + number: 9, + state: "MERGED", + headRepositoryOwner: { login: "fork" }, + headRepository: { name: "renamed" }, + }); + const open = currentPullRequestJson({ + number: 10, + headRepositoryOwner: { login: "fork" }, + headRepository: { name: "renamed" }, + }); + const runner = createScriptedRunner([hint, open]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headRepositoryOwner: "fork", + changeRequestNumber: 9, + }); + expect(status).toMatchObject({ number: 10, state: "open" }); + }); + + it("uses the full head path when no separate owner hint is present", async () => { + const other = currentPullRequestJson({ + url: "https://github.com/base/repo/pull/42", + state: "CLOSED", + headRepositoryOwner: { login: "base" }, + headRepository: { name: "repo" }, + }); + const wanted = currentPullRequestJson({ + number: 43, + url: "https://github.com/base/repo/pull/43", + headRepositoryOwner: { login: "fork" }, + headRepository: { name: "renamed" }, + }); + const runner = createScriptedRunner([ + other, + JSON.stringify({ owner: { login: "base" }, name: "repo", parent: null }), + JSON.stringify([JSON.parse(wanted)]), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + expect( + await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headRepositoryPath: "fork/renamed", + }), + ).toMatchObject({ number: 43, state: "open" }); + }); + + it("finds the correct repository when the branch search is crowded", async () => { + const other = JSON.parse( + currentPullRequestJson({ + headRepositoryOwner: { login: "same" }, + headRepository: { name: "other" }, + }), + ); + const crowd = Array.from({ length: 10 }, (_, i) => ({ ...other, number: i + 100 })); + const wanted = currentPullRequestJson({ + number: 42, + url: "https://github.com/base/project/pull/42", + headRepositoryOwner: { login: "same" }, + headRepository: { name: "wanted" }, + }); + const runner = createScriptedRunner([ + JSON.stringify(other), + JSON.stringify(crowd), + JSON.stringify({ owner: { login: "base" }, name: "project", parent: null }), + JSON.stringify([{ number: 42, state: "open", head: { repo: { full_name: "same/wanted" } } }]), + wanted, + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + expect( + await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headRepositoryOwner: "same", + headRepositoryPath: "same/wanted", + }), + ).toMatchObject({ number: 42, state: "open" }); + }); + + it("selects the most recently updated terminal PR instead of the checkout SHA", async () => { + const older = JSON.parse( + currentPullRequestJson({ + number: 41, + state: "MERGED", + mergedAt: "2026-07-01T00:00:00Z", + updatedAt: "2026-08-01T01:00:00+02:00", + headRefOid: "checkout", + }), + ); + const newer = JSON.parse( + currentPullRequestJson({ + number: 42, + state: "CLOSED", + mergedAt: null, + updatedAt: "2026-08-01T00:00:00Z", + headRefOid: "remote", + }), + ); + const runner = createScriptedRunner([ + JSON.stringify(older), + JSON.stringify({ owner: { login: "parentOwner" }, name: "parentRepo", parent: null }), + JSON.stringify([older, newer]), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + expect( + await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headSha: "checkout", + headRepositoryPath: "parentOwner/parentRepo", + }), + ).toMatchObject({ number: 42, state: "closed" }); + }); + it("prefers an open PR over an exact-SHA merged PR for the same head", async () => { const checkoutSha = "2222222222222222222222222222222222222222"; const owner = { login: "forkOwner" }; @@ -3308,6 +3569,7 @@ describe("ForgeService", () => { headRef: "feature/fork", headSha: checkoutSha, headRepositoryOwner: "forkOwner", + headRepositoryPath: "forkOwner/parentRepo", }); expect(status).toMatchObject({ number: 43, state: "open" }); @@ -3326,7 +3588,18 @@ describe("ForgeService", () => { name: "parentRepo", parent: { owner: { login: "parentOwner" }, name: "parentRepo" }, }), - JSON.stringify([{ number: 41 }, { number: 42 }]), + JSON.stringify([ + { + number: 41, + updated_at: "2026-08-02", + head: { repo: { full_name: "forkOwner/parentRepo" } }, + }, + { + number: 42, + updated_at: "2026-08-01", + head: { repo: { full_name: "forkOwner/parentRepo" } }, + }, + ]), JSON.stringify({ number: 41, url: "https://github.com/parentOwner/parentRepo/pull/41", @@ -3339,6 +3612,7 @@ describe("ForgeService", () => { statusCheckRollup: [], reviewDecision: "REVIEW_REQUIRED", headRepositoryOwner: { login: "otherFork" }, + headRepository: { name: "parentRepo" }, }), JSON.stringify({ number: 42, @@ -3352,6 +3626,7 @@ describe("ForgeService", () => { statusCheckRollup: [], reviewDecision: "REVIEW_REQUIRED", headRepositoryOwner: { login: "forkOwner" }, + headRepository: { name: "parentRepo" }, }), ]); const service = createGitHubService({ @@ -3381,7 +3656,7 @@ describe("ForgeService", () => { ]); }); - it("reads the fork PR carrying the checked-out commit past newer attempts on the branch", async () => { + it("selects the latest fork PR when a branch was reused", async () => { const checkoutSha = "3333333333333333333333333333333333333333"; const viewed: number[] = []; const runner: GitHubCommandRunner = async (args) => { @@ -3399,10 +3674,19 @@ describe("ForgeService", () => { // Newest first, and the checkout is sitting on the oldest attempt. return { stdout: JSON.stringify([ - { number: 50, head: { sha: "5".repeat(40) } }, - { number: 49, head: { sha: "4".repeat(40) } }, - { number: 48, head: { sha: "8".repeat(40) } }, - { number: 47, head: { sha: checkoutSha } }, + { + number: 50, + head: { sha: "5".repeat(40), repo: { full_name: "forkOwner/parentRepo" } }, + }, + { + number: 49, + head: { sha: "4".repeat(40), repo: { full_name: "forkOwner/parentRepo" } }, + }, + { + number: 48, + head: { sha: "8".repeat(40), repo: { full_name: "forkOwner/parentRepo" } }, + }, + { number: 47, head: { sha: checkoutSha, repo: { full_name: "forkOwner/parentRepo" } } }, ]), stderr: "", }; @@ -3436,14 +3720,39 @@ describe("ForgeService", () => { }); expect(status).toMatchObject({ - number: 47, + number: 50, repoOwner: "parentOwner", repoName: "parentRepo", }); - expect(viewed).toContain(47); + expect(viewed).not.toContain(47); expect(viewed).toHaveLength(3); }); + it("does not scan beyond the fork request budget when open status is unknown", async () => { + const refs = Array.from({ length: 100 }, (_, i) => ({ + number: i + 1, + state: "closed", + head: { repo: { full_name: "forkOwner/parentRepo" } }, + })); + const runner = createScriptedRunner([ + { error: noPullRequestError() }, + JSON.stringify({ + owner: { login: "forkOwner" }, + name: "parentRepo", + parent: { owner: { login: "parentOwner" }, name: "parentRepo" }, + }), + JSON.stringify(refs), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + expect( + await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "feature/fork" }), + ).toBeNull(); + expect(runner.calls.filter((call) => call.args[0] === "api")).toHaveLength(1); + }); + it("propagates a failed fork PR lookup instead of reporting no pull request", async () => { const rateLimitError = new GitHubCommandError({ args: ["api", "repos/parentOwner/parentRepo/pulls"], @@ -3493,6 +3802,7 @@ describe("ForgeService", () => { statusCheckRollup: [], reviewDecision: "REVIEW_REQUIRED", headRepositoryOwner: { login: headRepositoryOwner }, + headRepository: { name: "parentRepo" }, }; } const crowd = Array.from({ length: 10 }, (_, index) => @@ -3524,7 +3834,17 @@ describe("ForgeService", () => { } if (args[0] === "api" && args[1]?.startsWith("repos/")) { expect(args[1]).toContain("head=forkOwner%3Amain"); - return { stdout: JSON.stringify([{ number: 4984 }]), stderr: "" }; + return { + stdout: JSON.stringify([ + ...[1, 2, 3, 4].map((number) => ({ + number, + state: "open", + head: { repo: { full_name: "forkOwner/other" } }, + })), + { number: 4984, state: "open", head: { repo: { full_name: "forkOwner/parentRepo" } } }, + ]), + stderr: "", + }; } return { stdout: "{}", stderr: "" }; }; @@ -3560,7 +3880,7 @@ describe("ForgeService", () => { name: "parentRepo", parent: { owner: { login: "parentOwner" }, name: "parentRepo" }, }), - JSON.stringify([{ number: 42 }]), + JSON.stringify([{ number: 42, head: { repo: { full_name: "forkOwner/parentRepo" } } }]), { error: statusCheckRollupPermissionError([ "pr", @@ -3583,6 +3903,7 @@ describe("ForgeService", () => { mergedAt: null, reviewDecision: "REVIEW_REQUIRED", headRepositoryOwner: { login: "forkOwner" }, + headRepository: { name: "parentRepo" }, }), ]); const service = createGitHubService({ @@ -3679,6 +4000,8 @@ describe("ForgeService", () => { "all", "--head", "main", + "--search", + "sort:updated-desc", "--limit", "10", "--json", @@ -3716,6 +4039,7 @@ describe("ForgeService", () => { statusCheckRollup: [], reviewDecision: "REVIEW_REQUIRED", headRepositoryOwner: { login: "chethanuk" }, + headRepository: { name: "repo" }, }, ]), ]); @@ -3729,6 +4053,7 @@ describe("ForgeService", () => { cwd: "/repo", headRef: "main", headRepositoryOwner: "chethanuk", + headRepositoryPath: "chethanuk/repo", }); expect(status).toMatchObject({ @@ -3746,6 +4071,8 @@ describe("ForgeService", () => { "all", "--head", "main", + "--search", + "sort:updated-desc", "--limit", "10", "--json", @@ -3762,7 +4089,7 @@ describe("ForgeService", () => { name: "repo", parent: { owner: { login: "parentOwner" }, name: "repo" }, }), - JSON.stringify([{ number: 88 }]), + JSON.stringify([{ number: 88, head: { repo: { full_name: "forkOwner/repo" } } }]), JSON.stringify({ number: 88, url: "https://github.com/parentOwner/repo/pull/88", @@ -3775,6 +4102,7 @@ describe("ForgeService", () => { statusCheckRollup: [], reviewDecision: null, headRepositoryOwner: { login: "forkOwner" }, + headRepository: { name: "repo" }, }), ]); const service = createGitHubService({ @@ -4420,6 +4748,49 @@ describe("ForgeService", () => { expect(currentPullRequestStatusCalls(runner.calls)).toHaveLength(1); }); + it("requires forge clone URL proof for mixed ports and separates cache entries", async () => { + const calls: string[][] = []; + let cloneUrl: string | undefined = "ssh://git@forge.example:2222/parentOwner/parentRepo.git"; + const service = createGitHubService({ + resolveGhPath: async () => "/usr/bin/gh", + runner: async (args) => { + calls.push(args); + if (args[0] === "pr") return { stdout: currentPullRequestJson(), stderr: "" }; + if (args[0] === "api" && args[1] === "repos/parentOwner/parentRepo") + return { stdout: JSON.stringify({ ssh_url: cloneUrl }), stderr: "" }; + return { stdout: currentPullRequestGithubFactsJson(), stderr: "" }; + }, + }); + const endpoint = { + transport: "ssh" as const, + host: "forge.example", + port: "2222", + path: "parentOwner/parentRepo", + }; + const input = { + cwd: "/repo", + headRef: "feature/fork", + headRepositoryPath: endpoint.path, + headRepositoryEndpoint: endpoint, + }; + expect((await service.getCurrentPullRequestStatus(input))?.number).toBe(42); + expect( + await service.getCurrentPullRequestStatus({ + ...input, + headRepositoryEndpoint: { ...endpoint, port: "2223" }, + }), + ).toBeNull(); + expect( + (await service.getCurrentPullRequestStatus({ ...input, force: true, reason: "archive" })) + ?.number, + ).toBe(42); + cloneUrl = undefined; + expect( + await service.getCurrentPullRequestStatus({ ...input, force: true, reason: "archive" }), + ).toBeNull(); + expect(calls.filter((args) => args[1] === "repos/parentOwner/parentRepo")).toHaveLength(4); + }); + it("bypasses the warm PR status cache for forced reads", async () => { const runner = createRunner([ currentPullRequestJson({ number: 41, title: "First" }), diff --git a/packages/server/src/services/github-service.ts b/packages/server/src/services/github-service.ts index 2dd3fdc269..4090e61da0 100644 --- a/packages/server/src/services/github-service.ts +++ b/packages/server/src/services/github-service.ts @@ -28,6 +28,7 @@ import { createUnavailableSearchResult, normalizeForgeSearchKinds, parseOptionalTime, + matchesHeadRepositoryEndpoint, } from "./forge-service.js"; import type { CheckAnnotation, @@ -346,6 +347,8 @@ const CurrentPullRequestStatusSchema = z.object({ baseRefName: z.string().catch(""), headRefName: z.string().catch(""), headRefOid: z.string().optional(), + headRepository: z.object({ name: z.string() }).nullable().optional(), + updatedAt: z.string().optional(), mergedAt: z.string().nullable().optional(), statusCheckRollup: z.unknown().optional(), reviewDecision: z.unknown().optional(), @@ -469,8 +472,13 @@ const GitHubRepoViewSchema = z.object({ const ForkPullRequestRefsSchema = z.array( z.object({ number: z.number(), + state: z.string().optional(), + updated_at: z.string().optional(), head: z - .object({ sha: z.string().catch("") }) + .object({ + sha: z.string().catch(""), + repo: z.object({ full_name: z.string() }).nullable().optional(), + }) .nullable() .optional(), }), @@ -524,7 +532,7 @@ query PullRequestCheckoutTarget($owner: String!, $name: String!, $number: Int!) }`; const CURRENT_PR_STATUS_BASE_FIELDS = - "number,url,title,state,isDraft,baseRefName,headRefName,headRefOid,mergedAt,reviewDecision,mergeable,headRepositoryOwner"; + "number,url,title,state,isDraft,baseRefName,headRefName,headRefOid,updatedAt,mergedAt,reviewDecision,mergeable,headRepositoryOwner,headRepository"; const CURRENT_PR_STATUS_FIELDS = `${CURRENT_PR_STATUS_BASE_FIELDS},statusCheckRollup`; const PULL_REQUEST_STATUS_FACTS_QUERY = ` @@ -640,6 +648,8 @@ const BatchPollPrNodeSchema = z.object({ baseRefName: z.string().catch(""), headRefName: z.string().catch(""), headRefOid: z.string().catch(""), + headRepository: z.object({ name: z.string() }).nullable().optional(), + updatedAt: z.string().optional(), mergedAt: z.string().nullable().optional(), mergeable: PullRequestMergeableSchema.optional().default("UNKNOWN"), reviewDecision: z.unknown().optional(), @@ -744,12 +754,14 @@ fragment PaseoPollPullRequest on PullRequest { baseRefName headRefName headRefOid + updatedAt mergedAt mergeable reviewDecision headRepositoryOwner { login } + headRepository { name } mergeStateStatus autoMergeRequest { enabledAt @@ -799,7 +811,7 @@ function buildBatchPullRequestStatusQuery( viewerDefaultMergeMethod pullRequests(headRefName: ${graphqlString( entry.headRef, - )}, first: ${BATCH_PR_CANDIDATE_LIMIT}, orderBy: {field: CREATED_AT, direction: DESC}) { + )}, first: ${BATCH_PR_CANDIDATE_LIMIT}, orderBy: {field: UPDATED_AT, direction: DESC}) { nodes { ...PaseoPollPullRequest } @@ -1071,6 +1083,9 @@ interface GitHubPollTarget { headRef: string; headSha?: string; headRepositoryOwner?: string; + headRepositoryPath?: string; + headRepositoryEndpoint?: import("./forge-service.js").HeadRepositoryEndpoint; + changeRequestNumber?: number; retainCount: number; /** Absolute due time on the shared poll grid; null while a poll is in flight. */ nextDueAt: number | null; @@ -1116,8 +1131,9 @@ interface GitHubBatchDistribution { interface ResolvedPullRequestCandidate { status: CurrentPullRequestStatus; - headSha?: string; + updatedAt?: string; headRepositoryOwner?: string; + headRepositoryPath?: string; } export function createGitHubService(options: CreateGitHubServiceOptions = {}): GitHubService { @@ -1357,6 +1373,9 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G headRef: string; headSha?: string; headRepositoryOwner?: string; + headRepositoryPath?: string; + headRepositoryEndpoint?: import("./forge-service.js").HeadRepositoryEndpoint; + changeRequestNumber?: number; }): string { return buildCacheKey({ cwd: target.cwd, @@ -1365,6 +1384,9 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G headRef: target.headRef, headSha: target.headSha, headRepositoryOwner: target.headRepositoryOwner, + headRepositoryPath: target.headRepositoryPath, + headRepositoryEndpoint: target.headRepositoryEndpoint, + changeRequestNumber: target.changeRequestNumber, }, }); } @@ -1374,6 +1396,9 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G headRef: string; headSha?: string; headRepositoryOwner?: string; + headRepositoryPath?: string; + headRepositoryEndpoint?: import("./forge-service.js").HeadRepositoryEndpoint; + changeRequestNumber?: number; status: CurrentPullRequestStatus | null; notify: boolean; }): void { @@ -1702,6 +1727,13 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G redirected: [], }; for (const [index, entry] of input.entries.entries()) { + if ( + entry.target.headRepositoryEndpoint || + (entry.target.headRepositoryOwner && !entry.target.headRepositoryPath) + ) { + distribution.legacyFallback.push(entry.target); + continue; + } const aliasValue = input.aliases[batchAlias(index)]; const repository = aliasValue ? BatchPollRepositorySchema.safeParse(aliasValue) : null; if (!repository?.success) { @@ -1726,16 +1758,14 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G distribution.redirected.push(entry); continue; } + if (repository.data.pullRequests.nodes.length >= BATCH_PR_CANDIDATE_LIMIT) { + distribution.legacyFallback.push(entry.target); + continue; + } const node = selectBatchPollNode(entry, repository.data); if (!node) { - // Legacy fallback covers the two ways "no match" can be wrong: a fork - // whose PR lives in the parent repository, and a full candidate page — - // ten newer same-named fork PRs can crowd the checkout's own PR out of - // the first:N window, so absence is only conclusive on a partial page. - if ( - repository.data.isFork || - repository.data.pullRequests.nodes.length >= BATCH_PR_CANDIDATE_LIMIT - ) { + // A fork's PR can live in the parent repository. + if (repository.data.isFork) { distribution.legacyFallback.push(entry.target); } else { finalizeBatchPollTarget({ @@ -1812,7 +1842,11 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G // name. Without a fork-owner hint the workspace branch lives in this // repository, so only self-owned heads are real candidates (mirrors // `gh pr view` resolving the current branch's own PR). - if (!headRepositoryOwner && candidate.headRepositoryOwner?.toLowerCase() !== selfOwner) { + if ( + !headRepositoryOwner && + !entry.target.headRepositoryPath && + candidate.headRepositoryOwner?.toLowerCase() !== selfOwner + ) { continue; } nodeByCandidate.set(candidate, node); @@ -1821,8 +1855,9 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G const match = pickPullRequestCandidate({ candidates, headRef: entry.target.headRef, - headSha: entry.target.headSha, headRepositoryOwner, + headRepositoryPath: + entry.target.headRepositoryPath ?? `${entry.originOwner}/${entry.originName}`, }); return match ? (nodeByCandidate.get(match) ?? null) : null; } @@ -1958,6 +1993,9 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G headRef: entry.target.headRef, headSha: entry.target.headSha, headRepositoryOwner: entry.target.headRepositoryOwner, + headRepositoryPath: entry.target.headRepositoryPath, + headRepositoryEndpoint: entry.target.headRepositoryEndpoint, + changeRequestNumber: entry.target.changeRequestNumber, }, }); // A direct or forced read can finish between this batch becoming due and @@ -1970,6 +2008,9 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G headRef: entry.target.headRef, headSha: entry.target.headSha, headRepositoryOwner: entry.target.headRepositoryOwner, + headRepositoryPath: entry.target.headRepositoryPath, + headRepositoryEndpoint: entry.target.headRepositoryEndpoint, + changeRequestNumber: entry.target.changeRequestNumber, status: existing.value as CurrentPullRequestStatus | null, notify: true, }); @@ -1983,6 +2024,9 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G headRef: entry.target.headRef, headSha: entry.target.headSha, headRepositoryOwner: entry.target.headRepositoryOwner, + headRepositoryPath: entry.target.headRepositoryPath, + headRepositoryEndpoint: entry.target.headRepositoryEndpoint, + changeRequestNumber: entry.target.changeRequestNumber, status, notify: true, }); @@ -1995,6 +2039,9 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G headRef: target.headRef, headSha: target.headSha, headRepositoryOwner: target.headRepositoryOwner, + headRepositoryPath: target.headRepositoryPath, + headRepositoryEndpoint: target.headRepositoryEndpoint, + changeRequestNumber: target.changeRequestNumber, reason: "self-heal-github", }); } catch (error) { @@ -2161,6 +2208,9 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G headRef: input.headRef, headSha: input.headSha, headRepositoryOwner: input.headRepositoryOwner, + headRepositoryPath: input.headRepositoryPath, + headRepositoryEndpoint: input.headRepositoryEndpoint, + changeRequestNumber: input.changeRequestNumber, }, readOptions: input, load: async () => { @@ -2169,8 +2219,29 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G headRef: input.headRef, headSha: input.headSha, headRepositoryOwner: input.headRepositoryOwner, + headRepositoryPath: input.headRepositoryPath, + changeRequestNumber: input.changeRequestNumber, run, }); + if (status && input.headRepositoryEndpoint) { + if (!input.headRepositoryPath) return null; + const repo = await runGhJson( + ["api", `repos/${input.headRepositoryPath}`], + { cwd: input.cwd }, + z.object({ + ssh_url: z.string().nullable().optional(), + clone_url: z.string().nullable().optional(), + }), + "{}", + ); + if ( + !matchesHeadRepositoryEndpoint(input.headRepositoryEndpoint, [ + repo.ssh_url, + repo.clone_url, + ]) + ) + return null; + } return addCurrentPullRequestGithubFacts({ cwd: input.cwd, status, run }); }, }).then((status) => { @@ -2179,6 +2250,9 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G headRef: input.headRef, headSha: input.headSha, headRepositoryOwner: input.headRepositoryOwner, + headRepositoryPath: input.headRepositoryPath, + headRepositoryEndpoint: input.headRepositoryEndpoint, + changeRequestNumber: input.changeRequestNumber, status, notify: input.reason === "self-heal-github", }); @@ -2571,6 +2645,9 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G headRef: input.headRef, headSha: input.headSha, headRepositoryOwner: input.headRepositoryOwner, + headRepositoryPath: input.headRepositoryPath, + headRepositoryEndpoint: input.headRepositoryEndpoint, + changeRequestNumber: input.changeRequestNumber, retainCount: 0, nextDueAt: null, pollCycleStartedAt: null, @@ -2952,60 +3029,124 @@ function isGitHubRateLimitError(error: unknown): boolean { ); } -async function resolveCurrentPullRequestView(options: { +interface CurrentPullRequestLookup { cwd: string; headRef: string; headSha?: string; headRepositoryOwner?: string; + headRepositoryPath?: string; + changeRequestNumber?: number; run: (args: string[], options: GitHubCommandRunnerOptions) => Promise; -}): Promise { - const viewCandidate = await tryCurrentPullRequestView(options); - const viewMatch = viewCandidate - ? pickPullRequestCandidate({ - candidates: [viewCandidate], - headRef: options.headRef, - headSha: options.headSha, - headRepositoryOwner: options.headRepositoryOwner, - }) - : null; - if (viewMatch) { +} + +async function resolveCurrentPullRequestView( + options: CurrentPullRequestLookup, +): Promise { + const resolved = await resolveHintedHeadRepository(options); + return resolved ? resolveCurrentPullRequestForRepository(resolved) : null; +} + +async function resolveHintedHeadRepository( + options: CurrentPullRequestLookup, +): Promise { + if (options.headRepositoryOwner && !options.headRepositoryPath) { + if (!options.changeRequestNumber) return null; + const hint = await tryCurrentPullRequestView({ + ...options, + args: ["pr", "view", String(options.changeRequestNumber)], + }); + if ( + !hint?.headRepositoryPath || + hint.headRepositoryOwner?.toLowerCase() !== options.headRepositoryOwner.toLowerCase() + ) + return null; + return { ...options, headRepositoryPath: hint.headRepositoryPath }; + } + return options; +} + +async function resolveCurrentPullRequestForRepository( + options: CurrentPullRequestLookup, +): Promise { + const viewMatch = await findDirectPullRequestMatch(options); + if (viewMatch?.status.state === "open") { return viewMatch.status; } - let headRepositoryOwner = options.headRepositoryOwner; + if (!options.headRepositoryOwner) return resolvePullRequestWithoutOwner(options, viewMatch); + return resolveListedPullRequest(options, viewMatch, options.headRepositoryOwner); +} - if (!headRepositoryOwner) { - const repo = await getGitHubRepoView(options); - const forkOwner = repo?.owner?.login; - const parentOwner = repo?.parent?.owner?.login; - const parentName = repo?.parent?.name; - if (!forkOwner) { - return null; - } - if (parentOwner && parentName) { - return resolveForkPullRequestView({ - ...options, - forkOwner, - repo: `${parentOwner}/${parentName}`, - }); - } - headRepositoryOwner = forkOwner; - } +async function resolvePullRequestWithoutOwner( + options: CurrentPullRequestLookup, + viewMatch: ResolvedPullRequestCandidate | null, +): Promise { + const repo = await getGitHubRepoView(options); + const forkOwner = repo?.owner?.login; + const forkName = repo?.name; + const parentOwner = repo?.parent?.owner?.login; + const parentName = repo?.parent?.name; + if (!forkOwner || !forkName) return null; + const headRepositoryPath = options.headRepositoryPath ?? `${forkOwner}/${forkName}`; + options = { ...options, headRepositoryPath }; + const expectedOwner = headRepositoryPath.split("/")[0]; + if (!expectedOwner) return null; + if (parentOwner && parentName) + return resolveForkPullRequestView({ + ...options, + forkOwner: expectedOwner, + repo: `${parentOwner}/${parentName}`, + }); + return resolveListedPullRequest(options, viewMatch, expectedOwner); +} +async function resolveListedPullRequest( + options: CurrentPullRequestLookup, + viewMatch: ResolvedPullRequestCandidate | null, + headRepositoryOwner: string, +): Promise { const candidates = await listCurrentPullRequestCandidates({ cwd: options.cwd, headRef: options.headRef, run: options.run, }); + if (candidates.length >= BATCH_PR_CANDIDATE_LIMIT && options.headRepositoryPath) { + return resolveCrowdedPullRequests(options, options.headRepositoryPath); + } const match = pickPullRequestCandidate({ - candidates, + candidates: viewMatch ? [viewMatch, ...candidates] : candidates, headRef: options.headRef, - headSha: options.headSha, headRepositoryOwner, + headRepositoryPath: options.headRepositoryPath, }); return match?.status ?? null; } +async function resolveCrowdedPullRequests( + options: CurrentPullRequestLookup, + headRepositoryPath: string, +): Promise { + const repo = await getGitHubRepoView(options); + const owner = repo?.parent?.owner?.login ?? repo?.owner?.login; + const name = repo?.parent?.name ?? repo?.name; + const forkOwner = headRepositoryPath.split("/")[0]; + if (!owner || !name || !forkOwner) return null; + return resolveForkPullRequestView({ ...options, forkOwner, repo: `${owner}/${name}` }); +} + +async function findDirectPullRequestMatch( + options: CurrentPullRequestLookup, +): Promise { + const candidate = await tryCurrentPullRequestView(options); + if (!candidate) return null; + return pickPullRequestCandidate({ + candidates: [candidate], + headRef: options.headRef, + headRepositoryOwner: options.headRepositoryOwner, + headRepositoryPath: options.headRepositoryPath, + }); +} + // A fork checkout's pull request lives in the parent repository, where only // its head repository tells it apart from every other fork's pull request on // the same branch name. `gh pr list --head` cannot ask that question: it @@ -3019,6 +3160,7 @@ async function resolveForkPullRequestView(options: { headRef: string; headSha?: string; forkOwner: string; + headRepositoryPath?: string; repo: string; run: (args: string[], options: GitHubCommandRunnerOptions) => Promise; }): Promise { @@ -3038,22 +3180,19 @@ async function resolveForkPullRequestView(options: { (candidate): candidate is ResolvedPullRequestCandidate => candidate !== null, ), headRef: options.headRef, - headSha: options.headSha, headRepositoryOwner: options.forkOwner, + headRepositoryPath: options.headRepositoryPath, }); return match?.status ?? null; } -// The endpoint answers with the fork branch's pull requests, newest first, so -// a branch reused for several attempts can carry the checkout's head commit on -// an older one. Those are read first, and the rest of the window is filled -// newest-first, so the full read stays bounded without dropping the attempt the -// checkout is actually sitting on. +// The endpoint answers with the fork branch's pull requests, newest first. async function listForkPullRequestNumbers(options: { cwd: string; headRef: string; headSha?: string; forkOwner: string; + headRepositoryPath?: string; repo: string; run: (args: string[], options: GitHubCommandRunnerOptions) => Promise; }): Promise { @@ -3061,6 +3200,8 @@ async function listForkPullRequestNumbers(options: { state: "all", per_page: String(FORK_PR_PAGE_SIZE), head: `${options.forkOwner}:${options.headRef}`, + sort: "updated", + direction: "desc", }); const args = ["api", `repos/${options.repo}/pulls?${query.toString()}`]; const stdout = await options.run(args, { cwd: options.cwd }); @@ -3069,12 +3210,19 @@ async function listForkPullRequestNumbers(options: { cwd: options.cwd, emptyFallback: "[]", }); - function carriesHeadSha(ref: (typeof refs)[number]): boolean { - return options.headSha !== undefined && ref.head?.sha === options.headSha; - } - return [...refs.filter(carriesHeadSha), ...refs.filter((ref) => !carriesHeadSha(ref))] - .slice(0, FORK_PR_VIEW_LIMIT) - .map((ref) => ref.number); + const matching = refs + .filter( + (ref) => + ref.head?.repo?.full_name?.toLowerCase() === options.headRepositoryPath?.toLowerCase(), + ) + .sort( + (a, b) => + Number(b.state === "open") - Number(a.state === "open") || + 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 []; + return matching.slice(0, FORK_PR_VIEW_LIMIT).map((ref) => ref.number); } async function addCurrentPullRequestGithubFacts(options: { @@ -3163,7 +3311,18 @@ async function listCurrentPullRequestCandidates(options: { headRef: string; run: (args: string[], options: GitHubCommandRunnerOptions) => Promise; }): Promise { - const args = ["pr", "list", "--state", "all", "--head", options.headRef, "--limit", "10"]; + const args = [ + "pr", + "list", + "--state", + "all", + "--head", + options.headRef, + "--search", + "sort:updated-desc", + "--limit", + String(BATCH_PR_CANDIDATE_LIMIT), + ]; try { const stdout = await runCurrentPullRequestStatusCommand({ cwd: options.cwd, @@ -3337,11 +3496,13 @@ function toBatchCurrentPullRequestItem( baseRefName: node.baseRefName, headRefName: node.headRefName, ...(node.headRefOid ? { headRefOid: node.headRefOid } : {}), + ...(node.updatedAt ? { updatedAt: node.updatedAt } : {}), mergedAt: node.mergedAt ?? null, ...(statusCheckRollup ? { statusCheckRollup } : {}), reviewDecision: node.reviewDecision, mergeable: node.mergeable, headRepositoryOwner: node.headRepositoryOwner, + headRepository: node.headRepository, }; } @@ -3354,11 +3515,15 @@ function toCurrentPullRequestCandidate( return null; } const headRepositoryOwner = item.headRepositoryOwner?.login; - const headSha = item.headRefOid; + const headRepositoryPath = + headRepositoryOwner && item.headRepository?.name + ? `${headRepositoryOwner}/${item.headRepository.name}` + : undefined; return { status, - ...(headSha ? { headSha } : {}), + updatedAt: item.updatedAt, ...(headRepositoryOwner ? { headRepositoryOwner } : {}), + ...(headRepositoryPath ? { headRepositoryPath } : {}), }; } @@ -3373,55 +3538,53 @@ function hasResolvedRepoIdentity(status: CurrentPullRequestStatus): boolean { function pickPullRequestCandidate(options: { candidates: ResolvedPullRequestCandidate[]; headRef: string; - headSha?: string; headRepositoryOwner?: string; + headRepositoryPath?: string; }): ResolvedPullRequestCandidate | null { const matching = options.candidates.filter((candidate) => { if (!isCandidateForHeadRef(candidate, options.headRef)) { return false; } if ( - candidate.status.state !== "open" && - (!options.headSha || candidate.headSha !== options.headSha) - ) { + options.headRepositoryPath && + candidate.headRepositoryPath?.toLowerCase() !== options.headRepositoryPath.toLowerCase() + ) return false; - } if (!options.headRepositoryOwner) { - return true; + if (options.headRepositoryPath) return true; + return Boolean( + candidate.headRepositoryPath && + candidate.headRepositoryPath.toLowerCase() === + `${candidate.status.repoOwner}/${candidate.status.repoName}`.toLowerCase(), + ); } - return candidate.headRepositoryOwner === options.headRepositoryOwner; + return ( + candidate.headRepositoryOwner?.toLowerCase() === options.headRepositoryOwner.toLowerCase() + ); }); - matching.sort((left, right) => - comparePullRequestCandidatePreference(left, right, options.headSha), - ); + matching.sort(comparePullRequestCandidatePreference); return matching[0] ?? null; } function comparePullRequestCandidatePreference( left: ResolvedPullRequestCandidate, right: ResolvedPullRequestCandidate, - headSha?: string, ): number { const stateRank = getPullRequestStateRank(left.status) - getPullRequestStateRank(right.status); if (stateRank !== 0) { return stateRank; } - const leftExact = headSha !== undefined && left.headSha === headSha; - const rightExact = headSha !== undefined && right.headSha === headSha; - if (leftExact !== rightExact) { - return leftExact ? -1 : 1; - } - return 0; + return ( + parseOptionalTime(right.updatedAt) - parseOptionalTime(left.updatedAt) || + (right.status.number ?? 0) - (left.status.number ?? 0) + ); } function getPullRequestStateRank(status: CurrentPullRequestStatus): number { - if (status.state === "open" || status.isDraft) { + if (status.state === "open") { return 0; } - if (status.state === "merged") { - return 1; - } - return 2; + return 1; } type GitHubCloneProtocol = "https" | "ssh"; @@ -3963,6 +4126,7 @@ function toCurrentPullRequestStatus( const checks = parseStatusCheckRollup(item.statusCheckRollup); return { ...(typeof item.number === "number" ? { number: item.number } : {}), + ...(item.headRefOid ? { headSha: item.headRefOid } : {}), ...(repoIdentity ? { repoOwner: repoIdentity.owner, repoName: repoIdentity.name } : {}), url: item.url, title: item.title, diff --git a/packages/server/src/services/gitlab-service.test.ts b/packages/server/src/services/gitlab-service.test.ts index b9b6d447d4..5d81bbf55f 100644 --- a/packages/server/src/services/gitlab-service.test.ts +++ b/packages/server/src/services/gitlab-service.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import type { PullRequestCommandStatus } from "./forge-service.js"; +import { ForgeDiscoveryIncompleteError } from "./forge-service.js"; import { GITLAB_ACTIVE_PIPELINE_STATUS_SET } from "@getpaseo/protocol/gitlab-pipeline"; import type { GitLabStatusFacts } from "./gitlab-facts.js"; import { @@ -261,6 +262,84 @@ const DISCUSSIONS = [ ]; describe("createGitLabService", () => { + 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 = + "ssh://git@gitlab.example.com:2222/example-group/example-project.git"; + const { service, calls } = makeService((args) => { + if (args[1] === "list") return ok(JSON.stringify([mr])); + if (args[1] === "view") return ok(JSON.stringify(mr)); + if (args[1] === "projects/example-group%2Fexample-project") + return ok( + JSON.stringify({ + id: 101, + path_with_namespace: "example-group/example-project", + ssh_url_to_repo: cloneUrl, + }), + ); + return ok("{}"); + }); + const endpoint = { + transport: "ssh" as const, + host: "gitlab.example.com", + port: "2222", + path: "example-group/example-project", + }; + const input = { + cwd: "/repo", + headRef: "topic", + headRepositoryPath: endpoint.path, + headRepositoryEndpoint: endpoint, + }; + expect((await service.getCurrentPullRequestStatus(input))?.number).toBe(14); + expect( + await service.getCurrentPullRequestStatus({ + ...input, + headRepositoryEndpoint: { ...endpoint, port: "2223" }, + }), + ).toBeNull(); + expect( + (await service.getCurrentPullRequestStatus({ ...input, force: true, reason: "archive" })) + ?.number, + ).toBe(14); + cloneUrl = undefined; + expect( + await service.getCurrentPullRequestStatus({ ...input, force: true, reason: "archive" }), + ).toBeNull(); + expect( + calls.filter((args) => args[1] === "projects/example-group%2Fexample-project"), + ).toHaveLength(3); + }); + it("finds the target fork without reading unrelated source projects", async () => { + const unrelated = Array.from({ length: 4 }, (_, i) => ({ + ...OPEN_MR, + iid: i + 30, + source_branch: "topic", + source_project_id: i + 200, + })); + const wanted = { ...OPEN_MR, iid: 44, source_branch: "topic", source_project_id: 300 }; + const { service, calls } = makeService((args) => { + if (args[1] === "list") return ok(JSON.stringify([...unrelated, wanted])); + if (args[1] === "view") return ok(JSON.stringify(wanted)); + if (args[1] === "projects/team%2Ffork") + return ok(JSON.stringify({ id: 300, path_with_namespace: "team/fork" })); + if (args[0] === "api" && /^projects\/\d+$/.test(args[1])) + return ok( + JSON.stringify({ id: Number(args[1].split("/")[1]), path_with_namespace: "other/repo" }), + ); + return ok("{}"); + }); + expect( + ( + await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "topic", + headRepositoryPath: "team/fork", + }) + )?.number, + ).toBe(44); + expect(calls.filter((args) => /^projects\/\d+$/.test(args[1] ?? ""))).toHaveLength(0); + }); it("maps a glab merge request view to the neutral current PR status", async () => { const { service, calls } = makeService((args) => ok(JSON.stringify(args[1] === "list" ? [OPEN_MR] : OPEN_MR)), @@ -321,7 +400,7 @@ describe("createGitLabService", () => { expect(status).toBeNull(); }); - it("selects a terminal merge request only when its head SHA matches the checkout", async () => { + it("selects the latest terminal merge request even when the checkout head differs", async () => { const branch = "dev"; const checkoutSha = "2222222222222222222222222222222222222222"; const newestStale = { @@ -342,7 +421,7 @@ describe("createGitLabService", () => { }; const { service, calls } = makeService((args) => { if (args[1] === "list") return ok(JSON.stringify([newestStale, exactOlder])); - if (args[1] === "view" && args[2] === "270") return ok(JSON.stringify(exactOlder)); + if (args[1] === "view" && args[2] === "271") return ok(JSON.stringify(newestStale)); if (args[0] === "api") return ok("{}"); throw new Error(`unexpected call: ${args.join(" ")}`); }); @@ -353,18 +432,159 @@ describe("createGitLabService", () => { headSha: checkoutSha, }); - expect(status?.number).toBe(270); - expect(calls[1]).toEqual(["mr", "view", "270", "-F", "json"]); + expect(status?.number).toBe(271); + expect(status?.headSha).toBe(newestStale.sha); + expect(calls[1]).toEqual(["mr", "view", "271", "-F", "json"]); }); - it("does not attach the latest historical merge request after a reused branch advances", async () => { + it.each(["opened", "merged"])( + "matches a nested, renamed fork for a %s merge request", + async (state) => { + const wrong = { ...OPEN_MR, iid: 41, state, source_branch: "topic", source_project_id: 201 }; + const right = { ...wrong, iid: 42, source_project_id: 202 }; + const { service } = makeService((args) => { + if (args[1] === "list") return ok(JSON.stringify([wrong, right])); + if (args[1] === "view") return ok(JSON.stringify(right)); + if (args[1] === "projects/team%2Fsubgroup%2Frenamed") + return ok(JSON.stringify({ id: 202, path_with_namespace: "team/subgroup/renamed" })); + if (args[1] === "projects/201") + return ok(JSON.stringify({ path_with_namespace: "team/subgroup/other" })); + if (args[1] === "projects/202") + return ok(JSON.stringify({ path_with_namespace: "team/subgroup/renamed" })); + if (args[0] === "api") return ok("{}"); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + const result = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "topic", + headRepositoryPath: "team/subgroup/renamed", + }); + expect(result?.number).toBe(42); + }, + ); + + it("resolves a checkout hint to the full fork project before selecting the current request", async () => { + const hint = { ...OPEN_MR, iid: 41, source_branch: "topic", source_project_id: 202 }; + const current = { ...hint, iid: 42, state: "opened" }; + const { service } = makeService((args) => { + if (args[1] === "list") return ok(JSON.stringify([current])); + if (args[1] === "view") return ok(JSON.stringify(args[2] === "41" ? hint : current)); + if (args[1] === "projects/202") + return ok(JSON.stringify({ path_with_namespace: "team/subgroup/renamed" })); + if (args[1] === "projects/team%2Fsubgroup%2Frenamed") + return ok(JSON.stringify({ id: 202, path_with_namespace: "team/subgroup/renamed" })); + if (args[0] === "api") return ok("{}"); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + const result = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "topic", + headRepositoryOwner: "team", + changeRequestNumber: 41, + }); + expect(result?.number).toBe(42); + }); + + it("prefers an open merge request over a more recently updated terminal request", async () => { + const terminal = { + ...OPEN_MR, + iid: 50, + state: "merged", + source_branch: "topic", + updated_at: "2026-08-01T00:00:00Z", + }; + const open = { + ...OPEN_MR, + iid: 49, + state: "opened", + source_branch: "topic", + updated_at: "2026-07-01T00:00:00Z", + }; + const { service } = makeService((args) => { + if (args[1] === "list") return ok(JSON.stringify([terminal, open])); + if (args[1] === "view") return ok(JSON.stringify(open)); + if (args[0] === "api") return ok("{}"); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + expect( + await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "topic" }), + ).toMatchObject({ number: 49, state: "open" }); + }); + + it("does not select a terminal request when the first page cannot establish whether an open one exists", async () => { + const terminal = Array.from({ length: 100 }, (_, i) => ({ + ...OPEN_MR, + iid: i + 100, + state: "merged", + source_branch: "topic", + })); + const open = { ...OPEN_MR, iid: 99, state: "opened", source_branch: "topic" }; + const { service, calls } = makeService((args) => { + if (args[1] === "list") + return ok(JSON.stringify(args.includes("--page") ? [open] : terminal)); + if (args[1] === "view") return ok(JSON.stringify(open)); + if (args[0] === "api") return ok("{}"); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + await expect( + service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "topic" }), + ).rejects.toBeInstanceOf(ForgeDiscoveryIncompleteError); + expect(calls.filter((args) => args[1] === "list")).toHaveLength(1); + }); + + it("bounds source project identity reads when a branch is crowded", async () => { + const requests = Array.from({ length: 4 }, (_, i) => ({ + ...OPEN_MR, + iid: i + 20, + state: "merged", + source_branch: "topic", + source_project_id: i + 200, + })); + const { service, calls } = makeService((args) => { + if (args[1] === "list") return ok(JSON.stringify(requests)); + if (args[0] === "api" && args[1].startsWith("projects/")) + return ok(JSON.stringify({ path_with_namespace: "elsewhere/repo" })); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + expect( + await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "topic" }), + ).toBeNull(); + expect( + calls.filter((args) => args[0] === "api" && /^projects\/\d+$/.test(args[1])), + ).toHaveLength(0); + }); + + it("reuses a resolved source project identity on subsequent polls", async () => { + const mr = { ...OPEN_MR, iid: 44, source_branch: "topic", source_project_id: 202 }; + const { service, calls } = makeService((args) => { + if (args[1] === "list") return ok(JSON.stringify([mr])); + if (args[1] === "view") return ok(JSON.stringify(mr)); + if (args[1] === "projects/team%2Ffork") + return ok(JSON.stringify({ id: 202, path_with_namespace: "team/fork" })); + if (args[0] === "api") return ok("{}"); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + const input = { cwd: "/repo", headRef: "topic", headRepositoryPath: "team/fork" }; + expect((await service.getCurrentPullRequestStatus(input))?.number).toBe(44); + expect((await service.getCurrentPullRequestStatus(input))?.number).toBe(44); + expect(calls.filter((args) => args[1] === "projects/team%2Ffork")).toHaveLength(1); + await service.getCurrentPullRequestStatus({ ...input, force: true, reason: "archive" }); + expect(calls.filter((args) => args[1] === "projects/team%2Ffork")).toHaveLength(2); + }); + + it("shows the latest historical merge request after a reused branch advances", async () => { const stale = { ...OPEN_MR, state: "merged", source_branch: "dev", sha: "1111111111111111111111111111111111111111", }; - const { service, calls } = makeService(() => ok(JSON.stringify([stale]))); + const { service, calls } = makeService((args) => { + if (args[1] === "list") return ok(JSON.stringify([stale])); + if (args[1] === "view") return ok(JSON.stringify(stale)); + if (args[0] === "api") return ok("{}"); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); await expect( service.getCurrentPullRequestStatus({ @@ -372,8 +592,8 @@ describe("createGitLabService", () => { headRef: "dev", headSha: "2222222222222222222222222222222222222222", }), - ).resolves.toBeNull(); - expect(calls).toEqual([currentMrListArgs("dev")]); + ).resolves.toMatchObject({ number: stale.iid, state: "merged" }); + expect(calls[0]).toEqual(currentMrListArgs("dev")); }); it("looks up a numeric current branch through the source-branch list filter", async () => { diff --git a/packages/server/src/services/gitlab-service.ts b/packages/server/src/services/gitlab-service.ts index c5f54be2cb..31b7b4e8cc 100644 --- a/packages/server/src/services/gitlab-service.ts +++ b/packages/server/src/services/gitlab-service.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { LRUCache } from "lru-cache"; import { parseGitRemoteLocation } from "@getpaseo/protocol/git-remote"; import { findExecutable } from "../executable-resolution/executable-resolution.js"; import { @@ -17,6 +18,8 @@ import { createUnavailableSearchResult, normalizeForgeSearchKinds, parseOptionalTime, + matchesHeadRepositoryEndpoint, + ForgeDiscoveryIncompleteError, } from "./forge-service.js"; import type { CheckDetails, @@ -720,6 +723,7 @@ function toCurrentPullRequestStatus( const projectPath = extractProjectPath(mr.references?.full); return { number: mr.iid, + ...(mr.sha ? { headSha: mr.sha } : {}), ...(owner ? { repoOwner: owner } : {}), ...(name ? { repoName: name } : {}), ...(projectPath ? { projectPath } : {}), @@ -911,6 +915,15 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F const runner = options.runner ?? runGlabCommand; const resolveGlab = createCachedCliPathResolver(options.resolveGlabPath ?? resolveGlabPath); const resolveRemoteUrl = options.resolveRemoteUrl ?? defaultResolveRemoteUrl; + const projectPaths = new LRUCache< + string, + { + id?: number; + path_with_namespace: string; + ssh_url_to_repo?: string | null; + http_url_to_repo?: string | null; + } + >({ max: 512, ttl: 120_000 }); async function run(args: string[], runOptions: GlabCommandRunnerOptions): Promise { const glabPath = await resolveGlab(); @@ -974,20 +987,122 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F ); } + async function resolveExpectedHeadProject( + cwd: string, + headRepositoryPath?: string, + headRepositoryOwner?: string, + changeRequestNumber?: number, + force = false, + ): 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 (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; + } + + async function readProject(cwd: string, projectId: number | string, force = false) { + const key = JSON.stringify([cwd, await resolveRemoteUrl(cwd), projectId]); + const cached = force ? undefined : projectPaths.get(key); + if (cached) return cached; + const source = await runJson( + ["api", `projects/${encodeURIComponent(projectId)}`], + { cwd }, + z.object({ + id: z.number().optional(), + path_with_namespace: z.string(), + ssh_url_to_repo: z.string().nullable().optional(), + http_url_to_repo: z.string().nullable().optional(), + }), + ); + projectPaths.set(key, source); + return source; + } + + async function resolveExpectedProjectId(input: { + cwd: string; + expectedPath: string; + mergeRequests: GitLabMergeRequest[]; + force: boolean; + endpoint?: import("./forge-service.js").HeadRepositoryEndpoint; + }): Promise { + const remote = await resolveRemoteUrl(input.cwd); + const originPath = remote ? parseGitRemoteLocation(remote)?.path : null; + const targetId = + input.expectedPath.toLowerCase() === originPath?.toLowerCase() + ? input.mergeRequests.find( + (mr) => + extractProjectPath(mr.references?.full)?.toLowerCase() === + input.expectedPath.toLowerCase(), + )?.target_project_id + : undefined; + if (!input.endpoint && targetId != null) return targetId; + const project = await readProject(input.cwd, input.expectedPath, input.force); + if ( + !matchesHeadRepositoryEndpoint(input.endpoint, [ + project.ssh_url_to_repo, + project.http_url_to_repo, + ]) + ) + return null; + return project.id ?? null; + } + async function resolveCurrentMergeRequest( cwd: string, headRef: string, - headSha?: string, + headRepositoryPath?: string, + headRepositoryOwner?: string, + changeRequestNumber?: number, + force = false, + headRepositoryEndpoint?: import("./forge-service.js").HeadRepositoryEndpoint, ): Promise { const mergeRequests = await listMergeRequestsBySourceBranch(cwd, headRef); - const candidates = mergeRequests.filter((mr) => mr.source_branch === headRef); + if (mergeRequests.length === 0) return null; + const expectedPath = await resolveExpectedHeadProject( + cwd, + headRepositoryPath, + headRepositoryOwner, + changeRequestNumber, + force, + ); + if (!expectedPath) return null; + const expectedProjectId = await resolveExpectedProjectId({ + cwd, + expectedPath, + mergeRequests, + force, + endpoint: headRepositoryEndpoint, + }); + if (expectedProjectId == null) return null; + const candidates: GitLabMergeRequest[] = []; + for (const mr of mergeRequests) { + if ( + mr.source_branch !== headRef || + mr.source_project_id == null || + mr.target_project_id == null + ) + continue; + if (mr.source_project_id === expectedProjectId) { + if (mapMergeRequestState(mr.state) === "open") return viewMergeRequest(cwd, String(mr.iid)); + candidates.push(mr); + } + } + candidates.sort( + (a, b) => + parseGitLabTimestamp(b.updated_at) - parseGitLabTimestamp(a.updated_at) || b.iid - a.iid, + ); const match = - candidates.find((mr) => mapMergeRequestState(mr.state) === "open") ?? - candidates.find( - (mr) => - mapMergeRequestState(mr.state) !== "open" && headSha !== undefined && mr.sha === headSha, - ) ?? - null; + candidates.find((mr) => mapMergeRequestState(mr.state) === "open") ?? candidates[0] ?? null; + if (mergeRequests.length >= 100 && (!match || mapMergeRequestState(match.state) !== "open")) + throw new ForgeDiscoveryIncompleteError( + "GitLab merge request discovery exceeded its candidate window", + ); return match ? viewMergeRequest(cwd, String(match.iid)) : null; } @@ -1151,7 +1266,15 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F async getCurrentPullRequestStatus(input): Promise { try { - const mr = await resolveCurrentMergeRequest(input.cwd, input.headRef, input.headSha); + const mr = await resolveCurrentMergeRequest( + input.cwd, + input.headRef, + input.headRepositoryPath, + input.headRepositoryOwner, + input.changeRequestNumber, + input.force === true, + input.headRepositoryEndpoint, + ); if (!mr) { return null; } @@ -1448,6 +1571,8 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F return { success: true }; }, - invalidate(_input: { cwd: string }): void {}, + invalidate(_input: { cwd: string }): void { + projectPaths.clear(); + }, }; } diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index bd52b0e49f..514c82ef5c 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -47,10 +47,11 @@ import { isDescendantPath, warmCheckoutShortstatInBackground, } from "./checkout-git.js"; -import { startGitCommandMetrics, stopGitCommandMetrics } from "./run-git-command.js"; +import { runGitCommand, startGitCommandMetrics, stopGitCommandMetrics } from "./run-git-command.js"; import { createForgeResolver } from "../services/forge-resolver.js"; import { GitHubCommandError, GitHubCliMissingError } from "../services/github-service.js"; import type { CurrentPullRequestStatus, ForgeService } from "../services/forge-service.js"; +import { ForgeDiscoveryIncompleteError } from "../services/forge-service.js"; import { TeaAuthenticationError, TeaCliMissingError, @@ -178,6 +179,10 @@ interface RequestedPullRequestTarget { headRef: string; headSha?: string; headRepositoryOwner?: string; + headRepositoryPath?: string; + headRepositoryHost?: string; + headRepositoryEndpoint?: import("../services/forge-service.js").HeadRepositoryEndpoint; + changeRequestNumber?: number; } interface RecordingPullRequestTargetsOptions { @@ -194,6 +199,12 @@ function createGitHubServiceRecordingPullRequestTargets( headRef: request.headRef, ...(request.headSha ? { headSha: request.headSha } : {}), ...(request.headRepositoryOwner ? { headRepositoryOwner: request.headRepositoryOwner } : {}), + ...(request.headRepositoryPath ? { headRepositoryPath: request.headRepositoryPath } : {}), + ...(request.headRepositoryHost ? { headRepositoryHost: request.headRepositoryHost } : {}), + ...(request.headRepositoryEndpoint + ? { headRepositoryEndpoint: request.headRepositoryEndpoint } + : {}), + ...(request.changeRequestNumber ? { changeRequestNumber: request.changeRequestNumber } : {}), }); return createPullRequestStatus({ ...options.statusOverrides, @@ -1085,6 +1096,9 @@ const x = 1; } expect(status.aheadOfOrigin).toBeNull(); expect(status.behindOfOrigin).toBeNull(); + const facts = await getCheckoutSnapshotFacts(repoDir); + expect(facts.isGit && facts.upstreamKnown).toBe(true); + expect(facts.isGit && facts.upstreamStatus).toBeNull(); }); it("does not report full history as unpushed for fresh no-track Paseo worktrees", async () => { @@ -3205,6 +3219,124 @@ const x = 1; }); }); + it("carries the full nested head repository path from the tracking remote", async () => { + execFileSync( + "git", + ["remote", "add", "origin", "https://gitlab.example.com/base/project.git"], + { cwd: repoDir }, + ); + execFileSync( + "git", + ["remote", "add", "fork", "https://gitlab.example.com/team/subgroup/renamed.git"], + { cwd: repoDir }, + ); + execFileSync("git", ["checkout", "-b", "topic"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.topic.remote", "fork"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.topic.merge", "refs/heads/topic"], { cwd: repoDir }); + expect(await readPullRequestLookupTargetFromFacts(repoDir, paseoHome)).toMatchObject({ + headRef: "topic", + headRepositoryPath: "team/subgroup/renamed", + }); + }); + + it("does not associate a tracked branch from a different forge host", async () => { + execFileSync("git", ["remote", "add", "origin", "https://github.com/acme/project.git"], { + cwd: repoDir, + }); + execFileSync("git", ["remote", "add", "other", "https://gitea.example.com/acme/project.git"], { + cwd: repoDir, + }); + execFileSync("git", ["checkout", "-b", "topic"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.topic.remote", "other"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.topic.merge", "refs/heads/topic"], { cwd: repoDir }); + const targets: RequestedPullRequestTarget[] = []; + const forge = createGitHubServiceRecordingPullRequestTargets({ requestedTargets: targets }); + const facts = await getCheckoutSnapshotFacts(repoDir, { paseoHome }); + expect( + (await getPullRequestStatus(repoDir, forge, undefined, { facts, paseoHome })).status, + ).toBeNull(); + expect(targets).toEqual([]); + }); + + it.each(["https://forge.example:4000/acme/project.git"])( + "does not infer a forge instance for an unverified endpoint %s", + async (headRemote) => { + execFileSync( + "git", + ["remote", "add", "origin", "https://forge.example:3000/acme/project.git"], + { cwd: repoDir }, + ); + execFileSync("git", ["remote", "add", "other", headRemote], { cwd: repoDir }); + execFileSync("git", ["checkout", "-b", "topic"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.topic.remote", "other"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.topic.merge", "refs/heads/topic"], { cwd: repoDir }); + const facts = await getCheckoutSnapshotFacts(repoDir, { + paseoHome, + resolveSshHostname: async (host) => host, + }); + const targets: RequestedPullRequestTarget[] = []; + const forge = createGitHubServiceRecordingPullRequestTargets({ requestedTargets: targets }); + expect( + (await getPullRequestStatus(repoDir, forge, undefined, { facts, paseoHome })).status, + ).toBeNull(); + expect(targets).toEqual([]); + }, + ); + + it("passes a normalized mixed-transport endpoint to the forge for proof", async () => { + execFileSync( + "git", + ["remote", "add", "origin", "https://forge.example:3000/acme/project.git"], + { cwd: repoDir }, + ); + execFileSync("git", ["remote", "add", "other", "ssh://git@work-alias:2222/acme/project.git"], { + cwd: repoDir, + }); + execFileSync("git", ["checkout", "-b", "topic"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.topic.remote", "other"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.topic.merge", "refs/heads/topic"], { cwd: repoDir }); + const facts = await getCheckoutSnapshotFacts(repoDir, { + paseoHome, + resolveSshHostname: async () => "forge.example", + }); + const targets: RequestedPullRequestTarget[] = []; + const forge = createGitHubServiceRecordingPullRequestTargets({ requestedTargets: targets }); + await getPullRequestStatus(repoDir, forge, undefined, { facts, paseoHome }); + expect(targets).toEqual([ + expect.objectContaining({ + headRepositoryEndpoint: { + transport: "ssh", + host: "forge.example", + port: "2222", + path: "acme/project", + }, + }), + ]); + }); + + it("recognizes an SSH alias for the same forge host", async () => { + execFileSync("git", ["remote", "add", "origin", "https://github.com/acme/project.git"], { + cwd: repoDir, + }); + execFileSync("git", ["remote", "add", "fork", "git@work-alias:acme/project.git"], { + cwd: repoDir, + }); + execFileSync("git", ["checkout", "-b", "topic"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.topic.remote", "fork"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.topic.merge", "refs/heads/topic"], { cwd: repoDir }); + const context = { + paseoHome, + resolveSshHostname: async (host: string) => (host === "work-alias" ? "github.com" : host), + }; + const facts = await getCheckoutSnapshotFacts(repoDir, context); + const targets: RequestedPullRequestTarget[] = []; + const forge = createGitHubServiceRecordingPullRequestTargets({ requestedTargets: targets }); + expect( + (await getPullRequestStatus(repoDir, forge, undefined, { ...context, facts })).status, + ).not.toBeNull(); + expect(targets).toHaveLength(1); + }); + it.each([ "git@github.com:contributor/paseo.git", "https://github.com/contributor/paseo.git", @@ -3231,6 +3363,8 @@ const x = 1; expect(await readPullRequestLookupTargetFromFacts(repoDir, paseoHome)).toEqual({ headRef: "topic", headRepositoryOwner: "contributor", + headRepositoryPath: "contributor/paseo", + headRepositoryHost: "github.com:", headSha, }); }); @@ -3472,6 +3606,22 @@ const x = 1; expect(callCount).toBe(1); }); + it("distinguishes a failed upstream read from a successful missing upstream", async () => { + const known = await getCheckoutSnapshotFacts(repoDir, { paseoHome }); + expect(known.isGit && known.upstreamKnown).toBe(true); + expect(known.isGit && known.upstreamStatus).toBeNull(); + const failed = await getCheckoutSnapshotFacts(repoDir, { + paseoHome, + runGitCommand: async (args, options) => { + if (args[0] === "for-each-ref" && args.some((arg) => arg.includes("%(upstream:track"))) + throw new Error("upstream unavailable"); + return runGitCommand(args, options); + }, + }); + expect(failed.isGit && failed.upstreamKnown).toBe(false); + expect(failed.isGit && failed.upstreamStatus).toBeNull(); + }); + it("does not reuse a PR status cache entry after HEAD changes on the same branch", async () => { execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir }); execFileSync("git", ["remote", "add", "origin", "https://github.com/getpaseo/paseo.git"], { @@ -3499,6 +3649,29 @@ const x = 1; expect(requestedShas[0]).not.toBe(requestedShas[1]); }); + it("does not reuse a PR status after changing branch or head repository at the same commit", async () => { + execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir }); + execFileSync("git", ["remote", "add", "origin", "https://github.com/acme/first.git"], { + cwd: repoDir, + }); + const targets: RequestedPullRequestTarget[] = []; + const forge = createGitHubServiceRecordingPullRequestTargets({ requestedTargets: targets }); + await getPullRequestStatus(repoDir, forge); + execFileSync("git", ["checkout", "-b", "other"], { cwd: repoDir }); + await getPullRequestStatus(repoDir, forge); + execFileSync("git", ["remote", "set-url", "origin", "https://github.com/acme/second.git"], { + cwd: repoDir, + }); + await getPullRequestStatus(repoDir, forge); + expect(targets.map(({ headRef, headRepositoryPath }) => [headRef, headRepositoryPath])).toEqual( + [ + ["feature", "acme/first"], + ["other", "acme/first"], + ["other", "acme/second"], + ], + ); + }); + it("passes forced PR status reads through to the GitHub service", async () => { execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir }); execFileSync("git", ["remote", "add", "origin", "https://github.com/getpaseo/paseo.git"], { @@ -3627,6 +3800,78 @@ const x = 1; } }); + it("preserves known status on incomplete discovery but refuses a forced archive read", async () => { + execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir }); + execFileSync("git", ["remote", "add", "origin", "https://gitea.example.com/acme/repo.git"], { + cwd: repoDir, + }); + __setPullRequestStatusCacheTtlForTests(50); + try { + let calls = 0; + const service = createGitHubServiceForStatus(null); + service.getCurrentPullRequestStatus = async () => { + if (calls++ === 0) return createPullRequestStatus(); + throw new ForgeDiscoveryIncompleteError("candidate window full"); + }; + const fresh = await getPullRequestStatus(repoDir, service); + await sleep(80); + expect(await getPullRequestStatus(repoDir, service)).toEqual(fresh); + await expect( + getPullRequestStatus(repoDir, service, { force: true, reason: "archive" }), + ).rejects.toBeInstanceOf(ForgeDiscoveryIncompleteError); + } finally { + __resetPullRequestStatusCacheForTests(); + } + }); + + it("passes the last observed request to later status reads for revalidation", async () => { + execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir }); + execFileSync("git", ["remote", "add", "origin", "https://gitea.example.com/acme/repo.git"], { + cwd: repoDir, + }); + __setPullRequestStatusCacheTtlForTests(50); + try { + const seen: Array<{ number?: number; url: string; state: string } | null | undefined> = []; + const service = createGitHubServiceForStatus(null); + service.getCurrentPullRequestStatus = async (request) => { + seen.push(request.previousPullRequest); + return createPullRequestStatus({ + number: 2, + url: "https://gitea.example.com/acme/repo/pulls/2", + }); + }; + await getPullRequestStatus(repoDir, service); + await sleep(80); + await getPullRequestStatus(repoDir, service); + await getPullRequestStatus(repoDir, service, { force: true, reason: "archive" }); + expect(seen[0]).toBeUndefined(); + expect(seen.slice(1)).toEqual([ + expect.objectContaining({ number: 2, state: "open" }), + expect.objectContaining({ number: 2, state: "open" }), + ]); + } finally { + __resetPullRequestStatusCacheForTests(); + } + }); + + it("uses the workspace association instead of a standalone cached result", async () => { + execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir }); + execFileSync("git", ["remote", "add", "origin", "https://gitea.example.com/acme/repo.git"], { + cwd: repoDir, + }); + const service = createGitHubServiceForStatus(null); + const open = { number: 2, url: "https://gitea.example.com/acme/repo/pulls/2", state: "open" }; + service.getCurrentPullRequestStatus = async (input) => + createPullRequestStatus( + input.previousPullRequest?.number === 2 ? open : { number: 1, state: "merged" }, + ); + expect((await getPullRequestStatus(repoDir, service)).status?.number).toBe(1); + expect( + (await getPullRequestStatus(repoDir, service, { previousPullRequest: open })).status?.number, + ).toBe(2); + expect((await getPullRequestStatus(repoDir, service)).status?.number).toBe(1); + }); + it("does not use stale PR status fallback for forced GitHub errors", async () => { execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir }); execFileSync("git", ["remote", "add", "origin", "https://github.com/getpaseo/paseo.git"], { diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index 358bd6eb69..e72afe1479 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -14,7 +14,7 @@ import { tmpdir } from "node:os"; import { setImmediate } from "node:timers/promises"; import { TTLCache } from "@isaacs/ttlcache"; import type { CheckoutCommit, CheckoutCommitFile } from "@getpaseo/protocol/messages"; -import { parseGitHubRemoteIdentity, parseGitRemoteLocation } from "@getpaseo/protocol/git-remote"; +import { parseGitRemoteLocation } from "@getpaseo/protocol/git-remote"; import { maxSecuredPlaintextByteLength } from "@getpaseo/relay"; import type { Logger } from "pino"; import type { ParsedDiffFile } from "../server/utils/diff-highlighter.js"; @@ -23,8 +23,8 @@ import { parseAndHighlightDiff, parseDiff, } from "../server/utils/diff-highlighter.js"; -import { parseGitHubRepoFromRemote } from "../server/workspace-git-metadata.js"; import { createGitHubService } from "../services/github-service.js"; +import { ForgeDiscoveryIncompleteError } from "../services/forge-service.js"; import type { CurrentPullRequestStatus, ForgeAuthState, @@ -39,6 +39,7 @@ import { ForgeCommandError, } from "../services/forge-cli-command.js"; import { parseGitRevParsePath, resolveGitRevParsePath } from "./git-rev-parse-path.js"; +import { resolveSshHostname } from "./ssh-hostname.js"; import { runGitCommand, type RunGitCommand } from "./run-git-command.js"; import { readGitFileContents } from "./git-file-contents.js"; import { unquoteGitPath } from "./git-quoted-path.js"; @@ -97,12 +98,19 @@ const shortstatInFlight = new Map>(); interface CheckoutReadCacheOptions { force?: boolean; reason?: string; + previousPullRequest?: { number?: number; url: string; state: string } | null; } interface PullRequestStatusLookupTarget { headRef: string; headSha?: string; headRepositoryOwner?: string; + headRepositoryPath?: string; + headRepositoryHost?: string; + headRepositoryEndpoint?: import("../services/forge-service.js").HeadRepositoryEndpoint; + headRepositoryRemoteUrl?: string; + forgeHost?: string; + changeRequestNumber?: number; } interface PullRequestLookupTargetBranchConfig { @@ -151,8 +159,22 @@ function createShortstatCache(ttlMs: number) { }); } -function getPullRequestStatusCacheKey(cwd: string, headSha: string | null): string { - return `${resolve(cwd)}\u0000${headSha ?? ""}`; +function getPullRequestStatusCacheKey( + cwd: string, + remoteUrl: string | null, + target: PullRequestStatusLookupTarget, +): string { + return JSON.stringify([ + resolve(cwd), + remoteUrl, + target.headRef, + target.headSha ?? null, + target.headRepositoryPath ?? null, + target.headRepositoryHost ?? null, + target.headRepositoryEndpoint ?? null, + target.headRepositoryOwner ?? null, + target.changeRequestNumber ?? null, + ]); } function rememberPullRequestStatus(cacheKey: string, status: PullRequestStatusResult): void { @@ -944,6 +966,7 @@ export interface CheckoutContext { logger?: Pick; facts?: CheckoutSnapshotFacts | null; runGitCommand?: RunGitCommand; + resolveSshHostname?: typeof resolveSshHostname; } export type CheckoutSnapshotFacts = @@ -965,6 +988,7 @@ export type CheckoutSnapshotFacts = branchRemoteName: string | null; branchMergeRef: string | null; upstreamStatus: UpstreamStatus | null; + upstreamKnown?: boolean; pullRequestLookupTarget: PullRequestStatusLookupTarget | null; }; @@ -1484,25 +1508,6 @@ async function resolvePullRequestStatusLookupTarget( branchMergeRef = await getGitConfigValue(cwd, `branch.${currentBranch}.merge`, context); } - const localBranchTarget = buildPullRequestLookupTargetFromBranchConfig({ - currentBranch, - branchRemoteName, - branchMergeRef, - branchRemoteUrl: null, - originRemoteUrl: null, - resolvedBaseRef: null, - }); - if (localBranchTarget.headRef === currentBranch) { - const pushTarget = await resolvePullRequestLookupTargetFromPushConfig( - cwd, - currentBranch, - null, - null, - context, - ); - return pushTarget ?? localBranchTarget; - } - const [branchRemoteUrl, originRemoteUrl, resolvedBaseRef] = await Promise.all([ branchRemoteName ? resolveGitRemoteUrl({ cwd, remote: branchRemoteName, context }) : null, getGitConfigValue(cwd, "remote.origin.url", context), @@ -1822,7 +1827,7 @@ async function getUpstreamStatus( cwd: string, currentBranch: string, context?: CheckoutContext, -): Promise { +): Promise<{ status: UpstreamStatus | null; known: boolean }> { try { const { stdout } = await getRunGitCommand(context)( [ @@ -1834,13 +1839,13 @@ async function getUpstreamStatus( ); const [ref = "", track = ""] = stdout.trim().split("\0", 2); if (!ref || track === "gone") { - return null; + return { status: null, known: true }; } const ahead = Number.parseInt(track.match(/ahead (\d+)/)?.[1] ?? "0", 10); const behind = Number.parseInt(track.match(/behind (\d+)/)?.[1] ?? "0", 10); - return { ref, aheadBehind: { ahead, behind } }; + return { status: { ref, aheadBehind: { ahead, behind } }, known: true }; } catch { - return null; + return { status: null, known: false }; } } @@ -1889,12 +1894,14 @@ function buildPullRequestLookupTargetFromBranchConfig( ): PullRequestStatusLookupTarget { const trackedHeadRef = parseBranchMergeHeadRef(input.branchMergeRef); if (!input.branchRemoteName || !trackedHeadRef) { - return { headRef: input.currentBranch }; + const origin = parseRepositoryIdentityFromRemote(input.originRemoteUrl); + return { headRef: input.currentBranch, ...(origin ? { headRepositoryPath: origin } : {}) }; } const remoteRepo = parseRepositoryIdentityFromRemote(input.branchRemoteUrl); + const remoteHost = getRepositoryHost(input.branchRemoteUrl); const originRepo = parseRepositoryIdentityFromRemote(input.originRemoteUrl); - const isSameRepo = areSameGitHubRepository(remoteRepo, originRepo); + const isSameRepo = areSameRepositoryPath(remoteRepo, originRepo); const headRepositoryOwner = remoteRepo && !isSameRepo ? remoteRepo.split("/")[0] : null; const normalizedBaseRef = input.resolvedBaseRef ? branchNameFromRef(input.resolvedBaseRef) : null; if ( @@ -1909,12 +1916,83 @@ function buildPullRequestLookupTargetFromBranchConfig( } if (isSameRepo) { - return { headRef: trackedHeadRef }; + return { + headRef: trackedHeadRef, + ...(remoteRepo ? { headRepositoryPath: remoteRepo } : {}), + ...(remoteHost ? { headRepositoryHost: remoteHost } : {}), + ...(input.branchRemoteUrl ? { headRepositoryRemoteUrl: input.branchRemoteUrl } : {}), + }; } return { headRef: trackedHeadRef, ...(headRepositoryOwner ? { headRepositoryOwner } : {}), + ...(remoteRepo ? { headRepositoryPath: remoteRepo } : {}), + ...(remoteHost ? { headRepositoryHost: remoteHost } : {}), + ...(input.branchRemoteUrl ? { headRepositoryRemoteUrl: input.branchRemoteUrl } : {}), + }; +} + +function getRepositoryHost(remoteUrl: string | null): string | null { + const location = remoteUrl ? parseGitRemoteLocation(remoteUrl) : null; + return location ? `${location.host}:${location.port ?? ""}` : null; +} + +export function isPullRequestTargetOnForge( + remoteUrl: string | null, + target: PullRequestStatusLookupTarget, +): boolean { + return ( + !target.headRepositoryHost || + target.headRepositoryHost === (target.forgeHost ?? getRepositoryHost(remoteUrl)) || + Boolean(target.headRepositoryEndpoint) + ); +} + +async function resolveRepositoryHost( + remoteUrl: string | null, + resolveHost: typeof resolveSshHostname, +): Promise { + const location = remoteUrl ? parseGitRemoteLocation(remoteUrl) : null; + if (!location) return null; + const host = + location.transport === "ssh" || location.transport === "scp" + ? ((await resolveHost(location.host)) ?? location.host) + : location.host; + return `${host}:${location.port ?? ""}`; +} + +async function normalizePullRequestTargetHost( + target: PullRequestStatusLookupTarget, + remoteUrl: string | null, + context?: CheckoutContext, +): Promise { + if (!target.headRepositoryRemoteUrl) return target; + const resolveHost = context?.resolveSshHostname ?? resolveSshHostname; + const [headRepositoryHost, forgeHost] = await Promise.all([ + resolveRepositoryHost(target.headRepositoryRemoteUrl, resolveHost), + resolveRepositoryHost(remoteUrl, resolveHost), + ]); + const { headRepositoryRemoteUrl: _headRepositoryRemoteUrl, ...normalized } = target; + const head = parseGitRemoteLocation(target.headRepositoryRemoteUrl); + const origin = remoteUrl ? parseGitRemoteLocation(remoteUrl) : null; + const headHost = headRepositoryHost?.split(":")[0]; + const originHost = forgeHost?.split(":")[0]; + const isSsh = (transport: string) => transport === "ssh" || transport === "scp"; + const endpoint = + head && + origin && + headHost && + headHost === originHost && + headRepositoryHost !== forgeHost && + isSsh(head.transport) !== isSsh(origin.transport) + ? { ...head, host: headHost } + : undefined; + return { + ...normalized, + ...(endpoint ? { headRepositoryEndpoint: endpoint } : {}), + ...(headRepositoryHost ? { headRepositoryHost } : {}), + ...(forgeHost && forgeHost !== getRepositoryHost(remoteUrl) ? { forgeHost } : {}), }; } @@ -1923,10 +2001,10 @@ function parseRepositoryIdentityFromRemote(remoteUrl: string | null): string | n return null; } const location = parseGitRemoteLocation(remoteUrl); - return location ? (parseGitHubRemoteIdentity(location.path)?.repo ?? null) : null; + return location?.path ?? null; } -function areSameGitHubRepository(left: string | null, right: string | null): boolean { +function areSameRepositoryPath(left: string | null, right: string | null): boolean { return Boolean(left && right && left.toLowerCase() === right.toLowerCase()); } @@ -1949,16 +2027,17 @@ function doesLocalBranchNameIdentifyTrackedHead( function buildPullRequestLookupTargetFromPushConfig( input: PullRequestLookupTargetPushConfig, ): PullRequestStatusLookupTarget | null { - const pushedHeadRef = parseHeadPushRefspec(input.pushRefspec); - if (!input.pushRemoteName || !pushedHeadRef || pushedHeadRef === input.currentBranch) { + const configuredHeadRef = parseHeadPushRefspec(input.pushRefspec); + if (input.pushRefspec && !configuredHeadRef) return null; + const pushedHeadRef = configuredHeadRef ?? input.currentBranch; + if (!input.pushRemoteName) { return null; } - const remoteRepo = input.pushRemoteUrl ? parseGitHubRepoFromRemote(input.pushRemoteUrl) : null; - const originRepo = input.originRemoteUrl - ? parseGitHubRepoFromRemote(input.originRemoteUrl) - : null; - const isSameRepo = areSameGitHubRepository(remoteRepo, originRepo); + const remoteRepo = parseRepositoryIdentityFromRemote(input.pushRemoteUrl); + const remoteHost = getRepositoryHost(input.pushRemoteUrl); + const originRepo = parseRepositoryIdentityFromRemote(input.originRemoteUrl); + const isSameRepo = areSameRepositoryPath(remoteRepo, originRepo); const headRepositoryOwner = remoteRepo && !isSameRepo ? remoteRepo.split("/")[0] : null; const normalizedBaseRef = input.resolvedBaseRef ? branchNameFromRef(input.resolvedBaseRef) : null; if (pushedHeadRef === normalizedBaseRef && !headRepositoryOwner) { @@ -1968,6 +2047,9 @@ function buildPullRequestLookupTargetFromPushConfig( return { headRef: pushedHeadRef, ...(headRepositoryOwner ? { headRepositoryOwner } : {}), + ...(remoteRepo ? { headRepositoryPath: remoteRepo } : {}), + ...(remoteHost ? { headRepositoryHost: remoteHost } : {}), + ...(input.pushRemoteUrl ? { headRepositoryRemoteUrl: input.pushRemoteUrl } : {}), }; } @@ -1982,6 +2064,7 @@ function buildPullRequestLookupTargetFromMetadata( return { headRef: target.headRef, ...(target.headRepositoryOwner ? { headRepositoryOwner: target.headRepositoryOwner } : {}), + ...(target.changeRequestNumber ? { changeRequestNumber: target.changeRequestNumber } : {}), }; } @@ -2053,7 +2136,7 @@ async function resolvePullRequestLookupTargetFromPushConfig( }); } -async function resolveFactsPullRequestLookupTarget(input: { +interface PullRequestFactsLookupInput { cwd: string; inspected: CheckoutInspectionContext; metadata: PaseoWorktreeMetadata | null; @@ -2062,13 +2145,56 @@ async function resolveFactsPullRequestLookupTarget(input: { branchRemoteUrl: string | null; resolvedBaseRef: string | null; context?: CheckoutContext; -}): Promise { +} + +async function resolveMetadataPullRequestTarget( + input: PullRequestFactsLookupInput, + metadataTarget: PullRequestStatusLookupTarget, + currentBranch: string, +): Promise { + const { cwd, inspected, context } = input; + const pushTarget = await resolvePullRequestLookupTargetFromPushConfig( + cwd, + currentBranch, + inspected.remoteUrl, + input.resolvedBaseRef, + context, + ); + if (pushTarget?.headRef === metadataTarget.headRef && pushTarget.headRepositoryPath) { + return { ...pushTarget, changeRequestNumber: metadataTarget.changeRequestNumber }; + } + const trackedHeadMatches = + parseBranchMergeHeadRef(input.branchMergeRef) === metadataTarget.headRef; + let remoteUrl = trackedHeadMatches + ? (input.branchRemoteUrl ?? inspected.remoteUrl) + : inspected.remoteUrl; + if (metadataTarget.headRepositoryOwner) { + remoteUrl = + !metadataTarget.changeRequestNumber && trackedHeadMatches ? input.branchRemoteUrl : null; + } + const remotePath = parseRepositoryIdentityFromRemote(remoteUrl); + const remoteHost = getRepositoryHost(remoteUrl); + return remotePath && + (!metadataTarget.headRepositoryOwner || + remotePath.split("/")[0] === metadataTarget.headRepositoryOwner) + ? { + ...metadataTarget, + headRepositoryPath: remotePath, + ...(remoteHost ? { headRepositoryHost: remoteHost } : {}), + ...(remoteUrl ? { headRepositoryRemoteUrl: remoteUrl } : {}), + } + : metadataTarget; +} + +async function resolveFactsPullRequestLookupTarget( + input: PullRequestFactsLookupInput, +): Promise { const { cwd, inspected, metadata, context } = input; const metadataTarget = inspected.currentBranch ? buildPullRequestLookupTargetFromMetadata(metadata, inspected.currentBranch) : null; - if (metadataTarget) { - return metadataTarget; + if (metadataTarget && inspected.currentBranch) { + return resolveMetadataPullRequestTarget(input, metadataTarget, inspected.currentBranch); } let target = buildInitialPullRequestLookupTarget({ @@ -2093,6 +2219,10 @@ async function resolveFactsPullRequestLookupTarget(input: { context, )) ?? target; } + if (target && !target.headRepositoryPath && !target.headRepositoryOwner) { + const origin = parseRepositoryIdentityFromRemote(inspected.remoteUrl); + if (origin) target = { ...target, headRepositoryPath: origin }; + } return target; } @@ -2135,7 +2265,7 @@ export async function getCheckoutSnapshotFacts( let branchRemoteUrl: string | null = null; const upstreamStatusPromise = inspected.currentBranch ? getUpstreamStatus(cwd, inspected.currentBranch, context) - : Promise.resolve(null); + : Promise.resolve({ status: null, known: false }); if (inspected.currentBranch) { branchRemoteName = await getGitConfigValue( cwd, @@ -2166,7 +2296,14 @@ export async function getCheckoutSnapshotFacts( pullRequestLookupTarget, context, ); - const upstreamStatus = await upstreamStatusPromise; + if (pullRequestLookupTarget) { + pullRequestLookupTarget = await normalizePullRequestTargetHost( + pullRequestLookupTarget, + inspected.remoteUrl, + context, + ); + } + const upstream = await upstreamStatusPromise; return { isGit: true, @@ -2182,7 +2319,8 @@ export async function getCheckoutSnapshotFacts( comparisonBaseRef, branchRemoteName, branchMergeRef, - upstreamStatus, + upstreamStatus: upstream.status, + upstreamKnown: upstream.known, pullRequestLookupTarget, }; } @@ -4307,14 +4445,49 @@ export async function createPullRequest( return result; } +async function resolvePullRequestStatusRead( + cwd: string, + context?: CheckoutContext, +): Promise<{ target: PullRequestStatusLookupTarget; remoteUrl: string | null } | null> { + if (!context?.facts?.isGit) await requireGitRepo(cwd, context); + const head = context?.facts?.isGit + ? context.facts.currentBranch + : await getCurrentBranch(cwd, context); + if (!head) return null; + const headSha = await getCurrentHeadSha(cwd, context); + const resolvedTarget = await resolvePullRequestStatusLookupTarget(cwd, head, context); + let target = headSha && !resolvedTarget.headSha ? { ...resolvedTarget, headSha } : resolvedTarget; + const remoteUrl = context?.facts?.isGit + ? context.facts.remoteUrl + : await getOriginRemoteUrl(cwd, context); + if (!context?.facts?.isGit) + target = await normalizePullRequestTargetHost(target, remoteUrl, context); + return { target, remoteUrl }; +} + export async function getPullRequestStatus( cwd: string, forgeService: ForgeService = createGitHubService(), options?: CheckoutReadCacheOptions, context?: CheckoutContext, ): Promise { - const headSha = await getCurrentHeadSha(cwd, context); - const cacheKey = getPullRequestStatusCacheKey(cwd, headSha); + const unavailable = getUnavailablePullRequestStatus(context?.facts); + if (unavailable) return unavailable; + const read = await resolvePullRequestStatusRead(cwd, context); + if (!read) return buildPullRequestStatusResult(null, "no_remote"); + const { target, remoteUrl } = read; + if (!isPullRequestTargetOnForge(remoteUrl, target)) + return buildPullRequestStatusResult(null, "authenticated"); + if (options && "previousPullRequest" in options) { + return getPullRequestStatusUncached( + cwd, + forgeService, + target, + options, + options.previousPullRequest, + ); + } + const cacheKey = getPullRequestStatusCacheKey(cwd, remoteUrl, target); if (!options?.force) { const cached = pullRequestStatusCache.get(cacheKey); if (cached) { @@ -4327,14 +4500,23 @@ export async function getPullRequestStatus( } } - const lookup = getPullRequestStatusUncached(cwd, forgeService, options, context, headSha) + const lookup = getPullRequestStatusUncached( + cwd, + forgeService, + target, + options, + lastSuccessfulPullRequestStatus.get(cacheKey)?.status, + ) .then((status) => { pullRequestStatusCache.set(cacheKey, status); rememberPullRequestStatus(cacheKey, status); return status; }) .catch((error) => { - if (!options?.force && error instanceof ForgeCommandError) { + if ( + !options?.force && + (error instanceof ForgeCommandError || error instanceof ForgeDiscoveryIncompleteError) + ) { const stale = lastSuccessfulPullRequestStatus.get(cacheKey); if (stale) { return stale; @@ -4353,27 +4535,11 @@ export async function getPullRequestStatus( async function getPullRequestStatusUncached( cwd: string, forgeService: ForgeService, + lookupTarget: PullRequestStatusLookupTarget, options?: CheckoutReadCacheOptions, - context?: CheckoutContext, - headSha?: string | null, + previousPullRequest?: { number?: number; url: string; state: string } | null, ): Promise { - const unavailable = getUnavailablePullRequestStatus(context?.facts); - if (unavailable) return unavailable; - if (!context?.facts?.isGit) { - await requireGitRepo(cwd, context); - } - const head = context?.facts?.isGit - ? context.facts.currentBranch - : await getCurrentBranch(cwd, context); - if (!head) { - return buildPullRequestStatusResult(null, "no_remote"); - } try { - const resolvedLookupTarget = await resolvePullRequestStatusLookupTarget(cwd, head, context); - const lookupTarget = - headSha && !resolvedLookupTarget.headSha - ? { ...resolvedLookupTarget, headSha } - : resolvedLookupTarget; let status: CurrentPullRequestStatus | null; if (options?.force) { const reason = options.reason; @@ -4383,6 +4549,7 @@ async function getPullRequestStatusUncached( status = await forgeService.getCurrentPullRequestStatus({ cwd, ...lookupTarget, + ...(previousPullRequest ? { previousPullRequest } : {}), force: true, reason, }); @@ -4390,6 +4557,7 @@ async function getPullRequestStatusUncached( status = await forgeService.getCurrentPullRequestStatus({ cwd, ...lookupTarget, + ...(previousPullRequest ? { previousPullRequest } : {}), reason: options?.reason, }); }