Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,57 @@ describe("PullRequestRuntimeManager direct checkout PR linking", () => {
]);
});

test("does not attach a stale closed PR when workspace HEAD has moved past it (#4513)", async () => {
const state = makeState("master");
state.workspace = {
...state.workspace,
headSha: "current-master-sha",
upstreamOwner: "base-owner",
upstreamRepo: "base-repo",
upstreamBranch: "master",
pullRequestId: null,
};
const manager = createManager(state, {
execGh: async (args) => {
const path = args.find((arg) => arg.startsWith("repos/"));
if (path === "repos/base-owner/base-repo/pulls") {
return [
{
number: 42,
title: "Old closed PR on master",
html_url: "https://github.com/base-owner/base-repo/pull/42",
state: "closed",
draft: false,
merged_at: null,
updated_at: "2025-01-01T00:00:00Z",
head: {
ref: "master",
sha: "stale-closed-sha",
repo: {
name: "base-repo",
owner: { login: "base-owner" },
},
},
base: {
repo: {
full_name: "base-owner/base-repo",
},
},
},
];
}
if (path?.endsWith("/reviews")) return [];
if (path?.endsWith("/check-runs")) return { check_runs: [] };
if (path?.endsWith("/statuses")) return [];
return null;
},
});

await manager.refreshPullRequestsByWorkspaces([WORKSPACE_ID]);

expect(state.workspace.pullRequestId).toBeNull();
});

test("preserves existing pullRequestId when head lookup fails", async () => {
const state = makeState("fix/sidebar");
state.workspace = {
Expand Down
34 changes: 29 additions & 5 deletions packages/host-service/src/runtime/pull-requests/pull-requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,19 @@ function mapCheckoutPullRequestState(
return "open";
}

// Historical PRs (closed / merged) only attach when the workspace HEAD still
// points at the PR head commit — otherwise a long-closed PR on a long-moved
// branch (e.g. a closed PR on `master`) keeps showing up on every poll.
// Matches V1 `shouldAcceptPRMatch` in `pr-resolution.ts`.
function shouldAttachHistoricalMatch(
match: { state: PullRequestState; headSha: string },
workspaceHeadSha: string | null,
): boolean {
if (match.state === "open" || match.state === "draft") return true;
if (!workspaceHeadSha) return false;
return match.headSha.toLowerCase() === workspaceHeadSha.trim().toLowerCase();
}

function deriveCheckoutPullRequestUpstream(
repo: NormalizedRepoIdentity,
pr: CheckoutPullRequestMetadata,
Expand Down Expand Up @@ -669,7 +682,7 @@ export class PullRequestRuntimeManager {
continue;
}
const match = keyToPullRequest.get(key);
if (match) {
if (match && shouldAttachHistoricalMatch(match, workspace.headSha)) {
this.db
.update(workspaces)
.set({ pullRequestId: match.id })
Expand Down Expand Up @@ -926,10 +939,16 @@ export class PullRequestRuntimeManager {
wantedRefs: Map<string, GitHubPullRequestHeadRef>,
options: { bypassCache?: boolean } = {},
): Promise<{
matched: Map<string, { id: string }>;
matched: Map<
string,
{ id: string; state: PullRequestState; headSha: string }
>;
failedKeys: Set<string>;
}> {
const matched = new Map<string, { id: string }>();
const matched = new Map<
string,
{ id: string; state: PullRequestState; headSha: string }
>();
const failedKeys = new Set<string>();
if (wantedRefs.size === 0) return { matched, failedKeys };

Expand Down Expand Up @@ -1031,14 +1050,15 @@ export class PullRequestRuntimeManager {
const reviewDecision = reviewDecisionByNumber.has(node.number)
? mapReviewDecision(reviewDecisionByNumber.get(node.number) ?? null)
: coerceReviewDecision(existing?.reviewDecision ?? null);
const mappedState = mapPullRequestState(node.state, node.isDraft);
const rowId = this.upsertPullRequestRow({
existing,
projectId,
prNumber: node.number,
repo,
url: node.url,
title: node.title,
state: mapPullRequestState(node.state, node.isDraft),
state: mappedState,
isDraft: node.isDraft,
headBranch: node.headRefName,
headSha: node.headRefOid,
Expand All @@ -1050,7 +1070,11 @@ export class PullRequestRuntimeManager {
now,
});

matched.set(key, { id: rowId });
matched.set(key, {
id: rowId,
state: mappedState,
headSha: node.headRefOid,
});
}

return { matched, failedKeys };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,66 @@ describe("GitHub pull request REST queries", () => {
]);
});

test("prefers an open PR over a more recently updated closed PR (#4513)", async () => {
const { execGh } = createExecGh([
[
{
number: 100,
title: "Stale closed",
html_url: "https://github.com/superset-sh/superset/pull/100",
state: "closed",
draft: false,
merged_at: null,
updated_at: "2026-05-09T12:00:00Z",
head: {
ref: "feat/x",
sha: "closed-sha",
repo: {
name: "superset",
owner: { login: "superset-sh" },
},
},
base: {
repo: {
full_name: "superset-sh/superset",
},
},
},
{
number: 101,
title: "Active open",
html_url: "https://github.com/superset-sh/superset/pull/101",
state: "open",
draft: false,
merged_at: null,
updated_at: "2026-05-08T12:00:00Z",
head: {
ref: "feat/x",
sha: "open-sha",
repo: {
name: "superset",
owner: { login: "superset-sh" },
},
},
base: {
repo: {
full_name: "superset-sh/superset",
},
},
},
],
]);

const result = await fetchPullRequestByHeadFromGh(
execGh,
{ owner: "superset-sh", name: "superset" },
{ owner: "superset-sh", repo: "superset", branch: "feat/x" },
);

expect(result?.number).toBe(101);
expect(result?.state).toBe("OPEN");
});

test("filters REST head candidates by exact upstream repository", async () => {
const { execGh } = createExecGh([
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,24 +118,40 @@ function headKey(
return `${owner.toLowerCase()}/${repo.toLowerCase()}#${branch}`;
}

function pullRequestStateRank(state: PullRequestState): number {
if (state === "OPEN") return 2;
if (state === "MERGED") return 1;
return 0;
}

function normalizePullRequestCandidates(
raw: unknown,
head: GitHubPullRequestHeadRef,
): GitHubPullRequestNode | null {
const requestedKey = headKey(head.owner, head.repo, head.branch);
return (
asArray(raw)
.map((item) => normalizePullRequest(item))
.find(
(node) =>
node &&
headKey(
node.headRepositoryOwner?.login,
node.headRepository?.name,
node.headRefName,
) === requestedKey,
) ?? null
);
const matches = asArray(raw)
.map((item) => normalizePullRequest(item))
.filter(
(node): node is GitHubPullRequestNode =>
node !== null &&
headKey(
node.headRepositoryOwner?.login,
node.headRepository?.name,
node.headRefName,
) === requestedKey,
);

// Prefer OPEN > MERGED > CLOSED so an active PR isn't shadowed by a more
// recently updated closed one. Ties fall back to updatedAt desc (matching
// the API's sort=updated&direction=desc).
matches.sort((left, right) => {
const stateDelta =
pullRequestStateRank(right.state) - pullRequestStateRank(left.state);
if (stateDelta !== 0) return stateDelta;
return Date.parse(right.updatedAt) - Date.parse(left.updatedAt);
});

return matches[0] ?? null;
}

function mapReviewDecision(
Expand Down