From e546497a21b93dcfe0b0d8d0a64764365f9add39 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 3 Nov 2026 13:43:02 +0700 Subject: [PATCH] fix(workspaces): keep merged pull requests visible * fix(workspaces): keep merged pull requests visible A remote update before merge can leave the checkout behind the PR head. Match terminal requests by branch for display and require an exact fresh head match before automatic archive. * fix(forges): retain pull requests targeting a fork A fork checkout can have its own pull request as well as one targeting its parent. Compare both candidates before choosing which request to display. * fix(forges): rank closed drafts as terminal GitHub retains the draft flag after closing a pull request. Selection must use the request state so a closed draft does not override an open or exact-head request. --- docs/forge-providers.md | 20 +- .../archive-if-safe.test.ts | 21 ++ .../auto-archive-on-merge/archive-if-safe.ts | 14 +- .../auto-archive-on-merge/behavior.test.ts | 63 +++++- .../auto-archive-on-merge/index.test.ts | 5 + .../src/server/auto-archive-on-merge/index.ts | 2 + .../src/server/workspace-git-service.ts | 6 +- packages/server/src/services/forge-service.ts | 1 + .../server/src/services/gitea-service.test.ts | 4 +- packages/server/src/services/gitea-service.ts | 11 +- .../src/services/github-service.test.ts | 180 +++++++++++++++++- .../server/src/services/github-service.ts | 46 +++-- .../src/services/gitlab-service.test.ts | 10 +- .../server/src/services/gitlab-service.ts | 2 + packages/server/src/utils/checkout-git.ts | 1 + 15 files changed, 332 insertions(+), 54 deletions(-) diff --git a/docs/forge-providers.md b/docs/forge-providers.md index 376915d3ce..2d31eaef2f 100644 --- a/docs/forge-providers.md +++ b/docs/forge-providers.md @@ -97,19 +97,15 @@ 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: +Current change-request lookup matches the branch and repository for both open and +terminal requests. Prefer an open request, then a terminal request at the local +HEAD, then the most recently updated terminal request. A reused branch can show +its previous request until a new one opens. Auto-archive separately requires an +observed open-to-merged transition and a fresh exact match between checkout HEAD +and the PR head, along with the existing workspace safety checks. -- 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. +Keep checkout HEAD in adapter cache and poll identities so checkout changes +refresh the facts used by archive eligibility. 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/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..f30966af2f 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 @@ -30,6 +30,7 @@ function createPullRequest( state: "open", baseRefName: "main", headRefName: "feature", + headSha: "same-head", isMerged: true, ...overrides, }; @@ -46,6 +47,7 @@ function createSnapshot(overrides?: { repoRoot: "/tmp/repo", mainRepoRoot: "/tmp/repo", currentBranch: "feature", + headSha: "same-head", remoteUrl: "https://github.com/acme/repo.git", isPaseoOwnedWorktree: true, isDirty: false, @@ -346,6 +348,25 @@ afterEach(() => { }); describe("archiveIfSafe", () => { + test("denies archive when checkout facts do not establish safety", async () => { + for (const git of [{ headSha: undefined }, { behindOfOrigin: 1 }, { isDirty: null }]) { + const harness = createHarness({ snapshot: createSnapshot({ git }) }); + await runArchiveIfSafe(harness); + expect(harness.deps.archiveByScope).not.toHaveBeenCalled(); + } + const harness = createHarness({ + snapshot: createSnapshot({ pullRequest: createPullRequest({ headSha: undefined }) }), + }); + await runArchiveIfSafe(harness); + expect(harness.deps.archiveByScope).not.toHaveBeenCalled(); + }); + + test("keeps the workspace active when the merged PR head differs from checkout", async () => { + const harness = createHarness({ snapshot: createSnapshot({ git: { headSha: "local-head" } }) }); + await runArchiveIfSafe(harness); + expect(harness.deps.archiveByScope).not.toHaveBeenCalled(); + }); + beforeEach(() => { vi.clearAllMocks(); }); 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..da94c5931b 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 @@ -59,13 +59,21 @@ export async function archiveIfSafe(input: { const cwd = snapshot.cwd; const pullRequest = snapshot.forge.pullRequest; - if (!pullRequest?.isMerged) { + if ( + !pullRequest?.isMerged || + !snapshot.git.headSha || + snapshot.git.headSha !== pullRequest.headSha || + snapshot.forge.error + ) { return; } - if (snapshot.git.isDirty === true) { + if (snapshot.git.isDirty !== false) { return; } - if (typeof snapshot.git.aheadOfOrigin === "number" && snapshot.git.aheadOfOrigin > 0) { + if ( + (snapshot.git.aheadOfOrigin !== null && snapshot.git.aheadOfOrigin !== 0) || + (snapshot.git.behindOfOrigin !== null && snapshot.git.behindOfOrigin !== 0) + ) { 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..6d24506d36 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 @@ -161,7 +161,12 @@ async function createWorkspaceJourney() { }; } -async function createUnobservedWorkspace(archiveBarrier?: Promise) { +async function createUnobservedWorkspace(journeyOptions?: { + archiveBarrier?: Promise; + changeBeforeArchiveRead?: boolean; +}) { + const { archiveBarrier, changeBeforeArchiveRead: shouldChange = false } = journeyOptions ?? {}; + let changeBeforeArchiveRead = shouldChange; const { repoDir, paseoHome, worktree } = await createWorkspaceFiles(); const cwd = worktree.worktreePath; run(repoDir, ["remote", "add", "origin", "https://github.com/acme/repo.git"]); @@ -173,8 +178,10 @@ async function createUnobservedWorkspace(archiveBarrier?: Promise) { runner: async () => ({ stdout: "", stderr: "" }), }); let observation: PullRequestObservation | null = null; + let remoteHeadSha = execFileSync("git", ["rev-parse", "HEAD"], { cwd, encoding: "utf8" }).trim(); const reads: Array = []; const archives: string[] = []; + let archiveDecisions = 0; let active = true; const service = new WorkspaceGitServiceImpl({ logger, @@ -186,6 +193,7 @@ async function createUnobservedWorkspace(archiveBarrier?: Promise) { return { status: observation && { ...observation, + headSha: remoteHeadSha, title: observation.headRefName, baseRefName: "main", isMerged: observation.state === "merged", @@ -202,15 +210,29 @@ async function createUnobservedWorkspace(archiveBarrier?: Promise) { logger, paseoHome, daemonConfigStore: { get: () => ({ autoArchiveAfterMerge: true }) }, - workspaceGitService: service, + workspaceGitService: { + onSnapshotUpdated: service.onSnapshotUpdated.bind(service), + getSnapshot: async ( + snapshotCwd: string, + readOptions: { force: true; afterInFlight?: boolean; reason: string }, + ) => { + if (changeBeforeArchiveRead && readOptions.reason === "auto-archive-on-merge") { + changeBeforeArchiveRead = false; + writeFileSync(path.join(cwd, "README.md"), "changed before archive\n"); + run(cwd, ["add", "README.md"]); + run(cwd, ["-c", "commit.gpgsign=false", "commit", "-m", "new commit"]); + } + return service.getSnapshot(snapshotCwd, readOptions); + }, + }, getAutoArchivedChangeRequestUrl: async () => null, listActiveWorkspaces: async () => active ? [{ workspaceId: "workspace-under-test", cwd, kind: "worktree" }] : [], } as unknown as AutoArchiveOnMergeOptions; setupAutoArchiveOnMerge(options, { resolvePath: path.resolve, - archiveIfSafe: async (input) => - archiveIfSafe({ + archiveIfSafe: async (input) => { + await archiveIfSafe({ ...input, deps: { isPaseoOwnedWorktreeCwd, @@ -227,13 +249,19 @@ async function createUnobservedWorkspace(archiveBarrier?: Promise) { }; }, }, - }), + }); + archiveDecisions++; + }, }); return { archives, + archiveDecisions: () => archiveDecisions, reads, metrics: () => service.getMetrics(), isActive: () => active, + 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,12 +298,33 @@ afterEach(async () => { }); describe("workspace auto-archive behavior", () => { + test("rechecks checkout changes made after the merge notification", async () => { + const workspace = await createUnobservedWorkspace({ changeBeforeArchiveRead: true }); + await workspace.observe("open"); + await workspace.observe("merged"); + await vi.waitFor(() => expect(workspace.archiveDecisions()).toBe(1)); + expect(workspace.archives).toEqual([]); + expect(workspace.isActive()).toBe(true); + }); + + test("keeps a merged PR visible and the workspace active when its remote head advances", async () => { + const workspace = await createUnobservedWorkspace(); + const open = await workspace.observe("open"); + workspace.advanceRemoteHead(); + const merged = await workspace.observe("merged"); + await vi.waitFor(() => expect(workspace.archiveDecisions()).toBe(1)); + expect(merged.forge.pullRequest?.url).toBe(open.forge.pullRequest?.url); + expect(merged.forge.pullRequest?.headSha).not.toBe(merged.git.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) => { releaseArchive = resolve; }); - const workspace = await createUnobservedWorkspace(archiveBarrier); + const workspace = await createUnobservedWorkspace({ archiveBarrier }); try { await workspace.observe("open"); expect(workspace.archives).toEqual([]); @@ -292,10 +341,10 @@ 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. expect(workspace.reads).toEqual([ "workspace-journey", "workspace-journey", + "auto-archive-on-merge", "workspace-journey", ]); } finally { 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.ts b/packages/server/src/server/workspace-git-service.ts index a7225d6b1e..295d579265 100644 --- a/packages/server/src/server/workspace-git-service.ts +++ b/packages/server/src/server/workspace-git-service.ts @@ -155,6 +155,7 @@ export interface WorkspaceGitRuntimeSnapshot { repoRoot: string | null; mainRepoRoot: string | null; currentBranch: string | null; + headSha?: string; remoteUrl: string | null; isPaseoOwnedWorktree: boolean; isDirty: boolean | null; @@ -185,6 +186,7 @@ export interface WorkspaceGitRuntimeSnapshot { state: string; baseRefName: string; headRefName: string; + headSha?: string; isMerged: boolean; isDraft?: boolean; mergeable?: PullRequestMergeable; @@ -320,6 +322,7 @@ export type WorkspaceGitSnapshotOptions = | { force: true; includeForge?: boolean; + afterInFlight?: boolean; reason: string; }; @@ -3250,7 +3253,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { refreshWorktree: true, includeForge: options?.includeForge ?? true, reason: options?.reason ?? defaultReason, - queueIfBusy: false, + queueIfBusy: force && options?.afterInFlight === true, movedRemoteRefs: new Set(), }; } @@ -3550,6 +3553,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { repoRoot: checkoutStatus.repoRoot, mainRepoRoot: checkoutStatus.mainRepoRoot, currentBranch: checkoutStatus.currentBranch, + headSha: facts.isGit ? facts.pullRequestLookupTarget?.headSha : undefined, remoteUrl: checkoutStatus.remoteUrl, isPaseoOwnedWorktree: checkoutStatus.isPaseoOwnedWorktree, isDirty: refreshWorktree diff --git a/packages/server/src/services/forge-service.ts b/packages/server/src/services/forge-service.ts index 8499cc8756..d676304f79 100644 --- a/packages/server/src/services/forge-service.ts +++ b/packages/server/src/services/forge-service.ts @@ -128,6 +128,7 @@ export interface CurrentPullRequestStatus { state: string; baseRefName: string; headRefName: string; + headSha?: string; isMerged: boolean; isDraft?: boolean; mergeable: PullRequestMergeable; diff --git a/packages/server/src/services/gitea-service.test.ts b/packages/server/src/services/gitea-service.test.ts index 9e3a3ecc62..44d263791f 100644 --- a/packages/server/src/services/gitea-service.test.ts +++ b/packages/server/src/services/gitea-service.test.ts @@ -2064,7 +2064,7 @@ describe("createGiteaService", () => { ]); }); - it("does not attach a stale Gitea-family PR after a same-name branch advances", async () => { + it("keeps a merged PR when its remote head advances beyond the checkout", async () => { const stale = currentPullRequestApi({ number: 8, state: "closed", @@ -2086,7 +2086,7 @@ describe("createGiteaService", () => { headRef: "dev", headSha: "2222222222222222222222222222222222222222", }), - ).resolves.toBeNull(); + ).resolves.toMatchObject({ number: 8, state: "merged" }); }); it("returns null when no PR matches the current branch", async () => { diff --git a/packages/server/src/services/gitea-service.ts b/packages/server/src/services/gitea-service.ts index 892f9cc33a..ef1df9f888 100644 --- a/packages/server/src/services/gitea-service.ts +++ b/packages/server/src/services/gitea-service.ts @@ -162,6 +162,7 @@ const GiteaPrListItemSchema = z updated: z.string().optional(), labels: z.string().optional(), ci: z.string().optional(), + headSha: z.string().optional(), }) .passthrough(); @@ -1010,6 +1011,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 +1084,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: [], @@ -1653,6 +1656,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For input.headSha !== undefined && item.head.sha === input.headSha, ) ?? + candidates[0] ?? null; return match ? currentPullRequestApiToListItem(match) : null; } @@ -1718,7 +1722,12 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For combined, actionTasks, ); - return applyGiteaChecks(status, combined, actionTasks, actionRequiredRunIds); + return applyGiteaChecks( + { ...status, headSha: pr.headSha }, + combined, + actionTasks, + actionRequiredRunIds, + ); } catch { return status; } diff --git a/packages/server/src/services/github-service.test.ts b/packages/server/src/services/github-service.test.ts index 3109bcc63c..0b97f551bb 100644 --- a/packages/server/src/services/github-service.test.ts +++ b/packages/server/src/services/github-service.test.ts @@ -23,7 +23,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,headRefOid,mergedAt,reviewDecision,mergeable,headRepositoryOwner"; + "number,url,title,state,isDraft,baseRefName,headRefName,headRefOid,updatedAt,mergedAt,reviewDecision,mergeable,headRepositoryOwner"; const CURRENT_PR_STATUS_FIELDS = `${CURRENT_PR_STATUS_BASE_FIELDS},statusCheckRollup`; const FORK_PR_QUERY = "state=all&per_page=100&head=forkOwner%3Afeature%2Ffork"; @@ -3219,7 +3219,7 @@ 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({ @@ -3243,8 +3243,9 @@ describe("ForgeService", () => { expect(status?.state).toBe("merged"); }); - it("does not attach a stale merged PR after a same-name branch advances", async () => { + it("keeps a merged PR when its remote head advances beyond the checkout", async () => { const stale = currentPullRequestJson({ + headRepositoryOwner: { login: "parentOwner" }, state: "MERGED", mergedAt: "2026-07-17T12:00:00Z", headRefOid: "1111111111111111111111111111111111111111", @@ -3265,12 +3266,105 @@ describe("ForgeService", () => { headRef: "feature/fork", headSha: "2222222222222222222222222222222222222222", }), - ).resolves.toBeNull(); + ).resolves.toMatchObject({ state: "merged" }); expect(runner.calls.some((call) => call.args[0] === "pr" && call.args[1] === "list")).toBe( true, ); }); + it("prefers an exact terminal head and otherwise the most recently updated terminal PR", async () => { + const owner = { login: "forkOwner" }; + const terminal = ({ + number, + headRefOid, + updatedAt, + state, + }: { + number: number; + headRefOid: string; + updatedAt: string; + state: string; + }) => + JSON.parse( + currentPullRequestJson({ + number, + headRefOid, + updatedAt, + state, + mergedAt: state === "MERGED" ? updatedAt : null, + headRepositoryOwner: owner, + }), + ); + const older = terminal({ + number: 41, + headRefOid: "old", + updatedAt: "2026-07-01T00:00:00Z", + state: "MERGED", + }); + const newer = terminal({ + number: 42, + headRefOid: "new", + updatedAt: "2026-07-02T00:00:00Z", + state: "CLOSED", + }); + for (const [headSha, expected] of [ + ["old", 41], + ["other", 42], + ] as const) { + const runner = createScriptedRunner([JSON.stringify(older), JSON.stringify([older, newer])]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headRepositoryOwner: "forkOwner", + headSha, + }); + expect(status?.number).toBe(expected); + } + }); + + it("prefers an exact merged PR over a closed draft", async () => { + const owner = { login: "forkOwner" }; + const closedDraft = JSON.parse( + currentPullRequestJson({ + number: 1, + state: "CLOSED", + isDraft: true, + headRefOid: "old", + updatedAt: "2026-07-01T00:00:00Z", + headRepositoryOwner: owner, + }), + ); + const merged = JSON.parse( + currentPullRequestJson({ + number: 2, + state: "MERGED", + mergedAt: "2026-07-02T00:00:00Z", + headRefOid: "exact", + updatedAt: "2026-07-02T00:00:00Z", + headRepositoryOwner: owner, + }), + ); + const runner = createScriptedRunner([ + JSON.stringify(closedDraft), + JSON.stringify([closedDraft, merged]), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headRepositoryOwner: "forkOwner", + headSha: "exact", + }); + expect(status).toMatchObject({ number: 2, state: "merged" }); + }); + it("prefers an open PR over an exact-SHA merged PR for the same head", async () => { const checkoutSha = "2222222222222222222222222222222222222222"; const owner = { login: "forkOwner" }; @@ -3313,6 +3407,84 @@ describe("ForgeService", () => { expect(status).toMatchObject({ number: 43, state: "open" }); }); + it("keeps a merged PR targeting the fork itself", async () => { + const runner = createScriptedRunner([ + currentPullRequestJson({ + number: 7, + url: "https://github.com/alice/app/pull/7", + state: "MERGED", + mergedAt: "2026-07-17T12:00:00Z", + headRefOid: "checkout-head", + headRepositoryOwner: { login: "alice" }, + }), + JSON.stringify({ + owner: { login: "alice" }, + name: "app", + parent: { owner: { login: "acme" }, name: "app" }, + }), + "[]", + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headSha: "checkout-head", + }); + expect(status).toMatchObject({ number: 7, repoOwner: "alice", state: "merged" }); + }); + + it("compares the fork's terminal PR against parent PRs by state, head and update time", async () => { + const local = JSON.parse( + currentPullRequestJson({ + number: 7, + url: "https://github.com/alice/app/pull/7", + state: "MERGED", + mergedAt: "2026-07-17T12:00:00Z", + updatedAt: "2026-07-17T12:00:00Z", + headRefOid: "local-head", + headRepositoryOwner: { login: "alice" }, + }), + ); + for (const [state, headSha, updatedAt, expected] of [ + ["OPEN", "other", "2026-07-16T12:00:00Z", 8], + ["CLOSED", "other", "2026-07-18T12:00:00Z", 7], + ["CLOSED", "other", "2026-07-18T12:00:00Z", 8], + ] as const) { + const parent = { + ...local, + number: 8, + url: "https://github.com/acme/app/pull/8", + state, + mergedAt: null, + headRefOid: headSha, + updatedAt, + }; + const runner = createScriptedRunner([ + JSON.stringify(local), + JSON.stringify({ + owner: { login: "alice" }, + name: "app", + parent: { owner: { login: "acme" }, name: "app" }, + }), + JSON.stringify([{ number: 8 }]), + JSON.stringify(parent), + ]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + }); + const status = await service.getCurrentPullRequestStatus({ + cwd: "/repo", + headRef: "feature/fork", + headSha: state === "OPEN" || expected === 7 ? "local-head" : "neither", + }); + expect(status?.number).toBe(expected); + } + }); + it("resolves fork PR heads to the parent repository when gh pr view returns a stale branch match", async () => { const runner = createScriptedRunner([ currentPullRequestJson({ diff --git a/packages/server/src/services/github-service.ts b/packages/server/src/services/github-service.ts index 2dd3fdc269..04731b63d2 100644 --- a/packages/server/src/services/github-service.ts +++ b/packages/server/src/services/github-service.ts @@ -346,6 +346,7 @@ const CurrentPullRequestStatusSchema = z.object({ baseRefName: z.string().catch(""), headRefName: z.string().catch(""), headRefOid: z.string().optional(), + updatedAt: z.string().optional(), mergedAt: z.string().nullable().optional(), statusCheckRollup: z.unknown().optional(), reviewDecision: z.unknown().optional(), @@ -524,7 +525,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"; const CURRENT_PR_STATUS_FIELDS = `${CURRENT_PR_STATUS_BASE_FIELDS},statusCheckRollup`; const PULL_REQUEST_STATUS_FACTS_QUERY = ` @@ -640,6 +641,7 @@ const BatchPollPrNodeSchema = z.object({ baseRefName: z.string().catch(""), headRefName: z.string().catch(""), headRefOid: z.string().catch(""), + updatedAt: z.string().optional(), mergedAt: z.string().nullable().optional(), mergeable: PullRequestMergeableSchema.optional().default("UNKNOWN"), reviewDecision: z.unknown().optional(), @@ -744,6 +746,7 @@ fragment PaseoPollPullRequest on PullRequest { baseRefName headRefName headRefOid + updatedAt mergedAt mergeable reviewDecision @@ -1118,6 +1121,7 @@ interface ResolvedPullRequestCandidate { status: CurrentPullRequestStatus; headSha?: string; headRepositoryOwner?: string; + updatedAt?: string; } export function createGitHubService(options: CreateGitHubServiceOptions = {}): GitHubService { @@ -2968,7 +2972,7 @@ async function resolveCurrentPullRequestView(options: { headRepositoryOwner: options.headRepositoryOwner, }) : null; - if (viewMatch) { + if (viewMatch?.status.state === "open") { return viewMatch.status; } @@ -2980,12 +2984,13 @@ async function resolveCurrentPullRequestView(options: { const parentOwner = repo?.parent?.owner?.login; const parentName = repo?.parent?.name; if (!forkOwner) { - return null; + return viewMatch?.status ?? null; } if (parentOwner && parentName) { return resolveForkPullRequestView({ ...options, forkOwner, + viewMatch, repo: `${parentOwner}/${parentName}`, }); } @@ -2998,7 +3003,7 @@ async function resolveCurrentPullRequestView(options: { run: options.run, }); const match = pickPullRequestCandidate({ - candidates, + candidates: viewMatch ? [...candidates, viewMatch] : candidates, headRef: options.headRef, headSha: options.headSha, headRepositoryOwner, @@ -3019,6 +3024,7 @@ async function resolveForkPullRequestView(options: { headRef: string; headSha?: string; forkOwner: string; + viewMatch: ResolvedPullRequestCandidate | null; repo: string; run: (args: string[], options: GitHubCommandRunnerOptions) => Promise; }): Promise { @@ -3034,12 +3040,15 @@ async function resolveForkPullRequestView(options: { ), ); const match = pickPullRequestCandidate({ - candidates: candidates.filter( - (candidate): candidate is ResolvedPullRequestCandidate => candidate !== null, - ), + candidates: [ + ...candidates.filter( + (candidate): candidate is ResolvedPullRequestCandidate => + candidate !== null && candidate.headRepositoryOwner === options.forkOwner, + ), + ...(options.viewMatch ? [options.viewMatch] : []), + ], headRef: options.headRef, headSha: options.headSha, - headRepositoryOwner: options.forkOwner, }); return match?.status ?? null; } @@ -3337,6 +3346,7 @@ function toBatchCurrentPullRequestItem( baseRefName: node.baseRefName, headRefName: node.headRefName, ...(node.headRefOid ? { headRefOid: node.headRefOid } : {}), + updatedAt: node.updatedAt, mergedAt: node.mergedAt ?? null, ...(statusCheckRollup ? { statusCheckRollup } : {}), reviewDecision: node.reviewDecision, @@ -3357,6 +3367,7 @@ function toCurrentPullRequestCandidate( const headSha = item.headRefOid; return { status, + updatedAt: item.updatedAt, ...(headSha ? { headSha } : {}), ...(headRepositoryOwner ? { headRepositoryOwner } : {}), }; @@ -3380,12 +3391,6 @@ function pickPullRequestCandidate(options: { 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; } @@ -3411,17 +3416,17 @@ function comparePullRequestCandidatePreference( if (leftExact !== rightExact) { return leftExact ? -1 : 1; } - return 0; + return ( + (right.updatedAt ?? "").localeCompare(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"; @@ -3969,6 +3974,7 @@ function toCurrentPullRequestStatus( state, baseRefName: item.baseRefName, headRefName: item.headRefName || fallbackHeadRefName, + ...(item.headRefOid ? { headSha: item.headRefOid } : {}), isMerged: mergedAt !== null, isDraft: item.isDraft ?? false, mergeable: item.mergeable, diff --git a/packages/server/src/services/gitlab-service.test.ts b/packages/server/src/services/gitlab-service.test.ts index b9b6d447d4..81e8bc966d 100644 --- a/packages/server/src/services/gitlab-service.test.ts +++ b/packages/server/src/services/gitlab-service.test.ts @@ -357,14 +357,16 @@ describe("createGitLabService", () => { expect(calls[1]).toEqual(["mr", "view", "270", "-F", "json"]); }); - it("does not attach the latest historical merge request after a reused branch advances", async () => { + it("keeps a merged MR when its remote head advances beyond the checkout", 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) => + ok(JSON.stringify(args[1] === "list" ? [stale] : stale)), + ); await expect( service.getCurrentPullRequestStatus({ @@ -372,8 +374,8 @@ describe("createGitLabService", () => { headRef: "dev", headSha: "2222222222222222222222222222222222222222", }), - ).resolves.toBeNull(); - expect(calls).toEqual([currentMrListArgs("dev")]); + ).resolves.toMatchObject({ 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..4fde7173f9 100644 --- a/packages/server/src/services/gitlab-service.ts +++ b/packages/server/src/services/gitlab-service.ts @@ -728,6 +728,7 @@ function toCurrentPullRequestStatus( state: mapMergeRequestState(mr.state), baseRefName: mr.target_branch, headRefName: mr.source_branch, + ...(mr.sha ? { headSha: mr.sha } : {}), isMerged: mr.state === "merged" || mr.merged_at != null, isDraft: mr.draft ?? mr.work_in_progress ?? false, mergeable: mapMergeable(mr), @@ -987,6 +988,7 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F (mr) => mapMergeRequestState(mr.state) !== "open" && headSha !== undefined && mr.sha === headSha, ) ?? + candidates[0] ?? null; return match ? viewMergeRequest(cwd, String(match.iid)) : null; } diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index bd25f46770..a0cbaa7afc 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -4223,6 +4223,7 @@ export interface PullRequestStatus { state: string; baseRefName: string; headRefName: string; + headSha?: string; isMerged: boolean; isDraft?: boolean; mergeable?: PullRequestMergeable;