diff --git a/docs/forge-providers.md b/docs/forge-providers.md index f633a1922..6cc4c518e 100644 --- a/docs/forge-providers.md +++ b/docs/forge-providers.md @@ -84,6 +84,20 @@ 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: + +- 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. + +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. + 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 host that is either a known cloud host or one the CLI is already authenticated diff --git a/packages/app/src/git/pull-request-panel/pane.tsx b/packages/app/src/git/pull-request-panel/pane.tsx index 3aba5bc89..582d9543a 100644 --- a/packages/app/src/git/pull-request-panel/pane.tsx +++ b/packages/app/src/git/pull-request-panel/pane.tsx @@ -411,6 +411,7 @@ export function PullRequestPane({ repoName: data.repoName, checkRunId: ref.checkRunId, workflowRunId: ref.workflowRunId, + changeRequestNumber: data.number, }; // COMPAT(githubCheckDetailsRpc): added in v0.1.106, remove after 2026-12-28 once // all supported clients use checkout.forge.get_check_details.*. diff --git a/packages/protocol/src/messages.ts b/packages/protocol/src/messages.ts index f1338bd45..a566f8e5c 100644 --- a/packages/protocol/src/messages.ts +++ b/packages/protocol/src/messages.ts @@ -1762,9 +1762,9 @@ const CheckoutCheckDetailsRequestPayloadSchema = z.object({ checkRunId: z.number().int().positive().optional(), workflowRunId: z.number().int().positive().optional(), // Permanent forge-routing field, optional because only some forges need it: - // GitLab routes the check-details fetch to the change request's head pipeline - // by iid, so a fork/detached MR pipeline (which lives in the source project, - // not the checkout's target project) resolves correctly. GitHub ignores it. + // GitLab routes check details to the MR's head pipeline; Gitea-family adapters + // resolve the PR head SHA by number, including after merge/close. GitHub + // ignores it. changeRequestNumber: z.number().int().positive().optional(), requestId: z.string(), }); diff --git a/packages/server/src/server/session/checkout/checkout-session.ts b/packages/server/src/server/session/checkout/checkout-session.ts index 0b97961a0..3505a5b12 100644 --- a/packages/server/src/server/session/checkout/checkout-session.ts +++ b/packages/server/src/server/session/checkout/checkout-session.ts @@ -1220,7 +1220,8 @@ export class CheckoutSession { try { // The payload schema keeps checkRunId and workflowRunId optional (a Gitea - // Actions run has no check-run id; GitLab routes by changeRequestNumber), + // Actions run has no check-run id; forge adapters may also route by + // changeRequestNumber), // but a request that addresses no check at all is not actionable — reject // it here with a clear message instead of failing deep in an adapter. The // schema itself cannot enforce this: it is a discriminated-union member, so 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 eb76955a3..99664c530 100644 --- a/packages/server/src/server/workspace-git-service.primitive.test.ts +++ b/packages/server/src/server/workspace-git-service.primitive.test.ts @@ -1144,7 +1144,10 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => { createCheckoutFacts(cwd, { currentBranch: "feature", remoteUrl: "https://forge-self-heal.test/acme/repo.git", - pullRequestLookupTarget: { headRef: "feature" }, + pullRequestLookupTarget: { + headRef: "feature", + headSha: "1111111111111111111111111111111111111111", + }, }), ); const getCheckoutStatus = vi.fn(async (cwd: string) => @@ -1163,12 +1166,13 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => { const subscription = service.registerWorkspace({ cwd: REPO_CWD }, listener); await flushPromises(); - await vi.advanceTimersByTimeAsync(60_000); + await vi.advanceTimersByTimeAsync(120_000); await flushPromises(); expect(forge.getCurrentPullRequestStatus).toHaveBeenCalledWith({ cwd: REPO_CWD, headRef: "feature", + headSha: "1111111111111111111111111111111111111111", reason: "self-heal-forge-pr-status", }); expect(listener).toHaveBeenCalledWith( @@ -1189,6 +1193,108 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => { } }); + test("generic forge self-heal uses the fast poll window while checks are pending", async () => { + const forge = { + ...createGitHubServiceStub(), + retainCurrentPullRequestStatusPoll: undefined, + getCurrentPullRequestStatus: vi.fn(async () => + createCurrentPullRequestStatus({ checksStatus: "pending" }), + ), + }; + const unregister = defaultForgeRegistry.register("forge-pending-test", { + createService: () => forge, + matchesHost: (host) => host === "forge-pending.test", + }); + const pendingResult = createPullRequestStatusResult(); + if (pendingResult.status) { + pendingResult.status.checksStatus = "pending"; + pendingResult.status.checks = [{ name: "ci", status: "pending" }]; + } + const service = createService({ + getCheckoutSnapshotFacts: vi.fn(async (cwd: string) => + createCheckoutFacts(cwd, { + currentBranch: "feature", + remoteUrl: "https://forge-pending.test/acme/repo.git", + pullRequestLookupTarget: { headRef: "feature" }, + }), + ), + getCheckoutStatus: vi.fn(async (cwd: string) => + createCheckoutStatus(cwd, { + currentBranch: "feature", + remoteUrl: "https://forge-pending.test/acme/repo.git", + }), + ), + getPullRequestStatus: vi.fn(async () => pendingResult), + }); + + try { + const subscription = service.registerWorkspace({ cwd: REPO_CWD }, vi.fn()); + await flushPromises(); + await vi.advanceTimersByTimeAsync(20_000); + await flushPromises(); + + expect(forge.getCurrentPullRequestStatus).toHaveBeenCalledTimes(1); + subscription.unsubscribe(); + } finally { + service.dispose(); + unregister(); + } + }); + + test("generic forge poll refreshes immediately when checkout HEAD changes", async () => { + let nowMs = 0; + let headSha = "1111111111111111111111111111111111111111"; + const forge = { + ...createGitHubServiceStub(), + retainCurrentPullRequestStatusPoll: undefined, + getCurrentPullRequestStatus: vi.fn(async () => createCurrentPullRequestStatus()), + }; + const unregister = defaultForgeRegistry.register("forge-head-change-test", { + createService: () => forge, + matchesHost: (host) => host === "forge-head-change.test", + }); + const service = createService({ + now: () => new Date(nowMs), + getCheckoutSnapshotFacts: vi.fn(async (cwd: string) => + createCheckoutFacts(cwd, { + currentBranch: "feature", + remoteUrl: "https://forge-head-change.test/acme/repo.git", + pullRequestLookupTarget: { headRef: "feature", headSha }, + }), + ), + getCheckoutStatus: vi.fn(async (cwd: string) => + createCheckoutStatus(cwd, { + currentBranch: "feature", + remoteUrl: "https://forge-head-change.test/acme/repo.git", + }), + ), + getPullRequestStatus: vi.fn(async () => createPullRequestStatusResult("Visible PR")), + }); + + try { + await service.getSnapshot(REPO_CWD); + const subscription = service.registerWorkspace({ cwd: REPO_CWD }, vi.fn()); + expect(service.peekSnapshot(REPO_CWD)?.forge.pullRequest?.title).toBe("Visible PR"); + + headSha = "2222222222222222222222222222222222222222"; + nowMs = 3_000; + await service.refresh(REPO_CWD); + + expect(service.peekSnapshot(REPO_CWD)?.forge.pullRequest).toBeNull(); + expect(forge.getCurrentPullRequestStatus).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(0); + await flushPromises(); + + expect(forge.getCurrentPullRequestStatus).toHaveBeenCalledTimes(1); + expect(service.peekSnapshot(REPO_CWD)?.forge.pullRequest?.title).toBe("MR self-healed"); + subscription.unsubscribe(); + } finally { + service.dispose(); + unregister(); + } + }); + test("subscription cancels generic forge PR status self-heal polling after unsubscribe", async () => { const forge = { ...createGitHubServiceStub(), @@ -1221,7 +1327,7 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => { await flushPromises(); subscription.unsubscribe(); - await vi.advanceTimersByTimeAsync(60_000); + await vi.advanceTimersByTimeAsync(120_000); await flushPromises(); expect(forge.getCurrentPullRequestStatus).not.toHaveBeenCalled(); diff --git a/packages/server/src/server/workspace-git-service.ts b/packages/server/src/server/workspace-git-service.ts index a0c7818c5..71209da34 100644 --- a/packages/server/src/server/workspace-git-service.ts +++ b/packages/server/src/server/workspace-git-service.ts @@ -47,7 +47,9 @@ import { checkoutLiteFromGitSnapshot } from "./workspace-registry-model.js"; const WORKSPACE_GIT_WATCH_DEBOUNCE_MS = 1_000; const BACKGROUND_GIT_FETCH_INTERVAL_MS = 180_000; export const WORKSPACE_GIT_SELF_HEAL_INTERVAL_MS = 60_000; -const FORGE_PR_STATUS_POLL_INTERVAL_MS = 60_000; +const FORGE_PR_STATUS_POLL_FAST_INTERVAL_MS = 20_000; +const FORGE_PR_STATUS_POLL_SLOW_INTERVAL_MS = 120_000; +const FORGE_PR_STATUS_POLL_ERROR_BACKOFF_CAP_MS = 300_000; const WORKING_TREE_WATCH_FALLBACK_REFRESH_MS = 5_000; // Auxiliary reads may reuse cached values within this window; snapshots do not expire on read. const WORKSPACE_GIT_AUXILIARY_READ_TTL_MS = 15_000; @@ -336,6 +338,7 @@ interface WorkspaceGitAuxiliaryReadCacheEntry { interface WorkspaceForgePrStatusPollTarget { headRef: string; + headSha?: string; headRepositoryOwner?: string; } @@ -1206,9 +1209,11 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { remoteUrl, target: pollTarget, }); + const previousPollKey = target.forgePrStatusPollKey; if (target.forgePrStatusPollKey === pollKey && target.forgePrStatusPollSubscription) { return; } + const pollImmediately = previousPollKey !== null && previousPollKey !== pollKey; this.stopForgePrStatusPollForTarget(target); target.forgePrStatusPollKey = pollKey; @@ -1216,6 +1221,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { target.forgePrStatusPollSubscription = resolution.service.retainCurrentPullRequestStatusPoll({ cwd: target.cwd, headRef: pollTarget.headRef, + ...(pollTarget.headSha ? { headSha: pollTarget.headSha } : {}), ...(pollTarget.headRepositoryOwner ? { headRepositoryOwner: pollTarget.headRepositoryOwner } : {}), @@ -1253,6 +1259,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { forge: resolution.forge, service: resolution.service, pollTarget, + pollImmediately, }); } @@ -1261,23 +1268,28 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { forge, service, pollTarget, + pollImmediately, }: { target: WorkspaceGitTarget; forge: string; service: ForgeService; pollTarget: WorkspaceForgePrStatusPollTarget; + pollImmediately: boolean; }): { 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 = () => { + const schedule = (delayMs: number) => { if (closed) { return; } timer = setTimeout(() => { timer = null; void poll(); - }, FORGE_PR_STATUS_POLL_INTERVAL_MS); + }, delayMs); }; const poll = async () => { @@ -1288,17 +1300,21 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { const status = await service.getCurrentPullRequestStatus({ cwd: target.cwd, headRef: pollTarget.headRef, + ...(pollTarget.headSha ? { headSha: pollTarget.headSha } : {}), ...(pollTarget.headRepositoryOwner ? { headRepositoryOwner: pollTarget.headRepositoryOwner } : {}), reason: "self-heal-forge-pr-status", }); if (!closed && this.isActiveObservedWorkspaceTarget(target)) { + latestStatus = status; + consecutiveErrors = 0; this.rememberForgePrStatusSnapshot(target, buildForgeSnapshotFromStatus(status, forge), { notify: true, }); } } catch (error) { + consecutiveErrors += 1; this.logger.warn( { err: error, @@ -1311,11 +1327,16 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { "Failed to run forge PR status self-heal refresh", ); } finally { - schedule(); + schedule(computeGenericForgeNextInterval(latestStatus, consecutiveErrors)); } }; - schedule(); + // A git-only refresh clears forge state when the commit-aware poll identity + // 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), + ); return { unsubscribe: () => { closed = true; @@ -2171,7 +2192,32 @@ function buildWorkspaceForgePrStatusPollKey({ remoteUrl: string; target: WorkspaceForgePrStatusPollTarget; }): string { - return JSON.stringify([forge, remoteUrl, target.headRef, target.headRepositoryOwner ?? null]); + return JSON.stringify([ + forge, + remoteUrl, + target.headRef, + target.headSha ?? null, + target.headRepositoryOwner ?? null, + ]); +} + +function computeGenericForgeNextInterval( + status: WorkspaceGitRuntimeSnapshot["forge"]["pullRequest"], + consecutiveErrors: number, +): number { + const isPending = + status?.checksStatus === "pending" || + status?.checks?.some((check) => check.status === "pending") === true; + const baseInterval = isPending + ? FORGE_PR_STATUS_POLL_FAST_INTERVAL_MS + : FORGE_PR_STATUS_POLL_SLOW_INTERVAL_MS; + if (consecutiveErrors <= 1) { + return baseInterval; + } + return Math.min( + baseInterval * 2 ** (consecutiveErrors - 1), + FORGE_PR_STATUS_POLL_ERROR_BACKOFF_CAP_MS, + ); } async function runGitFetch(cwd: string): Promise { diff --git a/packages/server/src/services/forge-registry.test.ts b/packages/server/src/services/forge-registry.test.ts index dd12174ec..8671f51f8 100644 --- a/packages/server/src/services/forge-registry.test.ts +++ b/packages/server/src/services/forge-registry.test.ts @@ -10,10 +10,12 @@ describe("forge registry", () => { const gitlab = createForgeService("gitlab"); const gitea = createForgeService("gitea"); const forgejo = createForgeService("forgejo"); + const codeberg = createForgeService("codeberg"); expect(github?.getCurrentPullRequestStatus).toBeTypeOf("function"); expect(gitlab?.getCurrentPullRequestStatus).toBeTypeOf("function"); expect(gitea?.getCurrentPullRequestStatus).toBeTypeOf("function"); expect(forgejo?.getCurrentPullRequestStatus).toBeTypeOf("function"); + expect(codeberg?.getCurrentPullRequestStatus).toBeTypeOf("function"); }); it("returns null for an unregistered forge", () => { @@ -29,9 +31,10 @@ describe("forge registry", () => { expect(defaultForgeRegistry.has("gitlab")).toBe(true); expect(defaultForgeRegistry.has("gitea")).toBe(true); expect(defaultForgeRegistry.has("forgejo")).toBe(true); + expect(defaultForgeRegistry.has("codeberg")).toBe(true); expect(defaultForgeRegistry.has("bitbucket")).toBe(false); expect(defaultForgeRegistry.ids()).toEqual( - expect.arrayContaining(["github", "gitlab", "gitea", "forgejo"]), + expect.arrayContaining(["github", "gitlab", "gitea", "forgejo", "codeberg"]), ); }); diff --git a/packages/server/src/services/forge-service.ts b/packages/server/src/services/forge-service.ts index b0bf9c2a1..7f6d36465 100644 --- a/packages/server/src/services/forge-service.ts +++ b/packages/server/src/services/forge-service.ts @@ -271,10 +271,10 @@ export type GetCheckDetailsOptions = { checkRunId?: number; workflowRunId?: number; /** - * GitLab-only: the change request iid. GitLab routes the fetch to the change - * request's head pipeline (`glab ci get --merge-request`), so a fork/detached - * MR pipeline living in the source project resolves instead of 404ing against - * the checkout's target project. GitHub ignores it. + * Change request number used when check details must resolve against a + * specific request rather than the current branch. GitLab routes the fetch to + * the MR's head pipeline; Gitea-family adapters resolve the PR head SHA by + * number, including for terminal PRs. GitHub ignores it. */ changeRequestNumber?: number; } & ForgeReadOptions; @@ -456,6 +456,7 @@ export interface ForgeService { options: { cwd: string; headRef: string; + headSha?: string; headRepositoryOwner?: string; } & ForgeReadOptions, ): Promise; @@ -496,6 +497,7 @@ export interface ForgeService { retainCurrentPullRequestStatusPoll?(options: { cwd: string; headRef: string; + headSha?: string; headRepositoryOwner?: string; onStatus?: (status: CurrentPullRequestStatus | null) => void; onError?: (error: unknown) => void; diff --git a/packages/server/src/services/gitea-service.test.ts b/packages/server/src/services/gitea-service.test.ts index 51e97b88f..6b2028e66 100644 --- a/packages/server/src/services/gitea-service.test.ts +++ b/packages/server/src/services/gitea-service.test.ts @@ -74,6 +74,35 @@ const CONFLICTING_PR = { ci: "failure", }; +function currentPullRequestApi(input: { + number: number; + state: "open" | "closed"; + headRef: string; + headSha: string; + merged?: boolean; +}) { + return { + number: input.number, + html_url: `https://gitea.com/example-user/sample-repo/pulls/${input.number}`, + title: "Historical pull request", + body: "", + state: input.state, + merged: input.merged ?? false, + mergeable: true, + updated_at: "2026-06-26T10:00:00Z", + labels: [], + head: { + ref: input.headRef, + sha: input.headSha, + repo: { id: 1, owner: { login: "example-user" } }, + }, + base: { + ref: "main", + repo: { id: 1, owner: { login: "example-user" } }, + }, + }; +} + const STATUS_PR_VIEW = { id: 161482, index: 5, @@ -1048,6 +1077,43 @@ describe("createGiteaService", () => { ]); }); + it("resolves terminal PR check details by explicit change request number", async () => { + const headSha = "8888888888888888888888888888888888888888"; + const terminalView = { + ...STATUS_PR_VIEW, + index: 8, + state: "closed", + head: "feat/recently-closed", + headSha, + hasMerged: true, + mergedAt: "2026-06-28T17:00:00Z", + }; + const { service, calls } = makeService( + (args) => { + if (args[0] === "pr" && args[1] === "8") return ok(JSON.stringify(terminalView)); + if (args[0] === "api" && args[1].endsWith(`/commits/${headSha}/status`)) { + return ok(JSON.stringify(SAMPLE_COMBINED_STATUS)); + } + throw new Error(`unexpected call: ${args.join(" ")}`); + }, + { resolveCurrentBranch: async () => "feat/recently-closed" }, + ); + + const details = await service.getCheckDetails({ + cwd: "/repo", + repoOwner: "example-user", + repoName: "sample-repo", + checkRunId: 2, + changeRequestNumber: 8, + }); + + expect(details).toMatchObject({ checkRunId: 2, name: "ci/lint", status: "pending" }); + expect(calls).toEqual([ + ["pr", "8", "-o", "json"], + ["api", `repos/example-user/sample-repo/commits/${headSha}/status`], + ]); + }); + it("resolves Gitea Actions check details addressed only by workflowRunId", async () => { const { service } = makeService( (args) => { @@ -1172,6 +1238,9 @@ describe("createGiteaService", () => { const { service, calls } = makeService( (args) => { if (args[0] === "pr" && args[1] === "list") return ok(JSON.stringify([])); + if (args[0] === "api" && args[1].includes("/pulls?state=all")) { + return ok(JSON.stringify([])); + } throw new Error(`unexpected call: ${args.join(" ")}`); }, { @@ -1187,13 +1256,9 @@ describe("createGiteaService", () => { checkRunId: 2, }), ).rejects.toThrow("Gitea pull request for branch feat/nonexistent was not found"); - expect( - calls - .filter((args) => args[0] === "pr" && args[1] === "list") - .map((args) => [argValue(args, "--state"), argValue(args, "--page")]), - ).toEqual([ - ["open", "1"], - ["all", undefined], + expect(calls[1]).toEqual([ + "api", + "repos/example-user/sample-repo/pulls?state=all&sort=recentupdate&page=1&limit=50", ]); }); @@ -1337,7 +1402,7 @@ describe("createGiteaService", () => { expect(status?.checksStatus).toBe("failure"); }); - it("finds the current branch PR on the second open pull-request page", async () => { + it("finds the open current-branch PR even when its remote head SHA differs", async () => { const firstPage = Array.from({ length: 50 }, (_, index) => ({ ...OPEN_PR, index: String(100 + index), @@ -1367,6 +1432,7 @@ describe("createGiteaService", () => { const status = await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "feat/sample-change", + headSha: "9999999999999999999999999999999999999999", }); expect(status?.number).toBe(5); @@ -1381,18 +1447,18 @@ describe("createGiteaService", () => { }); it("falls back to the recent all-state PR window for a recently closed current branch PR", async () => { - const closedPr = { - ...OPEN_PR, - index: "8", + const headSha = "8888888888888888888888888888888888888888"; + const closedPr = currentPullRequestApi({ + number: 8, state: "closed", - url: "https://gitea.com/example-user/sample-repo/pulls/8", - head: "feat/recently-closed", - }; + headRef: "feat/recently-closed", + headSha, + }); const { service, calls } = makeService((args) => { if (args[0] === "pr" && args[1] === "list" && argValue(args, "--state") === "open") { return ok(JSON.stringify([])); } - if (args[0] === "pr" && args[1] === "list" && argValue(args, "--state") === "all") { + if (args[0] === "api" && args[1].includes("/pulls?state=all")) { return ok(JSON.stringify([closedPr])); } throw new Error(`unexpected call: ${args.join(" ")}`); @@ -1401,22 +1467,79 @@ describe("createGiteaService", () => { const status = await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "feat/recently-closed", + headSha, }); expect(status).toMatchObject({ number: 8, headRefName: "feat/recently-closed" }); - expect( - calls - .filter((args) => args[0] === "pr" && args[1] === "list") - .map((args) => [argValue(args, "--state"), argValue(args, "--page")]), - ).toEqual([ - ["open", "1"], - ["all", undefined], + expect(calls[1]).toEqual([ + "api", + "repos/example-user/sample-repo/pulls?state=all&sort=recentupdate&page=1&limit=50", ]); }); + it("uses tea repository context when origin identity is unavailable", async () => { + const headSha = "8888888888888888888888888888888888888888"; + const closedPr = currentPullRequestApi({ + number: 8, + state: "closed", + headRef: "feat/recently-closed", + headSha, + }); + const { service, calls } = makeService( + (args) => { + if (args[0] === "pr" && args[1] === "list") return ok("[]"); + if (args[0] === "api" && args[1].startsWith("repos/{owner}/{repo}/pulls?")) { + return ok(JSON.stringify([closedPr])); + } + throw new Error(`unexpected call: ${args.join(" ")}`); + }, + { resolveRemoteUrl: async () => null }, + ); + + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feat/recently-closed", + headSha, + }); + + expect(status).toMatchObject({ number: 8, headRefName: "feat/recently-closed" }); + expect(calls[1]).toEqual([ + "api", + "repos/{owner}/{repo}/pulls?state=all&sort=recentupdate&page=1&limit=50", + ]); + }); + + it("does not attach a stale Gitea-family PR after a same-name branch advances", async () => { + const stale = currentPullRequestApi({ + number: 8, + state: "closed", + headRef: "dev", + headSha: "1111111111111111111111111111111111111111", + merged: true, + }); + 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([stale])); + } + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + + await expect( + service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "dev", + headSha: "2222222222222222222222222222222222222222", + }), + ).resolves.toBeNull(); + }); + 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([])); + if (args[0] === "api" && args[1].includes("/pulls?state=all")) { + return ok(JSON.stringify([])); + } throw new Error(`unexpected call: ${args.join(" ")}`); }); @@ -1426,13 +1549,9 @@ describe("createGiteaService", () => { }); expect(status).toBeNull(); - expect( - calls - .filter((args) => args[0] === "pr" && args[1] === "list") - .map((args) => [argValue(args, "--state"), argValue(args, "--page")]), - ).toEqual([ - ["open", "1"], - ["all", undefined], + expect(calls[1]).toEqual([ + "api", + "repos/example-user/sample-repo/pulls?state=all&sort=recentupdate&page=1&limit=50", ]); }); diff --git a/packages/server/src/services/gitea-service.ts b/packages/server/src/services/gitea-service.ts index 372a62810..2d467a26e 100644 --- a/packages/server/src/services/gitea-service.ts +++ b/packages/server/src/services/gitea-service.ts @@ -234,6 +234,36 @@ const GiteaPullRequestApiSchema = z }) .passthrough(); +const GiteaCurrentPullRequestApiSchema = z + .object({ + number: z.number(), + html_url: z.string(), + title: z.string(), + body: z.string().nullable().optional(), + state: z.string(), + merged: z.boolean().optional(), + mergeable: z.boolean().nullable().optional(), + updated_at: z.string().optional(), + labels: z + .array(z.object({ name: z.string() }).passthrough()) + .optional() + .default([]), + head: z + .object({ + ref: z.string(), + sha: z.string(), + repo: GiteaPullRequestRepoSchema.nullable().optional(), + }) + .passthrough(), + base: z + .object({ + ref: z.string(), + repo: GiteaPullRequestRepoSchema.nullable().optional(), + }) + .passthrough(), + }) + .passthrough(); + const GiteaCommitStatusSchema = z .object({ id: z.number(), @@ -324,6 +354,7 @@ const GiteaReviewCommentSchema = z type GiteaPrListItem = z.infer; type GiteaIssueListItem = z.infer; type GiteaPullRequestView = z.infer; +type GiteaCurrentPullRequestApi = z.infer; type GiteaCommitStatus = z.infer; type GiteaCombinedCommitStatus = z.infer; type GiteaActionsRun = z.infer; @@ -801,6 +832,25 @@ function parseGiteaRepoFromUrl(url: string): { owner?: string; name?: string } { return {}; } +function currentPullRequestApiToListItem(item: GiteaCurrentPullRequestApi): GiteaPrListItem { + const headOwner = item.head.repo?.owner?.login; + const baseOwner = item.base.repo?.owner?.login; + const head = + headOwner && headOwner !== baseOwner ? `${headOwner}:${item.head.ref}` : item.head.ref; + return { + index: String(item.number), + state: item.merged ? "merged" : item.state, + url: item.html_url, + title: item.title, + body: item.body ?? "", + ...(item.mergeable != null ? { mergeable: String(item.mergeable) } : {}), + base: item.base.ref, + head, + updated: item.updated_at, + labels: item.labels.map((label) => label.name).join(","), + }; +} + function toPullRequestSummary(item: GiteaPrListItem): PullRequestSummary { const { owner, name } = parseGiteaRepoFromUrl(item.url); return { @@ -1347,6 +1397,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For async function loadCurrentPullRequestStatus(input: { cwd: string; headRef: string; + headSha?: string; }): Promise { const match = await findCurrentPullRequestForHeadRef(input); if (!match) { @@ -1356,7 +1407,9 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For return loadCurrentPullRequestChecks(input.cwd, match, status); } - async function resolveExpectedHeadOwner(cwd: string): Promise { + async function resolveCurrentRepoIdentity( + cwd: string, + ): Promise<{ owner: string; name: string } | null> { const remoteUrl = await resolveRemoteUrl(cwd); if (!remoteUrl) { return null; @@ -1365,34 +1418,60 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For if (!location) { return null; } - return parseGitHubRemoteIdentity(location.path)?.owner ?? null; + const identity = parseGitHubRemoteIdentity(location.path); + return identity ? { owner: identity.owner, name: identity.name } : null; } async function findCurrentPullRequestForHeadRef(input: { cwd: string; headRef: string; + headSha?: string; }): Promise { - const expectedHeadOwner = await resolveExpectedHeadOwner(input.cwd); + const repoIdentity = await resolveCurrentRepoIdentity(input.cwd); + const expectedHeadOwner = repoIdentity?.owner ?? null; const openMatch = await findOpenPullRequestForHeadRef(input, expectedHeadOwner); if (openMatch) { return openMatch; } - const recentItems = await listPullRequestItems({ - cwd: input.cwd, - state: "all", - limit: CURRENT_PR_LOOKUP_PAGE_SIZE, - }); - return ( - recentItems.find((item) => matchesCurrentHeadRef(item, input.headRef, expectedHeadOwner)) ?? - null + // 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( + [ + "api", + `${repoPath}/pulls?state=all&sort=recentupdate&page=1&limit=${CURRENT_PR_LOOKUP_PAGE_SIZE}`, + ], + { cwd: input.cwd }, + GiteaCurrentPullRequestApiSchema, ); + const candidates = recentItems.filter((item) => + matchesCurrentHeadRef( + currentPullRequestApiToListItem(item), + input.headRef, + expectedHeadOwner, + ), + ); + 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; + return match ? currentPullRequestApiToListItem(match) : null; } async function findOpenPullRequestForHeadRef( input: { cwd: string; headRef: string; + headSha?: string; }, expectedHeadOwner: string | null, ): Promise { @@ -1668,7 +1747,11 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For }, getCurrentPullRequestStatus(input): Promise { - return loadCurrentPullRequestStatus({ cwd: input.cwd, headRef: input.headRef }); + return loadCurrentPullRequestStatus({ + cwd: input.cwd, + headRef: input.headRef, + headSha: input.headSha, + }); }, async getPullRequest(input: GetPullRequestOptions): Promise { diff --git a/packages/server/src/services/github-service.test.ts b/packages/server/src/services/github-service.test.ts index 0b3fa2787..d82defec0 100644 --- a/packages/server/src/services/github-service.test.ts +++ b/packages/server/src/services/github-service.test.ts @@ -21,7 +21,7 @@ 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,mergedAt,reviewDecision,mergeable,headRepositoryOwner"; + "number,url,title,state,isDraft,baseRefName,headRefName,headRefOid,mergedAt,reviewDecision,mergeable,headRepositoryOwner"; const CURRENT_PR_STATUS_FIELDS = `${CURRENT_PR_STATUS_BASE_FIELDS},statusCheckRollup`; interface RunnerCall { @@ -207,6 +207,7 @@ function currentPullRequestJson(overrides: Record = {}): string isDraft: false, baseRefName: "main", headRefName: "feature/fork", + headRefOid: "1111111111111111111111111111111111111111", mergedAt: null, statusCheckRollup: [], reviewDecision: "REVIEW_REQUIRED", @@ -2246,6 +2247,26 @@ describe("ForgeService", () => { expect(status?.mergeable).toBe("UNKNOWN"); }); + it("keeps an open PR when its remote head SHA differs from the checkout HEAD", async () => { + const runner = createRunner([ + currentPullRequestJson({ + headRefOid: "1111111111111111111111111111111111111111", + }), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headSha: "2222222222222222222222222222222222222222", + }); + + expect(status).toMatchObject({ number: 42, state: "open" }); + }); + it("loads GitHub merge, auto-merge, permission, policy, and queue facts for PR 993 shape", async () => { const runner = createScriptedRunner([ currentPullRequestJson({ @@ -2313,6 +2334,100 @@ describe("ForgeService", () => { }); }); + it("keeps a merged PR only 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 service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headSha, + }); + + expect(status?.number).toBe(42); + expect(status?.state).toBe("merged"); + }); + + it("does not attach a stale merged PR after a same-name branch advances", async () => { + const stale = currentPullRequestJson({ + state: "MERGED", + mergedAt: "2026-07-17T12:00:00Z", + headRefOid: "1111111111111111111111111111111111111111", + }); + const runner = createScriptedRunner([ + stale, + JSON.stringify({ owner: { login: "parentOwner" }, name: "parentRepo", parent: null }), + JSON.stringify([JSON.parse(stale)]), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + + await expect( + service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headSha: "2222222222222222222222222222222222222222", + }), + ).resolves.toBeNull(); + expect(runner.calls.some((call) => call.args[0] === "pr" && call.args[1] === "list")).toBe( + true, + ); + }); + + it("prefers an open PR over an exact-SHA merged PR for the same head", async () => { + const checkoutSha = "2222222222222222222222222222222222222222"; + const owner = { login: "forkOwner" }; + const staleView = currentPullRequestJson({ + state: "MERGED", + mergedAt: "2026-07-15T12:00:00Z", + headRefOid: "0000000000000000000000000000000000000000", + headRepositoryOwner: owner, + }); + const open = JSON.parse( + currentPullRequestJson({ + number: 43, + state: "OPEN", + headRefOid: "3333333333333333333333333333333333333333", + headRepositoryOwner: owner, + }), + ); + const exactMerged = JSON.parse( + currentPullRequestJson({ + number: 42, + state: "MERGED", + mergedAt: "2026-07-16T12:00:00Z", + headRefOid: checkoutSha, + headRepositoryOwner: owner, + }), + ); + const runner = createScriptedRunner([staleView, JSON.stringify([exactMerged, open])]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headSha: checkoutSha, + headRepositoryOwner: "forkOwner", + }); + + expect(status).toMatchObject({ number: 43, state: "open" }); + }); + it("resolves fork PR heads to the parent repository when gh pr view returns a stale branch match", async () => { const runner = createScriptedRunner([ currentPullRequestJson({ @@ -2541,6 +2656,18 @@ describe("ForgeService", () => { expect(calls.map((call) => call.args)).toEqual([ ["pr", "view", "--json", CURRENT_PR_STATUS_FIELDS], ["repo", "view", "--json", "owner,name,parent"], + [ + "pr", + "list", + "--state", + "all", + "--head", + "main", + "--limit", + "10", + "--json", + CURRENT_PR_STATUS_FIELDS, + ], ]); }); diff --git a/packages/server/src/services/github-service.ts b/packages/server/src/services/github-service.ts index 163fb6d30..e2d82db88 100644 --- a/packages/server/src/services/github-service.ts +++ b/packages/server/src/services/github-service.ts @@ -325,6 +325,7 @@ const CurrentPullRequestStatusSchema = z.object({ isDraft: z.boolean().optional().catch(false), baseRefName: z.string().catch(""), headRefName: z.string().catch(""), + headRefOid: z.string().optional(), mergedAt: z.string().nullable().optional(), statusCheckRollup: z.unknown().optional(), reviewDecision: z.unknown().optional(), @@ -493,7 +494,7 @@ query PullRequestCheckoutTarget($owner: String!, $name: String!, $number: Int!) }`; const CURRENT_PR_STATUS_BASE_FIELDS = - "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,reviewDecision,mergeable,headRepositoryOwner"; + "number,url,title,state,isDraft,baseRefName,headRefName,headRefOid,mergedAt,reviewDecision,mergeable,headRepositoryOwner"; const CURRENT_PR_STATUS_FIELDS = `${CURRENT_PR_STATUS_BASE_FIELDS},statusCheckRollup`; const PULL_REQUEST_STATUS_FACTS_QUERY = ` @@ -716,6 +717,7 @@ interface InFlightCacheEntry { interface GitHubPollTarget { cwd: string; headRef: string; + headSha?: string; headRepositoryOwner?: string; retainCount: number; timer: NodeJS.Timeout | null; @@ -727,6 +729,7 @@ interface GitHubPollTarget { interface ResolvedPullRequestCandidate { status: CurrentPullRequestStatus; + headSha?: string; headRepositoryOwner?: string; } @@ -868,6 +871,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G function getPollTargetKey(target: { cwd: string; headRef: string; + headSha?: string; headRepositoryOwner?: string; }): string { return buildCacheKey({ @@ -875,6 +879,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G method: "getCurrentPullRequestStatus", args: { headRef: target.headRef, + headSha: target.headSha, headRepositoryOwner: target.headRepositoryOwner, }, }); @@ -883,6 +888,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G function updatePollTargetAfterSuccess(update: { cwd: string; headRef: string; + headSha?: string; headRepositoryOwner?: string; status: CurrentPullRequestStatus | null; notify: boolean; @@ -932,6 +938,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G await api.getCurrentPullRequestStatus({ cwd: target.cwd, headRef: target.headRef, + headSha: target.headSha, headRepositoryOwner: target.headRepositoryOwner, reason: "self-heal-github", }); @@ -1095,6 +1102,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G method: "getCurrentPullRequestStatus", args: { headRef: input.headRef, + headSha: input.headSha, headRepositoryOwner: input.headRepositoryOwner, }, readOptions: input, @@ -1102,6 +1110,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G const status = await resolveCurrentPullRequestView({ cwd: input.cwd, headRef: input.headRef, + headSha: input.headSha, headRepositoryOwner: input.headRepositoryOwner, run, }); @@ -1111,6 +1120,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G updatePollTargetAfterSuccess({ cwd: input.cwd, headRef: input.headRef, + headSha: input.headSha, headRepositoryOwner: input.headRepositoryOwner, status, notify: input.reason === "self-heal-github", @@ -1502,6 +1512,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G target = { cwd: input.cwd, headRef: input.headRef, + headSha: input.headSha, headRepositoryOwner: input.headRepositoryOwner, retainCount: 0, timer: null, @@ -1869,6 +1880,7 @@ function isStatusCheckRollupPermissionError(error: unknown): boolean { async function resolveCurrentPullRequestView(options: { cwd: string; headRef: string; + headSha?: string; headRepositoryOwner?: string; run: (args: string[], options: GitHubCommandRunnerOptions) => Promise; }): Promise { @@ -1877,6 +1889,7 @@ async function resolveCurrentPullRequestView(options: { ? pickPullRequestCandidate({ candidates: [viewCandidate], headRef: options.headRef, + headSha: options.headSha, headRepositoryOwner: options.headRepositoryOwner, }) : null; @@ -1893,12 +1906,13 @@ async function resolveCurrentPullRequestView(options: { const forkOwner = repo?.owner?.login; const parentOwner = repo?.parent?.owner?.login; const parentName = repo?.parent?.name; - if (!forkOwner || !parentOwner || !parentName) { + if (!forkOwner) { return null; } - - listHeadRef = `${forkOwner}:${options.headRef}`; - listRepo = `${parentOwner}/${parentName}`; + if (parentOwner && parentName) { + listHeadRef = `${forkOwner}:${options.headRef}`; + listRepo = `${parentOwner}/${parentName}`; + } headRepositoryOwner = forkOwner; } @@ -1911,6 +1925,7 @@ async function resolveCurrentPullRequestView(options: { const match = pickPullRequestCandidate({ candidates, headRef: options.headRef, + headSha: options.headSha, headRepositoryOwner, }); return match?.status ?? null; @@ -2173,8 +2188,10 @@ function toCurrentPullRequestCandidate( return null; } const headRepositoryOwner = item.headRepositoryOwner?.login; + const headSha = item.headRefOid; return { status, + ...(headSha ? { headSha } : {}), ...(headRepositoryOwner ? { headRepositoryOwner } : {}), }; } @@ -2190,26 +2207,45 @@ function hasResolvedRepoIdentity(status: CurrentPullRequestStatus): boolean { function pickPullRequestCandidate(options: { candidates: ResolvedPullRequestCandidate[]; headRef: string; + headSha?: string; headRepositoryOwner?: 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) + ) { + return false; + } if (!options.headRepositoryOwner) { return true; } return candidate.headRepositoryOwner === options.headRepositoryOwner; }); - matching.sort(comparePullRequestCandidatePreference); + matching.sort((left, right) => + comparePullRequestCandidatePreference(left, right, options.headSha), + ); return matching[0] ?? null; } function comparePullRequestCandidatePreference( left: ResolvedPullRequestCandidate, right: ResolvedPullRequestCandidate, + headSha?: string, ): number { - return getPullRequestStateRank(left.status) - getPullRequestStateRank(right.status); + 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; } function getPullRequestStateRank(status: CurrentPullRequestStatus): number { diff --git a/packages/server/src/services/gitlab-service.test.ts b/packages/server/src/services/gitlab-service.test.ts index dc9cc5294..3f930a720 100644 --- a/packages/server/src/services/gitlab-service.test.ts +++ b/packages/server/src/services/gitlab-service.test.ts @@ -33,6 +33,24 @@ function makeService(responder: Responder, overrides: Partial = {}, ): PullRequestCommandStatus { @@ -60,6 +78,7 @@ const OPEN_MR = { state: "opened", source_branch: "release/v0.4.0", target_branch: "main", + sha: "1111111111111111111111111111111111111111", source_project_id: 101, target_project_id: 101, draft: false, @@ -231,7 +250,9 @@ const DISCUSSIONS = [ describe("createGitLabService", () => { it("maps a glab merge request view to the neutral current PR status", async () => { - const { service, calls } = makeService(() => ok(JSON.stringify(OPEN_MR))); + const { service, calls } = makeService((args) => + ok(JSON.stringify(args[1] === "list" ? [OPEN_MR] : OPEN_MR)), + ); const status = await service.getCurrentPullRequestStatus({ cwd: "/repo", @@ -261,23 +282,26 @@ describe("createGitLabService", () => { pipelineStatus: "success", mergeWhenPipelineSucceeds: false, }); - expect(calls[0]).toEqual(["mr", "view", "release/v0.4.0", "-F", "json"]); + expect(calls[0]).toEqual(currentMrListArgs("release/v0.4.0")); + expect(calls[1]).toEqual(["mr", "view", "14", "-F", "json"]); }); it("reports a conflicting merge request as CONFLICTING", async () => { - const { service } = makeService(() => - ok( - JSON.stringify({ ...OPEN_MR, has_conflicts: true, detailed_merge_status: "broken_status" }), - ), + const conflicting = { + ...OPEN_MR, + source_branch: "x", + has_conflicts: true, + detailed_merge_status: "broken_status", + }; + const { service } = makeService((args) => + ok(JSON.stringify(args[1] === "list" ? [conflicting] : conflicting)), ); const status = await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "x" }); expect(status?.mergeable).toBe("CONFLICTING"); }); it("returns null when no merge request exists for the branch", async () => { - const { service } = makeService(() => { - throw { code: 1, stderr: "no open merge request available for 'feature/x'" }; - }); + const { service } = makeService(() => ok("[]")); const status = await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "feature/x", @@ -285,6 +309,61 @@ describe("createGitLabService", () => { expect(status).toBeNull(); }); + it("selects a terminal merge request only when its head SHA matches the checkout", async () => { + const branch = "dev"; + const checkoutSha = "2222222222222222222222222222222222222222"; + const newestStale = { + ...OPEN_MR, + iid: 271, + state: "merged", + source_branch: branch, + sha: "1111111111111111111111111111111111111111", + updated_at: "2026-07-17T12:00:00.000Z", + }; + const exactOlder = { + ...OPEN_MR, + iid: 270, + state: "merged", + source_branch: branch, + sha: checkoutSha, + updated_at: "2026-07-16T12:00:00.000Z", + }; + 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[0] === "api") return ok("{}"); + throw new Error(`unexpected call: ${args.join(" ")}`); + }); + + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: branch, + headSha: checkoutSha, + }); + + expect(status?.number).toBe(270); + expect(calls[1]).toEqual(["mr", "view", "270", "-F", "json"]); + }); + + it("does not attach 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]))); + + await expect( + service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "dev", + headSha: "2222222222222222222222222222222222222222", + }), + ).resolves.toBeNull(); + expect(calls).toEqual([currentMrListArgs("dev")]); + }); + it("looks up a numeric current branch through the source-branch list filter", async () => { const numericBranchMr = { ...OPEN_MR, @@ -313,6 +392,7 @@ describe("createGitLabService", () => { const status = await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "1234", + headSha: "2222222222222222222222222222222222222222", }); expect(status).toMatchObject({ @@ -320,7 +400,7 @@ describe("createGitLabService", () => { title: "Fix numeric branch", headRefName: "1234", }); - expect(calls[0]).toEqual(["mr", "list", "--source-branch", "1234", "-F", "json"]); + expect(calls[0]).toEqual(currentMrListArgs("1234")); expect(calls[1]).toEqual(["mr", "view", "21", "-F", "json"]); expect(calls).not.toContainEqual(["mr", "view", "1234", "-F", "json"]); }); @@ -342,7 +422,7 @@ describe("createGitLabService", () => { }); expect(status).toBeNull(); - expect(calls).toEqual([["mr", "list", "--source-branch", "1234", "-F", "json"]]); + expect(calls).toEqual([currentMrListArgs("1234")]); }); it("lists merge requests as neutral PR summaries", async () => { @@ -549,17 +629,16 @@ describe("createGitLabService", () => { }); it("surfaces the head pipeline id and url on the gitlab status facts", async () => { - const { service } = makeService(() => - ok( - JSON.stringify({ - ...OPEN_MR, - head_pipeline: { - id: 306, - status: "running", - web_url: "https://gitlab.example.com/example-group/example-project/-/pipelines/306", - }, - }), - ), + const pipelineMr = { + ...OPEN_MR, + head_pipeline: { + id: 306, + status: "running", + web_url: "https://gitlab.example.com/example-group/example-project/-/pipelines/306", + }, + }; + const { service } = makeService((args) => + ok(JSON.stringify(args[1] === "list" ? [pipelineMr] : pipelineMr)), ); const status = await service.getCurrentPullRequestStatus({ @@ -682,6 +761,7 @@ describe("createGitLabService", () => { it("populates approval counts from the approvals endpoint", async () => { const { service, calls } = makeService((args) => { + if (args[0] === "mr" && args[1] === "list") return ok(JSON.stringify([OPEN_MR])); if (args[0] === "mr" && args[1] === "view") return ok(JSON.stringify(OPEN_MR)); if (args[0] === "api" && args[1].endsWith("/approvals")) return ok(JSON.stringify(APPROVALS)); throw new Error(`unexpected call: ${args.join(" ")}`); @@ -697,7 +777,7 @@ describe("createGitLabService", () => { approvalsRequired: 2, approvalsGiven: 1, }); - expect(calls[1]).toEqual([ + expect(calls[2]).toEqual([ "api", "projects/example-group%2Fexample-project/merge_requests/14/approvals", ]); @@ -705,6 +785,7 @@ describe("createGitLabService", () => { it("falls back to zero approvals when the approvals endpoint returns an error", async () => { const { service } = makeService((args) => { + if (args[0] === "mr" && args[1] === "list") return ok(JSON.stringify([OPEN_MR])); if (args[0] === "mr" && args[1] === "view") return ok(JSON.stringify(OPEN_MR)); throw { code: 1, stderr: "500 Internal Server Error" }; }); diff --git a/packages/server/src/services/gitlab-service.ts b/packages/server/src/services/gitlab-service.ts index 204c99dab..dae4cb2b4 100644 --- a/packages/server/src/services/gitlab-service.ts +++ b/packages/server/src/services/gitlab-service.ts @@ -149,6 +149,7 @@ const GitLabMergeRequestSchema = z state: z.string(), source_branch: z.string(), target_branch: z.string(), + sha: z.string().optional(), source_project_id: z.number().nullable().optional(), target_project_id: z.number().nullable().optional(), draft: z.boolean().optional(), @@ -861,16 +862,26 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F return runJson(["mr", "view", ref, "-F", "json"], { cwd }, GitLabMergeRequestSchema); } - function isNumericBranchRef(ref: string): boolean { - return /^[0-9]+$/.test(ref); - } - - async function listOpenMergeRequestsBySourceBranch( + async function listMergeRequestsBySourceBranch( cwd: string, sourceBranch: string, ): Promise { return runJson( - ["mr", "list", "--source-branch", sourceBranch, "-F", "json"], + [ + "mr", + "list", + "--all", + "--source-branch", + sourceBranch, + "--order", + "updated_at", + "--sort", + "desc", + "--per-page", + "100", + "-F", + "json", + ], { cwd }, z.array(GitLabMergeRequestSchema), ); @@ -879,16 +890,17 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F async function resolveCurrentMergeRequest( cwd: string, headRef: string, + headSha?: string, ): Promise { - if (!isNumericBranchRef(headRef)) { - return viewMergeRequest(cwd, headRef); - } - - const mergeRequests = await listOpenMergeRequestsBySourceBranch(cwd, headRef); + const mergeRequests = await listMergeRequestsBySourceBranch(cwd, headRef); + const candidates = mergeRequests.filter((mr) => mr.source_branch === headRef); const match = - mergeRequests.find( - (mr) => mr.source_branch === headRef && mapMergeRequestState(mr.state) === "open", - ) ?? null; + candidates.find((mr) => mapMergeRequestState(mr.state) === "open") ?? + candidates.find( + (mr) => + mapMergeRequestState(mr.state) !== "open" && headSha !== undefined && mr.sha === headSha, + ) ?? + null; return match ? viewMergeRequest(cwd, String(match.iid)) : null; } @@ -1004,7 +1016,7 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F async getCurrentPullRequestStatus(input): Promise { try { - const mr = await resolveCurrentMergeRequest(input.cwd, input.headRef); + const mr = await resolveCurrentMergeRequest(input.cwd, input.headRef, input.headSha); if (!mr) { return null; } diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index cf07bfc97..c28586d8a 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -48,7 +48,11 @@ import { startGitCommandMetrics, stopGitCommandMetrics } from "./run-git-command 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 { TeaAuthenticationError, TeaCliMissingError } from "../services/gitea-service.js"; +import { + TeaAuthenticationError, + TeaCliMissingError, + TeaCommandError, +} from "../services/gitea-service.js"; import { createWorktree as createWorktreePrimitive, type CreateWorktreeOptions, @@ -161,6 +165,7 @@ function createPullRequestStatus(overrides?: Partial) interface RequestedPullRequestTarget { headRef: string; + headSha?: string; headRepositoryOwner?: string; } @@ -176,6 +181,7 @@ function createGitHubServiceRecordingPullRequestTargets( github.getCurrentPullRequestStatus = async (request) => { options.requestedTargets.push({ headRef: request.headRef, + ...(request.headSha ? { headSha: request.headSha } : {}), ...(request.headRepositoryOwner ? { headRepositoryOwner: request.headRepositoryOwner } : {}), }); return createPullRequestStatus({ @@ -2341,7 +2347,8 @@ const x = 1; const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome); - expect(lookupTarget).toEqual({ headRef: "refactor/workspace-scripts" }); + expect(lookupTarget).toMatchObject({ headRef: "refactor/workspace-scripts" }); + expect(lookupTarget?.headSha).toMatch(/^[0-9a-f]{40}$/); }); it("keeps the local branch lookup when origin tracking uses the same head name", async () => { @@ -2356,7 +2363,8 @@ const x = 1; const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome); - expect(lookupTarget).toEqual({ headRef: "feature" }); + expect(lookupTarget).toMatchObject({ headRef: "feature" }); + expect(lookupTarget?.headSha).toMatch(/^[0-9a-f]{40}$/); }); it("does not attach an owner when the tracked remote is the same GitHub repository", async () => { @@ -2378,7 +2386,8 @@ const x = 1; const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome); - expect(lookupTarget).toEqual({ headRef: "refactor/workspace-scripts" }); + expect(lookupTarget).toMatchObject({ headRef: "refactor/workspace-scripts" }); + expect(lookupTarget?.headSha).toMatch(/^[0-9a-f]{40}$/); }); it("keeps the fork owner when same-repo comparison is indeterminate", async () => { @@ -2396,7 +2405,8 @@ const x = 1; const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome); - expect(lookupTarget).toEqual({ headRef: "main", headRepositoryOwner: "chethanuk" }); + expect(lookupTarget).toMatchObject({ headRef: "main", headRepositoryOwner: "chethanuk" }); + expect(lookupTarget?.headSha).toMatch(/^[0-9a-f]{40}$/); }); it("uses the configured push remote for fork PR lookup when upstream is absent", async () => { @@ -2425,8 +2435,11 @@ const x = 1; ); expect(getBranchUpstream(repoDir)).toBeNull(); - expect(factsTarget).toEqual({ headRef: "main", headRepositoryOwner: "chethanuk" }); - expect(requestedTargets).toEqual([{ headRef: "main", headRepositoryOwner: "chethanuk" }]); + expect(factsTarget).toMatchObject({ headRef: "main", headRepositoryOwner: "chethanuk" }); + expect(factsTarget?.headSha).toMatch(/^[0-9a-f]{40}$/); + expect(requestedTargets).toEqual([ + expect.objectContaining({ headRef: "main", headRepositoryOwner: "chethanuk" }), + ]); }); it("keeps the local branch lookup when same-repo tracking points at the base branch", async () => { @@ -2441,7 +2454,8 @@ const x = 1; const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome); - expect(lookupTarget).toEqual({ headRef: "tender-parrot" }); + expect(lookupTarget).toMatchObject({ headRef: "tender-parrot" }); + expect(lookupTarget?.headSha).toMatch(/^[0-9a-f]{40}$/); }); it("derives the same origin tracked head for on-demand PR status reads", async () => { @@ -2495,7 +2509,9 @@ const x = 1; const status = await getPullRequestStatus(repoDir, github); - expect(requestedTargets).toEqual([{ headRef: "main", headRepositoryOwner: "chethanuk" }]); + expect(requestedTargets).toEqual([ + expect.objectContaining({ headRef: "main", headRepositoryOwner: "chethanuk" }), + ]); expect(status.status?.number).toBe(345); expect(status.status?.headRefName).toBe("main"); }); @@ -2544,6 +2560,33 @@ const x = 1; expect(callCount).toBe(1); }); + 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"], { + cwd: repoDir, + }); + + const requestedShas: string[] = []; + const github = createGitHubServiceForStatus(null); + github.getCurrentPullRequestStatus = async (options) => { + if (options.headSha) requestedShas.push(options.headSha); + return createPullRequestStatus({ + url: `https://github.com/getpaseo/paseo/pull/${requestedShas.length}`, + }); + }; + + const first = await getPullRequestStatus(repoDir, github); + writeFileSync(join(repoDir, "next.txt"), "next\n"); + execFileSync("git", ["add", "next.txt"], { cwd: repoDir }); + execFileSync("git", ["commit", "-m", "next commit"], { cwd: repoDir }); + const second = await getPullRequestStatus(repoDir, github); + + expect(first.status?.url).toContain("/pull/1"); + expect(second.status?.url).toContain("/pull/2"); + expect(requestedShas).toHaveLength(2); + expect(requestedShas[0]).not.toBe(requestedShas[1]); + }); + 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"], { @@ -2637,6 +2680,41 @@ const x = 1; } }); + it("keeps stale PR status when a Gitea-family refresh hits a transient command error", 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 callCount = 0; + const service = createGitHubServiceForStatus(null); + service.getCurrentPullRequestStatus = async () => { + callCount += 1; + if (callCount === 1) { + return createPullRequestStatus({ url: "https://gitea.example.com/acme/repo/pulls/7" }); + } + throw new TeaCommandError({ + args: ["pr", "list"], + cwd: repoDir, + exitCode: 1, + stderr: "request timed out", + }); + }; + + const fresh = await getPullRequestStatus(repoDir, service); + await sleep(80); + const stale = await getPullRequestStatus(repoDir, service); + + expect(stale).toEqual(fresh); + expect(stale.status?.url).toContain("/pulls/7"); + expect(callCount).toBe(2); + } finally { + __resetPullRequestStatusCacheForTests(); + } + }); + 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 331d2413f..1b0752ede 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -7,7 +7,7 @@ import type { Logger } from "pino"; import type { ParsedDiffFile } from "../server/utils/diff-highlighter.js"; import { parseAndHighlightDiff } from "../server/utils/diff-highlighter.js"; import { parseGitHubRepoFromRemote } from "../server/workspace-git-metadata.js"; -import { GitHubCommandError, createGitHubService } from "../services/github-service.js"; +import { createGitHubService } from "../services/github-service.js"; import type { CurrentPullRequestStatus, ForgeAuthState, @@ -15,7 +15,11 @@ import type { ForgeSpecificStatusFacts, PullRequestMergeable, } from "../services/forge-service.js"; -import { ForgeAuthenticationError, ForgeCliMissingError } from "../services/forge-cli-command.js"; +import { + ForgeAuthenticationError, + ForgeCliMissingError, + ForgeCommandError, +} from "../services/forge-cli-command.js"; import { parseGitRevParsePath, resolveGitRevParsePath } from "./git-rev-parse-path.js"; import { runGitCommand } from "./run-git-command.js"; import { isPaseoOwnedWorktreeCwd, resolvePaseoWorktreesBaseRoot } from "./worktree.js"; @@ -67,6 +71,7 @@ interface CheckoutReadCacheOptions { interface PullRequestStatusLookupTarget { headRef: string; + headSha?: string; headRepositoryOwner?: string; } @@ -116,8 +121,8 @@ function createShortstatCache(ttlMs: number) { }); } -function getPullRequestStatusCacheKey(cwd: string): string { - return resolve(cwd); +function getPullRequestStatusCacheKey(cwd: string, headSha: string | null): string { + return `${resolve(cwd)}\u0000${headSha ?? ""}`; } function rememberPullRequestStatus(cacheKey: string, status: PullRequestStatusResult): void { @@ -829,6 +834,38 @@ export async function getCurrentBranch(cwd: string): Promise { } } +async function getCurrentHeadSha(cwd: string, context?: CheckoutContext): Promise { + const knownSha = context?.facts?.isGit + ? context.facts.pullRequestLookupTarget?.headSha + : undefined; + if (knownSha) { + return knownSha; + } + try { + const { stdout } = await runGitCommand(["rev-parse", "HEAD"], { + cwd, + envOverlay: READ_ONLY_GIT_ENV, + logger: context?.logger, + }); + const sha = stdout.trim(); + return sha.length > 0 ? sha : null; + } catch { + return null; + } +} + +async function addHeadShaToPullRequestLookupTarget( + cwd: string, + target: PullRequestStatusLookupTarget | null, + context?: CheckoutContext, +): Promise { + if (!target) { + return null; + } + const headSha = await getCurrentHeadSha(cwd, context); + return headSha ? { ...target, headSha } : target; +} + async function getRebaseHeadBranch(cwd: string): Promise { const paths = ["rebase-merge/head-name", "rebase-apply/head-name"]; const results = await Promise.all( @@ -1761,6 +1798,11 @@ export async function getCheckoutSnapshotFacts( context, )) ?? pullRequestLookupTarget; } + pullRequestLookupTarget = await addHeadShaToPullRequestLookupTarget( + cwd, + pullRequestLookupTarget, + context, + ); return { isGit: true, @@ -3402,7 +3444,8 @@ export async function getPullRequestStatus( options?: CheckoutReadCacheOptions, context?: CheckoutContext, ): Promise { - const cacheKey = getPullRequestStatusCacheKey(cwd); + const headSha = await getCurrentHeadSha(cwd, context); + const cacheKey = getPullRequestStatusCacheKey(cwd, headSha); if (!options?.force) { const cached = pullRequestStatusCache.get(cacheKey); if (cached) { @@ -3415,14 +3458,14 @@ export async function getPullRequestStatus( } } - const lookup = getPullRequestStatusUncached(cwd, forgeService, options, context) + const lookup = getPullRequestStatusUncached(cwd, forgeService, options, context, headSha) .then((status) => { pullRequestStatusCache.set(cacheKey, status); rememberPullRequestStatus(cacheKey, status); return status; }) .catch((error) => { - if (!options?.force && error instanceof GitHubCommandError) { + if (!options?.force && error instanceof ForgeCommandError) { const stale = lastSuccessfulPullRequestStatus.get(cacheKey); if (stale) { return stale; @@ -3443,6 +3486,7 @@ async function getPullRequestStatusUncached( forgeService: ForgeService, options?: CheckoutReadCacheOptions, context?: CheckoutContext, + headSha?: string | null, ): Promise { if (context?.facts?.isGit === false) { return buildPullRequestStatusResult(null, "no_remote"); @@ -3455,7 +3499,11 @@ async function getPullRequestStatusUncached( return buildPullRequestStatusResult(null, "no_remote"); } try { - const lookupTarget = await resolvePullRequestStatusLookupTarget(cwd, head, context); + 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;