From 59e70e39835bff22df752032350754d13dd7d51b Mon Sep 17 00:00:00 2001 From: Serhii Vecherenko Date: Thu, 30 Jul 2026 01:29:55 -0700 Subject: [PATCH] feat(pr-watch): trigger checks immediately after pending PRs settle --- src/main/ipc/localHandlers.ts | 2 + src/main/prWatch/PrWatchService.test.ts | 53 ++++++++++++++++++++++ src/main/prWatch/PrWatchService.ts | 23 +++++++++- src/main/remote/RemoteAccessServer.test.ts | 10 ++++ src/main/remote/RemoteAccessServer.ts | 1 + src/main/remote/server/httpRouter.ts | 7 +++ src/mobile/bridge.test.tsx | 5 +- src/mobile/bridge.ts | 1 + src/renderer/state/gitRefresh.test.ts | 8 ++++ src/renderer/state/gitRefresh.ts | 38 +++++++++++++--- src/shared/ipc/procedureMap.ts | 1 + src/shared/ipc/procedures/prWatches.ts | 5 ++ src/shared/remote/client.test.ts | 10 +++- src/shared/remote/client.ts | 4 ++ 14 files changed, 157 insertions(+), 11 deletions(-) diff --git a/src/main/ipc/localHandlers.ts b/src/main/ipc/localHandlers.ts index dd0b6d255..96ad37c6b 100644 --- a/src/main/ipc/localHandlers.ts +++ b/src/main/ipc/localHandlers.ts @@ -468,6 +468,8 @@ export function createLocalIpcHandlers( runScheduleNow: ({ id }) => options.scheduleService.runNow(id), getScheduleRuns: ({ id }) => dbListScheduleRuns(id), getPrWatch: ({ projectId, prNumber }) => options.prWatchService.get(projectId, prNumber), + checkPrWatch: ({ projectId, prNumber }) => + options.prWatchService.requestCheck(projectId, prNumber), upsertPrWatch: (watch) => options.prWatchService.upsert(watch), deletePrWatch: ({ projectId, prNumber }) => options.prWatchService.delete(projectId, prNumber), checkForUpdate: () => options.autoUpdater.checkForUpdate(), diff --git a/src/main/prWatch/PrWatchService.test.ts b/src/main/prWatch/PrWatchService.test.ts index a85e19056..19003cb7f 100644 --- a/src/main/prWatch/PrWatchService.test.ts +++ b/src/main/prWatch/PrWatchService.test.ts @@ -406,6 +406,59 @@ describe("PrWatchService", () => { expect(store.get(project.id, pr.number)).not.toBeNull(); }); + it("checks immediately when pending checks settle", async () => { + let currentPr = { ...pr, checksStatus: "PENDING" }; + let currentDetails: PrDetails = { + ...details, + checks: [{ name: "Test", state: "IN_PROGRESS", conclusion: "" }], + }; + const { service, mergePr } = setup( + withoutAgent(watch({ watchEnabled: false, autoMerge: true })), + { + getPrForBranch: async () => currentPr, + getPrDetails: async () => currentDetails, + }, + ); + + await service.tick(); + expect(mergePr).not.toHaveBeenCalled(); + + currentPr = { ...pr, checksStatus: "SUCCESS" }; + currentDetails = { + ...details, + checks: [{ name: "Test", state: "COMPLETED", conclusion: "SUCCESS" }], + }; + service.requestCheck(project.id, pr.number); + + await vi.waitFor(() => expect(mergePr).toHaveBeenCalledOnce()); + }); + + it("queues a settled-status check that arrives during an in-flight check", async () => { + let resolveFirstPr!: (value: PrData) => void; + const getPrForBranch = vi + .fn() + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirstPr = resolve; + }), + ) + .mockResolvedValue(pr); + const { service, mergePr } = setup( + withoutAgent(watch({ watchEnabled: false, autoMerge: true })), + { getPrForBranch }, + ); + + const firstCheck = service.tick(); + await vi.waitFor(() => expect(getPrForBranch).toHaveBeenCalledOnce()); + service.requestCheck(project.id, pr.number); + resolveFirstPr({ ...pr, checksStatus: "PENDING" }); + await firstCheck; + + await vi.waitFor(() => expect(getPrForBranch).toHaveBeenCalledTimes(2)); + await vi.waitFor(() => expect(mergePr).toHaveBeenCalledOnce()); + }); + it("removes a watch after the PR closes", async () => { const { service, store } = setup(watch(), { getPrForBranch: async () => ({ ...pr, state: "closed" }), diff --git a/src/main/prWatch/PrWatchService.ts b/src/main/prWatch/PrWatchService.ts index 7a4dd7e1d..84141a639 100644 --- a/src/main/prWatch/PrWatchService.ts +++ b/src/main/prWatch/PrWatchService.ts @@ -51,6 +51,7 @@ export class PrWatchService { private timer: ReturnType | null = null; private disposed = false; private readonly checking = new Set(); + private readonly recheckRequested = new Set(); constructor(private readonly options: PrWatchServiceOptions) {} @@ -69,6 +70,7 @@ export class PrWatchService { this.disposed = true; if (this.timer) clearInterval(this.timer); this.timer = null; + this.recheckRequested.clear(); } get(projectId: string, prNumber: number): PrWatch | null { @@ -92,14 +94,27 @@ export class PrWatchService { lastError: null, }; this.options.store.upsert(watch); - void this.checkWatch(watch); + this.requestCheck(watch.projectId, watch.prNumber); return watch; } delete(projectId: string, prNumber: number): void { + this.recheckRequested.delete(watchKey({ projectId, prNumber })); this.options.store.delete(projectId, prNumber); } + requestCheck(projectId: string, prNumber: number): void { + if (this.disposed) return; + const watch = this.options.store.get(projectId, prNumber); + if (!watch) return; + const key = watchKey(watch); + if (this.checking.has(key)) { + this.recheckRequested.add(key); + return; + } + void this.checkWatch(watch); + } + async tick(): Promise { if (this.disposed) return; await Promise.allSettled(this.options.store.list().map((watch) => this.checkWatch(watch))); @@ -121,7 +136,7 @@ export class PrWatchService { : null, }; this.options.store.upsert(settled); - if (!settled.lastError) void this.checkWatch(settled); + if (!settled.lastError) this.requestCheck(settled.projectId, settled.prNumber); } } @@ -214,6 +229,10 @@ export class PrWatchService { this.saveError(snapshot, error); } finally { this.checking.delete(key); + if (this.recheckRequested.delete(key) && !this.disposed) { + const latest = this.options.store.get(snapshot.projectId, snapshot.prNumber); + if (latest) void this.checkWatch(latest); + } } } diff --git a/src/main/remote/RemoteAccessServer.test.ts b/src/main/remote/RemoteAccessServer.test.ts index adbfe6f35..4ec24d197 100644 --- a/src/main/remote/RemoteAccessServer.test.ts +++ b/src/main/remote/RemoteAccessServer.test.ts @@ -4356,6 +4356,7 @@ describe("RemoteAccessServer", () => { const remove = vi.fn<(projectId: string, prNumber: number) => void>(() => { stored = null; }); + const requestCheck = vi.fn<(projectId: string, prNumber: number) => void>(); const server = new RemoteAccessServer({ appVersion: "1.0.0", identity: { desktopId: "desktop-test", label: "Test Desktop" }, @@ -4364,6 +4365,7 @@ describe("RemoteAccessServer", () => { callSupervisor: vi.fn(async () => "" as never), prWatches: { get: () => stored, + requestCheck, upsert, delete: remove, }, @@ -4422,6 +4424,14 @@ describe("RemoteAccessServer", () => { }); expect(upsert).toHaveBeenCalledWith(input); + const checkResponse = await fetch(new URL("/api/pr-watches/check", info.httpBaseUrl), { + method: "POST", + headers: operateHeaders, + body: JSON.stringify({ projectId: "project-1", prNumber: 42 }), + }); + expect(checkResponse.status).toBe(200); + expect(requestCheck).toHaveBeenCalledWith("project-1", 42); + const deleteResponse = await fetch(new URL("/api/pr-watches", info.httpBaseUrl), { method: "DELETE", headers: operateHeaders, diff --git a/src/main/remote/RemoteAccessServer.ts b/src/main/remote/RemoteAccessServer.ts index b585b2a4f..e1cbd237b 100644 --- a/src/main/remote/RemoteAccessServer.ts +++ b/src/main/remote/RemoteAccessServer.ts @@ -184,6 +184,7 @@ export interface RemoteAccessServerOptions { /** Persistent PR automation owned by the host process. */ readonly prWatches?: { get(projectId: string, prNumber: number): PrWatch | null; + requestCheck(projectId: string, prNumber: number): void; upsert(input: PrWatchInput): PrWatch; delete(projectId: string, prNumber: number): void; }; diff --git a/src/main/remote/server/httpRouter.ts b/src/main/remote/server/httpRouter.ts index aea471da8..0c0433ec0 100644 --- a/src/main/remote/server/httpRouter.ts +++ b/src/main/remote/server/httpRouter.ts @@ -580,6 +580,13 @@ export async function handleHttp( }); return; } + if (req.method === "POST" && url.pathname === "/api/pr-watches/check") { + ctx.security.requireBearer(req, ["session:operate"]); + const key = prWatchKeySchema.parse(await readJsonBody(req)); + ctx.requirePrWatchesGateway().requestCheck(key.projectId, key.prNumber); + writeJson(res, 200, { ok: true }); + return; + } if (req.method === "POST" && url.pathname === "/api/pr-watches") { ctx.security.requireBearer(req, ["session:operate"]); const input = prWatchInputSchema.parse(await readJsonBody(req)); diff --git a/src/mobile/bridge.test.tsx b/src/mobile/bridge.test.tsx index 45d0ad8e4..983a98c25 100644 --- a/src/mobile/bridge.test.tsx +++ b/src/mobile/bridge.test.tsx @@ -93,10 +93,11 @@ describe("remote bridge", () => { lastError: null, }; const getPrWatch = vi.fn<(input: PrWatchKey) => Promise>(async () => watch); + const checkPrWatch = vi.fn<(input: PrWatchKey) => Promise>(async () => undefined); const upsertPrWatch = vi.fn<(input: PrWatchInput) => Promise>(async () => watch); const deletePrWatch = vi.fn<(input: PrWatchKey) => Promise>(async () => undefined); setRemoteBridgeClient( - { getPrWatch, upsertPrWatch, deletePrWatch } as unknown as RemoteDesktopClient, + { getPrWatch, checkPrWatch, upsertPrWatch, deletePrWatch } as unknown as RemoteDesktopClient, "darwin", ); installRemoteBridge(); @@ -111,9 +112,11 @@ describe("remote bridge", () => { config: { model: "gpt-5.6-sol" }, }; await expect(window.poracode.getPrWatch(key)).resolves.toEqual(watch); + await expect(window.poracode.checkPrWatch(key)).resolves.toBeUndefined(); await expect(window.poracode.upsertPrWatch(input)).resolves.toEqual(watch); await expect(window.poracode.deletePrWatch(key)).resolves.toBeUndefined(); expect(getPrWatch).toHaveBeenCalledWith(key); + expect(checkPrWatch).toHaveBeenCalledWith(key); expect(upsertPrWatch).toHaveBeenCalledWith(input); expect(deletePrWatch).toHaveBeenCalledWith(key); }); diff --git a/src/mobile/bridge.ts b/src/mobile/bridge.ts index a8f234ae6..8dc661d44 100644 --- a/src/mobile/bridge.ts +++ b/src/mobile/bridge.ts @@ -250,6 +250,7 @@ const remoteBridge = { deleteSchedule: ({ id }: { id: string }) => requireClient().deleteSchedule(id), runScheduleNow: ({ id }: { id: string }) => requireClient().runScheduleNow(id), getPrWatch: (input: PrWatchKey) => requireClient().getPrWatch(input), + checkPrWatch: (input: PrWatchKey) => requireClient().checkPrWatch(input), upsertPrWatch: (input: PrWatchInput) => requireClient().upsertPrWatch(input), deletePrWatch: (input: PrWatchKey) => requireClient().deletePrWatch(input), diff --git a/src/renderer/state/gitRefresh.test.ts b/src/renderer/state/gitRefresh.test.ts index 905cda00f..254856b13 100644 --- a/src/renderer/state/gitRefresh.test.ts +++ b/src/renderer/state/gitRefresh.test.ts @@ -36,6 +36,8 @@ const ghGetPrDetailsMock = prNumber: number; }) => Promise<{ details: PrDetails }> >(); +const checkPrWatchMock = + vi.fn<(payload: { projectId: string; prNumber: number }) => Promise>(); const location: ProjectLocation = { kind: "posix", path: "/repo" }; const wslLocation: ProjectLocation = { @@ -133,6 +135,8 @@ describe("pending PR refresh", () => { vi.useFakeTimers(); ghGetPrForBranchMock.mockReset(); ghGetPrDetailsMock.mockReset(); + checkPrWatchMock.mockReset(); + checkPrWatchMock.mockResolvedValue(undefined); Object.defineProperty(window, "poracode", { configurable: true, value: { @@ -142,6 +146,7 @@ describe("pending PR refresh", () => { .mockResolvedValue(undefined), ghGetPrForBranch: ghGetPrForBranchMock, ghGetPrDetails: ghGetPrDetailsMock, + checkPrWatch: checkPrWatchMock, }, }); useGitStore.setState({ @@ -274,6 +279,8 @@ describe("pending PR refresh", () => { expect(ghGetPrDetailsMock).toHaveBeenCalledWith({ projectLocation: location, prNumber: 42 }); expect(useGitStore.getState().prData[prKey]?.checksStatus).toBe("SUCCESS"); expect(useGitStore.getState().prDetails["p1#42"]?.checks[0]?.conclusion).toBe("SUCCESS"); + expect(checkPrWatchMock).toHaveBeenCalledOnce(); + expect(checkPrWatchMock).toHaveBeenCalledWith({ projectId: "p1", prNumber: 42 }); ghGetPrForBranchMock.mockClear(); ghGetPrDetailsMock.mockClear(); @@ -281,6 +288,7 @@ describe("pending PR refresh", () => { expect(ghGetPrForBranchMock).not.toHaveBeenCalled(); expect(ghGetPrDetailsMock).not.toHaveBeenCalled(); + expect(checkPrWatchMock).toHaveBeenCalledOnce(); }); it("polls when the PR summary is stale failed but loaded check details are pending", async () => { diff --git a/src/renderer/state/gitRefresh.ts b/src/renderer/state/gitRefresh.ts index 2b107be3e..6811e9449 100644 --- a/src/renderer/state/gitRefresh.ts +++ b/src/renderer/state/gitRefresh.ts @@ -184,11 +184,12 @@ function applyWorktreeStatusBatch( type ActiveGitProject = { id: string; location: ProjectLocation }; interface PendingPrRefreshTarget { + projectId: string; projectLocation: ProjectLocation; prKey: string; branch: string; - detailsCacheKey?: string; - prNumber?: number; + detailsCacheKey: string; + prNumber: number; } interface PendingPrRefreshEntry { @@ -376,17 +377,18 @@ function buildPendingPrRefreshTargets( function visitBranchPr(project: ActiveGitProject, prKey: string, branch: string) { const pr = gitState.prData[prKey]; if (!pr) return; - const detailsCacheKey = pr.number ? `${project.id}#${pr.number}` : undefined; - const details = detailsCacheKey ? gitState.prDetails[detailsCacheKey] : undefined; + const detailsCacheKey = `${project.id}#${pr.number}`; + const details = gitState.prDetails[detailsCacheKey]; const detailsStatus = aggregatePrChecksStatus(details?.checks); const checksStatus = combineChecksStatus(detailsStatus, pr.checksStatus); if (pr.state !== "open" || checksStatus !== "PENDING") return; - targets.set(detailsCacheKey ?? prKey, { + targets.set(detailsCacheKey, { + projectId: project.id, projectLocation: project.location, prKey, branch, - ...(detailsCacheKey ? { detailsCacheKey } : {}), - ...(pr.number ? { prNumber: pr.number } : {}), + detailsCacheKey, + prNumber: pr.number, }); } @@ -408,6 +410,22 @@ function buildPendingPrRefreshTargets( return targets; } +function didPendingPrSettle(target: PendingPrRefreshTarget): boolean { + const gitState = useGitStore.getState(); + const pr = gitState.prData[target.prKey]; + if (pr === null) return true; + if (!pr || pr.number !== target.prNumber) return false; + const detailsStatus = aggregatePrChecksStatus(gitState.prDetails[target.detailsCacheKey]?.checks); + const checksStatus = combineChecksStatus(detailsStatus, pr.checksStatus); + return checksStatus === "SUCCESS" || checksStatus === "FAILURE"; +} + +function requestSettledPrCheck(target: PendingPrRefreshTarget): void { + void readBridge() + .checkPrWatch({ projectId: target.projectId, prNumber: target.prNumber }) + .catch(() => undefined); +} + /** * Fetch a single PR's data (and its details, when a number + cache key are * known) and write both into the git store. Shared by the background @@ -552,6 +570,12 @@ export function syncPendingPrRefreshProjects(activeProjects: readonly ActiveGitP if (!target) { clearInterval(entry.intervalId); pendingPrRefreshEntries.delete(key); + if ( + activeProjects.some((project) => project.id === entry.target.projectId) && + didPendingPrSettle(entry.target) + ) { + requestSettledPrCheck(entry.target); + } continue; } entry.target = target; diff --git a/src/shared/ipc/procedureMap.ts b/src/shared/ipc/procedureMap.ts index 055b0051c..f2affdf43 100644 --- a/src/shared/ipc/procedureMap.ts +++ b/src/shared/ipc/procedureMap.ts @@ -185,6 +185,7 @@ export const MAIN_LOCAL_PROCEDURE_NAMES = [ "runScheduleNow", "getScheduleRuns", "getPrWatch", + "checkPrWatch", "upsertPrWatch", "deletePrWatch", ] as const satisfies readonly IpcProcedureName[]; diff --git a/src/shared/ipc/procedures/prWatches.ts b/src/shared/ipc/procedures/prWatches.ts index a58a12b22..d001b100c 100644 --- a/src/shared/ipc/procedures/prWatches.ts +++ b/src/shared/ipc/procedures/prWatches.ts @@ -13,6 +13,11 @@ export const prWatchProcedures = { "main-local", prWatchKeySchema, ), + checkPrWatch: definePayloadProcedure( + "checkPrWatch", + "main-local", + prWatchKeySchema, + ), upsertPrWatch: definePayloadProcedure( "upsertPrWatch", "main-local", diff --git a/src/shared/remote/client.test.ts b/src/shared/remote/client.test.ts index 6c57a0136..63d30ff90 100644 --- a/src/shared/remote/client.test.ts +++ b/src/shared/remote/client.test.ts @@ -138,7 +138,7 @@ describe("RemoteDesktopClient", () => { ]); }); - it("reads, enables, and deletes PR automation through the remote API", async () => { + it("reads, checks, enables, and deletes PR automation through the remote API", async () => { const requests: Array<{ url: string; method: string; body: unknown }> = []; const watch = { projectId: "project one", @@ -186,6 +186,9 @@ describe("RemoteDesktopClient", () => { await expect( client.getPrWatch({ projectId: watch.projectId, prNumber: watch.prNumber }), ).resolves.toEqual(watch); + await expect( + client.checkPrWatch({ projectId: watch.projectId, prNumber: watch.prNumber }), + ).resolves.toBeUndefined(); await expect(client.upsertPrWatch(input)).resolves.toEqual(watch); await expect( client.deletePrWatch({ projectId: watch.projectId, prNumber: watch.prNumber }), @@ -197,6 +200,11 @@ describe("RemoteDesktopClient", () => { method: "GET", body: undefined, }, + { + url: "https://relay.example.test/s/server-1/api/pr-watches/check", + method: "POST", + body: { projectId: watch.projectId, prNumber: watch.prNumber }, + }, { url: "https://relay.example.test/s/server-1/api/pr-watches", method: "POST", diff --git a/src/shared/remote/client.ts b/src/shared/remote/client.ts index deaa83f40..9ea693e9f 100644 --- a/src/shared/remote/client.ts +++ b/src/shared/remote/client.ts @@ -466,6 +466,10 @@ export class RemoteDesktopClient { return result.watch; } + async checkPrWatch(input: PrWatchKey): Promise { + await this.requestJson("/api/pr-watches/check", { method: "POST", body: input }); + } + async upsertPrWatch(input: PrWatchInput): Promise { const result = parseResponse( prWatchResponseSchema,