Skip to content

Commit 670e4be

Browse files
committed
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
1 parent 6867e5d commit 670e4be

3 files changed

Lines changed: 54 additions & 10 deletions

File tree

packages/host-router/src/ports/git-pr-status.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@ export interface IGitPrStatus {
1313
cloudPrUrl: string | null,
1414
): Promise<TaskPrStatus>;
1515
getCachedPrUrl(taskId: string): CachedPrUrlOutput;
16-
setPrimaryPrUrl(taskId: string, prUrl: string): void;
16+
setPrimaryPrUrl(taskId: string, prUrl: string): Promise<void>;
1717
}

packages/workspace-server/src/services/git/task-pr-status.test.ts

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -215,30 +215,66 @@ describe("TaskPrStatusService revalidation PR detection", () => {
215215
});
216216

217217
describe("TaskPrStatusService.setPrimaryPrUrl", () => {
218-
it("emits the promoted url as prUrl even though the row column is stale", () => {
219-
const PR_OLD = "https://github.com/acme/repo/pull/1";
220-
const PR_NEW = "https://github.com/acme/repo/pull/2";
221-
const gitService = {} as unknown as GitService;
218+
const PR_OLD = "https://github.com/acme/repo/pull/1";
219+
const PR_NEW = "https://github.com/acme/repo/pull/2";
220+
221+
function makeService(getPrDetailsByUrl: ReturnType<typeof vi.fn>) {
222+
const gitService = { getPrDetailsByUrl } as unknown as GitService;
222223
const workspaceService = { emit: vi.fn() };
223224
const workspaceRepo = {
224225
promotePrUrl: vi.fn(),
225-
findByTaskId: vi.fn().mockReturnValue({ prUrl: PR_OLD, prState: "open" }),
226+
updatePrCache: vi.fn(),
226227
getPrUrls: vi.fn().mockReturnValue([PR_NEW, PR_OLD]),
227228
};
228229
const service = new TaskPrStatusService(
229230
gitService,
230231
workspaceRepo as unknown as IWorkspaceRepository,
231232
workspaceService as unknown as WorkspaceService,
232233
);
234+
return { service, workspaceService, workspaceRepo };
235+
}
236+
237+
it("recomputes and emits the promoted PR's live state, not the stale cache", async () => {
238+
const getPrDetailsByUrl = vi
239+
.fn()
240+
.mockResolvedValue({ state: "open", merged: false, draft: false });
241+
const { service, workspaceService, workspaceRepo } =
242+
makeService(getPrDetailsByUrl);
233243

234-
service.setPrimaryPrUrl("task-1", PR_NEW);
244+
await service.setPrimaryPrUrl("task-1", PR_NEW);
235245

236246
expect(workspaceRepo.promotePrUrl).toHaveBeenCalledWith("task-1", PR_NEW);
247+
expect(getPrDetailsByUrl).toHaveBeenCalledWith(PR_NEW);
248+
expect(workspaceRepo.updatePrCache).toHaveBeenCalledWith("task-1", {
249+
prUrl: PR_NEW,
250+
prState: "open",
251+
accumulate: false,
252+
});
237253
expect(workspaceService.emit).toHaveBeenCalledWith("taskPrInfoChanged", {
238254
taskId: "task-1",
239255
prUrl: PR_NEW,
240256
prUrls: [PR_NEW, PR_OLD],
241257
prState: "open",
242258
});
243259
});
260+
261+
it("emits a null state when the promoted PR's details are unavailable", async () => {
262+
const getPrDetailsByUrl = vi.fn().mockResolvedValue(null);
263+
const { service, workspaceService, workspaceRepo } =
264+
makeService(getPrDetailsByUrl);
265+
266+
await service.setPrimaryPrUrl("task-1", PR_NEW);
267+
268+
expect(workspaceRepo.updatePrCache).toHaveBeenCalledWith("task-1", {
269+
prUrl: PR_NEW,
270+
prState: null,
271+
accumulate: false,
272+
});
273+
expect(workspaceService.emit).toHaveBeenCalledWith("taskPrInfoChanged", {
274+
taskId: "task-1",
275+
prUrl: PR_NEW,
276+
prUrls: [PR_NEW, PR_OLD],
277+
prState: null,
278+
});
279+
});
244280
});

packages/workspace-server/src/services/git/task-pr-status.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,22 @@ export class TaskPrStatusService {
4848
};
4949
}
5050

51-
setPrimaryPrUrl(taskId: string, prUrl: string): void {
51+
async setPrimaryPrUrl(taskId: string, prUrl: string): Promise<void> {
5252
this.workspaceRepo.promotePrUrl(taskId, prUrl);
53-
const row = this.workspaceRepo.findByTaskId(taskId);
53+
const details = await this.gitService.getPrDetailsByUrl(prUrl);
54+
const prState: SidebarPrState = details
55+
? mapPrState(details.state, details.merged, details.draft)
56+
: null;
57+
this.workspaceRepo.updatePrCache(taskId, {
58+
prUrl,
59+
prState,
60+
accumulate: false,
61+
});
5462
this.workspaceService.emit("taskPrInfoChanged", {
5563
taskId,
5664
prUrl,
5765
prUrls: this.workspaceRepo.getPrUrls(taskId),
58-
prState: row?.prState ?? null,
66+
prState,
5967
});
6068
}
6169

0 commit comments

Comments
 (0)