fix(workspaces): preserve pull requests across checkout changes

Keep branch associations when the local commit changes and resolve transferred source identities before matching. Wait for the composer to accept input before queueing in browser tests.
This commit is contained in:
Mohamed Boudra
2026-11-02 07:18:48 +01:00
parent 24ef4c48cc
commit 5feb68da4f
5 changed files with 149 additions and 20 deletions
@@ -84,6 +84,9 @@ export async function typeIntoFocusedComposer(page: Page, text: string): Promise
}
export async function sendDraftToQueue(page: Page): Promise<void> {
await expect(
page.getByRole("button", { name: /^(Send and steer|Send and interrupt)$/ }),
).toBeEnabled({ timeout: 30_000 });
await composerInput(page).press("Control+Enter");
}
@@ -1604,7 +1604,7 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => {
nowMs = 3_000;
await service.refresh(REPO_CWD);
expect(workspaceRuntimeStore.get(REPO_CWD)?.forge.pullRequest).toBeNull();
expect(workspaceRuntimeStore.get(REPO_CWD)?.forge.pullRequest?.title).toBe("Visible PR");
expect(forge.getCurrentPullRequestStatus).not.toHaveBeenCalled();
await vi.advanceTimersByTimeAsync(0);
@@ -1619,6 +1619,57 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => {
}
});
test("keeps the known open association when a checkout commit changes before forge refresh", async () => {
let headSha = "1111111111111111111111111111111111111111";
const open = createCurrentPullRequestStatus({
number: 2,
url: "https://forge-commit.test/acme/repo/pulls/2",
state: "open",
});
const merged = createCurrentPullRequestStatus({
number: 1,
url: "https://forge-commit.test/acme/repo/pulls/1",
state: "merged",
isMerged: true,
});
const forge = { ...createGitHubServiceStub(), retainCurrentPullRequestStatusPoll: undefined };
const unregister = defaultForgeRegistry.register("forge-commit-test", {
createService: () => forge,
matchesHost: (host) => host === "forge-commit.test",
});
let first = true;
const service = createService({
getCheckoutSnapshotFacts: vi.fn(async (cwd) =>
createCheckoutFacts(cwd, {
currentBranch: "feature",
remoteUrl: "https://forge-commit.test/acme/repo.git",
pullRequestLookupTarget: { headRef: "feature", headSha },
}),
),
getCheckoutStatus: vi.fn(async (cwd) =>
createCheckoutStatus(cwd, {
currentBranch: "feature",
remoteUrl: "https://forge-commit.test/acme/repo.git",
}),
),
getPullRequestStatus: vi.fn(async (_cwd, _forge, options) => {
const result = first || options.previousPullRequest?.number === 2 ? open : merged;
first = false;
return { ...createPullRequestStatusResult(), status: result };
}),
});
try {
expect((await service.getSnapshot(REPO_CWD)).forge.pullRequest?.number).toBe(2);
headSha = "2222222222222222222222222222222222222222";
const refreshed = await service.getSnapshot(REPO_CWD, { force: true, reason: "test" });
expect(refreshed.git.headSha).toBe(headSha);
expect(refreshed.forge.pullRequest).toMatchObject({ number: 2, state: "open" });
} finally {
service.dispose();
unregister();
}
});
test("subscription cancels generic forge PR status self-heal polling after unsubscribe", async () => {
const forge = {
...createGitHubServiceStub(),
@@ -3565,7 +3565,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService {
target.lastShellOutAtMs = now.getTime();
const cwd = target.cwd;
const previousForgePrStatusPollKey = this.getForgePrStatusPollKey(target);
const previousForgePrAssociationKey = this.getForgePrAssociationKey(target);
const baseContext: CheckoutContext = {
paseoHome: this.paseoHome,
worktreesRoot: this.worktreesRoot,
@@ -3616,7 +3616,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService {
const loadedAtMs = this.deps.now().getTime();
target.latestGitLoadedAtMs = loadedAtMs;
if (previousForgePrStatusPollKey !== this.getForgePrStatusPollKey(target)) {
if (previousForgePrAssociationKey !== this.getForgePrAssociationKey(target)) {
target.latestForge = buildForgeUnavailableSnapshot();
target.latestForgeLoadedAtMs = target.latestGitLoadedAtMs;
}
@@ -3682,7 +3682,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService {
};
}
private getForgePrStatusPollKey(target: WorkspaceGitTarget): string | null {
private getForgePrAssociationKey(target: WorkspaceGitTarget): string | null {
const git = target.latestGit;
if (!git?.currentBranch || !git.remoteUrl) {
return null;
@@ -3698,7 +3698,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService {
return null;
}
return buildWorkspaceForgePrStatusPollKey({
return buildWorkspaceForgePrAssociationKey({
forge: resolution.forge,
remoteUrl: git.remoteUrl,
target: pollTarget,
@@ -4188,12 +4188,26 @@ function buildWorkspaceForgePrStatusPollKey({
forge: string;
remoteUrl: string;
target: WorkspaceForgePrStatusPollTarget;
}): string {
return JSON.stringify([
buildWorkspaceForgePrAssociationKey({ forge, remoteUrl, target }),
target.headSha ?? null,
]);
}
function buildWorkspaceForgePrAssociationKey({
forge,
remoteUrl,
target,
}: {
forge: string;
remoteUrl: string;
target: WorkspaceForgePrStatusPollTarget;
}): string {
return JSON.stringify([
forge,
remoteUrl,
target.headRef,
target.headSha ?? null,
target.headRepositoryOwner ?? null,
target.headRepositoryPath ?? null,
target.headRepositoryHost ?? null,
@@ -48,6 +48,8 @@ function makeService(responder: Responder, overrides: Partial<CreateGiteaService
return result;
} catch (error) {
if (error instanceof Error && error.message.startsWith("unexpected call:")) {
if (args[0] === "api" && args[1] === "repos/example-user/sample-repo")
return ok(JSON.stringify({ full_name: "example-user/sample-repo" }));
return listedDetail(args) ?? Promise.reject(error);
}
throw error;
@@ -2314,6 +2316,60 @@ describe("createGiteaService", () => {
expect(calls.filter((args) => args[1] === "repos/example-user/sample-repo")).toHaveLength(2);
});
it.each(["open", "merged"] as const)("matches a transferred fork for an %s PR", async (state) => {
const original = currentPullRequestApi({
number: 8,
state: state === "open" ? "open" : "closed",
headRef: "topic",
headSha: "1".repeat(40),
merged: state === "merged",
});
const pr = {
...original,
html_url: "https://gitea.example/base/repo/pulls/8",
head: {
...original.head,
repo: {
...original.head.repo,
id: 2,
full_name: "new-owner/repo",
owner: { login: "new-owner" },
},
},
base: { ...original.base, repo: { id: 1, owner: { login: "base" } } },
};
const { service } = makeService(
(args) => {
if (args[0] === "pr" && args[1] === "list")
return ok(
JSON.stringify(
state === "open"
? [{ ...OPEN_PR, index: "8", head: "new-owner:topic", url: pr.html_url }]
: [],
),
);
if (args[0] === "api" && args[1].endsWith("/pulls/8")) return ok(JSON.stringify(pr));
if (args[0] === "api" && args[1] === "repos/old-owner/repo")
return ok(JSON.stringify({ full_name: "new-owner/repo" }));
if (args[0] === "api" && args[1].includes("pulls?state=all"))
return ok(JSON.stringify([pr]));
if (args[0] === "pr" && args[1] === "8") return ok(JSON.stringify(STATUS_PR_VIEW));
if (args[0] === "api" && args[1].includes("/commits/"))
return ok(JSON.stringify({ state: "", statuses: [] }));
throw new Error(`unexpected call: ${args.join(" ")}`);
},
{ resolveRemoteUrl: async () => "https://gitea.example/base/repo.git" },
);
expect(
await service.getCurrentPullRequestStatus({
cwd: "/repo",
headRef: "topic",
headRepositoryOwner: "old-owner",
headRepositoryPath: "old-owner/repo",
}),
).toMatchObject({ number: 8, state });
});
it.each(["merged", "open"] as const)(
"finds a recent %s candidate after 200 unrelated open requests",
async (state) => {
+19 -14
View File
@@ -1682,6 +1682,15 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
canonicalRepositoryPaths.set(key, { path: result });
return result;
};
const matchesHead = async (item: GiteaPrListItem): Promise<boolean> => {
if (matchesCurrentHeadRef(item, input.headRef, expectedHeadOwner)) return true;
if (stripHeadOwner(item.head) !== input.headRef) return false;
canonicalLookup ??= loadCanonicalPath();
const canonicalPath = await canonicalLookup;
return canonicalPath
? matchesCurrentHeadRef(item, input.headRef, canonicalPath.split("/")[0] ?? null)
: false;
};
const matchesRepository = async (item: GiteaCurrentPullRequestApi): Promise<boolean> => {
const headPath =
item.head.repo?.full_name ??
@@ -1709,7 +1718,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
};
const open = await findOpenPullRequestForHeadRef(
input,
expectedHeadOwner,
matchesHead,
repoPath,
matchesRepository,
expectedPath,
@@ -1720,7 +1729,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
// Gitea does not expose a source-branch filter for this endpoint.
return resolveRecentPullRequest({
...input,
expectedHeadOwner,
matchesHead,
repoPath,
matchesRepository,
exhausted: open.exhausted,
@@ -1731,7 +1740,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
cwd: string;
headRef: string;
previousPullRequest?: { number?: number; url: string; state: string } | null;
expectedHeadOwner: string | null;
matchesHead: (item: GiteaPrListItem) => Promise<boolean>;
repoPath: string;
matchesRepository: (item: GiteaCurrentPullRequestApi) => Promise<boolean>;
exhausted: boolean;
@@ -1740,7 +1749,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
try {
recent = await findRecentPullRequestForHeadRef(
input,
input.expectedHeadOwner,
input.matchesHead,
input.repoPath,
input.matchesRepository,
);
@@ -1771,7 +1780,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
cwd: string;
headRef: string;
repoPath: string;
expectedHeadOwner: string | null;
matchesHead: (item: GiteaPrListItem) => Promise<boolean>;
matchesRepository: (item: GiteaCurrentPullRequestApi) => Promise<boolean>;
previousPullRequest?: { number?: number; url: string; state: string } | null;
recent: GiteaPrListItem | null;
@@ -1794,7 +1803,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
);
const candidate = currentPullRequestApiToListItem(current);
return candidate.url === previous.url &&
matchesCurrentHeadRef(candidate, input.headRef, input.expectedHeadOwner) &&
(await input.matchesHead(candidate)) &&
(await input.matchesRepository(current))
? candidate
: null;
@@ -1802,7 +1811,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
async function findRecentPullRequestForHeadRef(
input: { cwd: string; headRef: string },
expectedHeadOwner: string | null,
matchesHead: (item: GiteaPrListItem) => Promise<boolean>,
repoPath: string,
matchesRepository: (item: GiteaCurrentPullRequestApi) => Promise<boolean>,
): Promise<GiteaPrListItem | null> {
@@ -1820,11 +1829,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
const candidates: GiteaCurrentPullRequestApi[] = [];
for (const item of items) {
if (
matchesCurrentHeadRef(
currentPullRequestApiToListItem(item),
input.headRef,
expectedHeadOwner,
) &&
(await matchesHead(currentPullRequestApiToListItem(item))) &&
(await matchesRepository(item))
)
candidates.push(item);
@@ -1856,7 +1861,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
headSha?: string;
force?: boolean;
},
expectedHeadOwner: string | null,
matchesHead: (item: GiteaPrListItem) => Promise<boolean>,
repoPath: string,
matchesRepository: (item: GiteaCurrentPullRequestApi) => Promise<boolean>,
expectedPath: string,
@@ -1871,7 +1876,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
page,
});
for (const item of items) {
if (!matchesCurrentHeadRef(item, input.headRef, expectedHeadOwner)) continue;
if (!(await matchesHead(item))) continue;
const identityKey = JSON.stringify([
input.cwd,
repoPath,