From 670e4be5c722239ba3b481eb563d540169d140af Mon Sep 17 00:00:00 2001 From: Frank Hamand Date: Wed, 8 Jul 2026 12:27:09 +0100 Subject: [PATCH 1/2] fix(git): keep task PR state in sync when the primary PR changes Promoting a different PR to primary reordered the URL list but broadcast the old primary's cached prState alongside the new primary's URL, so a task could show "merged" while its primary PR was actually open until the next background poll (~60s later) overwrote the cache. setPrimaryPrUrl now fetches the promoted PR's live state via getPrDetailsByUrl + mapPrState, persists it through updatePrCache, and emits the fresh state, so the cache columns and prUrls[0] can no longer disagree and every taskPrInfoChanged consumer updates immediately. Generated-By: PostHog Code Task-Id: 2772f1aa-cba4-46df-a8b6-3a14a0f15299 --- .../host-router/src/ports/git-pr-status.ts | 2 +- .../src/services/git/task-pr-status.test.ts | 48 ++++++++++++++++--- .../src/services/git/task-pr-status.ts | 14 ++++-- 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/packages/host-router/src/ports/git-pr-status.ts b/packages/host-router/src/ports/git-pr-status.ts index eadfaed4b5..9fc55205a2 100644 --- a/packages/host-router/src/ports/git-pr-status.ts +++ b/packages/host-router/src/ports/git-pr-status.ts @@ -13,5 +13,5 @@ export interface IGitPrStatus { cloudPrUrl: string | null, ): Promise; getCachedPrUrl(taskId: string): CachedPrUrlOutput; - setPrimaryPrUrl(taskId: string, prUrl: string): void; + setPrimaryPrUrl(taskId: string, prUrl: string): Promise; } diff --git a/packages/workspace-server/src/services/git/task-pr-status.test.ts b/packages/workspace-server/src/services/git/task-pr-status.test.ts index b2492f7442..576c2e011c 100644 --- a/packages/workspace-server/src/services/git/task-pr-status.test.ts +++ b/packages/workspace-server/src/services/git/task-pr-status.test.ts @@ -215,14 +215,15 @@ describe("TaskPrStatusService revalidation PR detection", () => { }); describe("TaskPrStatusService.setPrimaryPrUrl", () => { - it("emits the promoted url as prUrl even though the row column is stale", () => { - const PR_OLD = "https://github.com/acme/repo/pull/1"; - const PR_NEW = "https://github.com/acme/repo/pull/2"; - const gitService = {} as unknown as GitService; + const PR_OLD = "https://github.com/acme/repo/pull/1"; + const PR_NEW = "https://github.com/acme/repo/pull/2"; + + function makeService(getPrDetailsByUrl: ReturnType) { + const gitService = { getPrDetailsByUrl } as unknown as GitService; const workspaceService = { emit: vi.fn() }; const workspaceRepo = { promotePrUrl: vi.fn(), - findByTaskId: vi.fn().mockReturnValue({ prUrl: PR_OLD, prState: "open" }), + updatePrCache: vi.fn(), getPrUrls: vi.fn().mockReturnValue([PR_NEW, PR_OLD]), }; const service = new TaskPrStatusService( @@ -230,10 +231,25 @@ describe("TaskPrStatusService.setPrimaryPrUrl", () => { workspaceRepo as unknown as IWorkspaceRepository, workspaceService as unknown as WorkspaceService, ); + return { service, workspaceService, workspaceRepo }; + } + + it("recomputes and emits the promoted PR's live state, not the stale cache", async () => { + const getPrDetailsByUrl = vi + .fn() + .mockResolvedValue({ state: "open", merged: false, draft: false }); + const { service, workspaceService, workspaceRepo } = + makeService(getPrDetailsByUrl); - service.setPrimaryPrUrl("task-1", PR_NEW); + await service.setPrimaryPrUrl("task-1", PR_NEW); expect(workspaceRepo.promotePrUrl).toHaveBeenCalledWith("task-1", PR_NEW); + expect(getPrDetailsByUrl).toHaveBeenCalledWith(PR_NEW); + expect(workspaceRepo.updatePrCache).toHaveBeenCalledWith("task-1", { + prUrl: PR_NEW, + prState: "open", + accumulate: false, + }); expect(workspaceService.emit).toHaveBeenCalledWith("taskPrInfoChanged", { taskId: "task-1", prUrl: PR_NEW, @@ -241,4 +257,24 @@ describe("TaskPrStatusService.setPrimaryPrUrl", () => { prState: "open", }); }); + + it("emits a null state when the promoted PR's details are unavailable", async () => { + const getPrDetailsByUrl = vi.fn().mockResolvedValue(null); + const { service, workspaceService, workspaceRepo } = + makeService(getPrDetailsByUrl); + + await service.setPrimaryPrUrl("task-1", PR_NEW); + + expect(workspaceRepo.updatePrCache).toHaveBeenCalledWith("task-1", { + prUrl: PR_NEW, + prState: null, + accumulate: false, + }); + expect(workspaceService.emit).toHaveBeenCalledWith("taskPrInfoChanged", { + taskId: "task-1", + prUrl: PR_NEW, + prUrls: [PR_NEW, PR_OLD], + prState: null, + }); + }); }); diff --git a/packages/workspace-server/src/services/git/task-pr-status.ts b/packages/workspace-server/src/services/git/task-pr-status.ts index 09c98723b6..a69bd7331d 100644 --- a/packages/workspace-server/src/services/git/task-pr-status.ts +++ b/packages/workspace-server/src/services/git/task-pr-status.ts @@ -48,14 +48,22 @@ export class TaskPrStatusService { }; } - setPrimaryPrUrl(taskId: string, prUrl: string): void { + async setPrimaryPrUrl(taskId: string, prUrl: string): Promise { this.workspaceRepo.promotePrUrl(taskId, prUrl); - const row = this.workspaceRepo.findByTaskId(taskId); + const details = await this.gitService.getPrDetailsByUrl(prUrl); + const prState: SidebarPrState = details + ? mapPrState(details.state, details.merged, details.draft) + : null; + this.workspaceRepo.updatePrCache(taskId, { + prUrl, + prState, + accumulate: false, + }); this.workspaceService.emit("taskPrInfoChanged", { taskId, prUrl, prUrls: this.workspaceRepo.getPrUrls(taskId), - prState: row?.prState ?? null, + prState, }); } From 0ed32195463a57b16852db0fa57b0fa86b896df7 Mon Sep 17 00:00:00 2001 From: Frank Hamand Date: Wed, 8 Jul 2026 12:33:49 +0100 Subject: [PATCH 2/2] fix(git): keep cache in sync when the PR details fetch fails Guard getPrDetailsByUrl with a catch so a network/API failure after the prUrls reorder still runs updatePrCache and emit with prState null, instead of leaving prUrls[0] and the prUrl/prState columns disagreeing. Collapse the setPrimaryPrUrl tests into an it.each table and add the rejection case. Generated-By: PostHog Code Task-Id: 2772f1aa-cba4-46df-a8b6-3a14a0f15299 --- .../src/services/git/task-pr-status.test.ts | 54 +++++++++---------- .../src/services/git/task-pr-status.ts | 4 +- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/packages/workspace-server/src/services/git/task-pr-status.test.ts b/packages/workspace-server/src/services/git/task-pr-status.test.ts index 576c2e011c..ca8dd5e7ba 100644 --- a/packages/workspace-server/src/services/git/task-pr-status.test.ts +++ b/packages/workspace-server/src/services/git/task-pr-status.test.ts @@ -234,47 +234,43 @@ describe("TaskPrStatusService.setPrimaryPrUrl", () => { return { service, workspaceService, workspaceRepo }; } - it("recomputes and emits the promoted PR's live state, not the stale cache", async () => { - const getPrDetailsByUrl = vi - .fn() - .mockResolvedValue({ state: "open", merged: false, draft: false }); - const { service, workspaceService, workspaceRepo } = - makeService(getPrDetailsByUrl); + it.each([ + { + name: "recomputes and emits the promoted PR's live state, not the stale cache", + details: vi.fn().mockResolvedValue({ + state: "open", + merged: false, + draft: false, + }), + expectedPrState: "open", + }, + { + name: "emits a null state when the promoted PR's details are unavailable", + details: vi.fn().mockResolvedValue(null), + expectedPrState: null, + }, + { + name: "falls back to a null state when the details fetch rejects", + details: vi.fn().mockRejectedValue(new Error("network down")), + expectedPrState: null, + }, + ])("$name", async ({ details, expectedPrState }) => { + const { service, workspaceService, workspaceRepo } = makeService(details); await service.setPrimaryPrUrl("task-1", PR_NEW); expect(workspaceRepo.promotePrUrl).toHaveBeenCalledWith("task-1", PR_NEW); - expect(getPrDetailsByUrl).toHaveBeenCalledWith(PR_NEW); - expect(workspaceRepo.updatePrCache).toHaveBeenCalledWith("task-1", { - prUrl: PR_NEW, - prState: "open", - accumulate: false, - }); - expect(workspaceService.emit).toHaveBeenCalledWith("taskPrInfoChanged", { - taskId: "task-1", - prUrl: PR_NEW, - prUrls: [PR_NEW, PR_OLD], - prState: "open", - }); - }); - - it("emits a null state when the promoted PR's details are unavailable", async () => { - const getPrDetailsByUrl = vi.fn().mockResolvedValue(null); - const { service, workspaceService, workspaceRepo } = - makeService(getPrDetailsByUrl); - - await service.setPrimaryPrUrl("task-1", PR_NEW); - + expect(details).toHaveBeenCalledWith(PR_NEW); expect(workspaceRepo.updatePrCache).toHaveBeenCalledWith("task-1", { prUrl: PR_NEW, - prState: null, + prState: expectedPrState, accumulate: false, }); expect(workspaceService.emit).toHaveBeenCalledWith("taskPrInfoChanged", { taskId: "task-1", prUrl: PR_NEW, prUrls: [PR_NEW, PR_OLD], - prState: null, + prState: expectedPrState, }); }); }); diff --git a/packages/workspace-server/src/services/git/task-pr-status.ts b/packages/workspace-server/src/services/git/task-pr-status.ts index a69bd7331d..d499c58087 100644 --- a/packages/workspace-server/src/services/git/task-pr-status.ts +++ b/packages/workspace-server/src/services/git/task-pr-status.ts @@ -50,7 +50,9 @@ export class TaskPrStatusService { async setPrimaryPrUrl(taskId: string, prUrl: string): Promise { this.workspaceRepo.promotePrUrl(taskId, prUrl); - const details = await this.gitService.getPrDetailsByUrl(prUrl); + const details = await this.gitService + .getPrDetailsByUrl(prUrl) + .catch(() => null); const prState: SidebarPrState = details ? mapPrState(details.state, details.merged, details.draft) : null;