diff --git a/src/renderer/hooks/usePendingPrRefresh.test.tsx b/src/renderer/hooks/usePendingPrRefresh.test.tsx new file mode 100644 index 0000000..05d4480 --- /dev/null +++ b/src/renderer/hooks/usePendingPrRefresh.test.tsx @@ -0,0 +1,145 @@ +import { act, renderHook } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { PrData, PrDetails, ProjectLocation } from "@/shared/contracts"; +import { useGitStore } from "@/renderer/state/gitStore"; +import { PR_PENDING_REFRESH_INTERVAL_MS, usePendingPrRefresh } from "./usePendingPrRefresh"; + +const ghGetPrForBranchMock = + vi.fn< + (payload: { projectLocation: ProjectLocation; branch: string }) => Promise + >(); +const ghGetPrDetailsMock = + vi.fn< + (payload: { + projectLocation: ProjectLocation; + prNumber: number; + }) => Promise<{ details: PrDetails }> + >(); + +const location: ProjectLocation = { kind: "posix", path: "/repo" }; + +const basePr: PrData = { + number: 42, + state: "open", + title: "Improve PR checks", + url: "https://github.com/owner/repo/pull/42", + baseBranch: "main", + isDraft: false, + checksStatus: "PENDING", + updatedAt: "2026-04-04T00:00:00.000Z", +}; + +const baseDetails: PrDetails = { + number: 42, + title: "Improve PR checks", + body: "", + baseBranch: "main", + headBranch: "feature/pr-checks", + additions: 1, + deletions: 0, + changedFiles: 1, + mergedAt: null, + mergedBy: null, + closedAt: null, + commits: [], + comments: [], + reviews: [], + checks: [{ name: "CI", state: "PENDING", conclusion: "" }], +}; + +describe("usePendingPrRefresh", () => { + beforeEach(() => { + vi.useFakeTimers(); + ghGetPrForBranchMock.mockReset(); + ghGetPrDetailsMock.mockReset(); + Object.defineProperty(window, "lightcode", { + configurable: true, + value: { + platform: "darwin", + ghGetPrForBranch: ghGetPrForBranchMock, + ghGetPrDetails: ghGetPrDetailsMock, + }, + }); + useGitStore.setState({ + statuses: {}, + worktreeStatuses: {}, + worktrees: {}, + branches: {}, + ghAvailable: {}, + prData: {}, + worktreeSourceInfo: {}, + prDetails: {}, + prFiles: {}, + prDiffs: {}, + }); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("refetches pending PR status immediately, then every 30 seconds until it leaves pending", async () => { + useGitStore.getState().setPrData("pr-key", basePr); + useGitStore.getState().setPrDetails("p1#42", baseDetails); + ghGetPrForBranchMock + .mockResolvedValueOnce({ + ...basePr, + checksStatus: "PENDING", + updatedAt: "2026-04-04T00:00:30.000Z", + }) + .mockResolvedValueOnce({ + ...basePr, + checksStatus: "SUCCESS", + updatedAt: "2026-04-04T00:01:00.000Z", + }); + ghGetPrDetailsMock.mockResolvedValueOnce({ details: baseDetails }).mockResolvedValueOnce({ + details: { + ...baseDetails, + checks: [{ name: "CI", state: "COMPLETED", conclusion: "SUCCESS" }], + }, + }); + + renderHook(() => + usePendingPrRefresh({ + prKey: "pr-key", + projectLocation: location, + branch: "feature/pr-checks", + cacheKey: "p1#42", + }), + ); + + await act(async () => {}); + + expect(ghGetPrForBranchMock).toHaveBeenCalledTimes(1); + expect(ghGetPrForBranchMock).toHaveBeenLastCalledWith({ + projectLocation: location, + branch: "feature/pr-checks", + }); + expect(ghGetPrDetailsMock).toHaveBeenCalledTimes(1); + expect(useGitStore.getState().prData["pr-key"]?.checksStatus).toBe("PENDING"); + + ghGetPrForBranchMock.mockClear(); + ghGetPrDetailsMock.mockClear(); + + await act(async () => { + await vi.advanceTimersByTimeAsync(PR_PENDING_REFRESH_INTERVAL_MS); + }); + + expect(ghGetPrForBranchMock).toHaveBeenCalledWith({ + projectLocation: location, + branch: "feature/pr-checks", + }); + expect(ghGetPrDetailsMock).toHaveBeenCalledWith({ projectLocation: location, prNumber: 42 }); + expect(useGitStore.getState().prData["pr-key"]?.checksStatus).toBe("SUCCESS"); + expect(useGitStore.getState().prDetails["p1#42"]?.checks[0]?.conclusion).toBe("SUCCESS"); + + ghGetPrForBranchMock.mockClear(); + ghGetPrDetailsMock.mockClear(); + await act(async () => { + await vi.advanceTimersByTimeAsync(PR_PENDING_REFRESH_INTERVAL_MS); + }); + + expect(ghGetPrForBranchMock).not.toHaveBeenCalled(); + expect(ghGetPrDetailsMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/renderer/hooks/usePendingPrRefresh.ts b/src/renderer/hooks/usePendingPrRefresh.ts new file mode 100644 index 0000000..7713a00 --- /dev/null +++ b/src/renderer/hooks/usePendingPrRefresh.ts @@ -0,0 +1,68 @@ +import { useEffect } from "react"; +import type { ProjectLocation } from "@/shared/contracts"; +import { readBridge } from "@/renderer/bridge"; +import { useGitStore } from "@/renderer/state/gitStore"; +import { usePrChecksStatus, usePrNumber, usePrState } from "@/renderer/state/gitSelectors"; +import { getPrStatusTone } from "@/renderer/utils/prStatus"; + +export const PR_PENDING_REFRESH_INTERVAL_MS = 30_000; + +export function usePendingPrRefresh(params: { + prKey: string | undefined; + projectLocation: ProjectLocation; + branch: string | undefined; + cacheKey?: string | undefined; +}) { + const { prKey, projectLocation, branch, cacheKey } = params; + const state = usePrState(prKey); + const number = usePrNumber(prKey); + const checksStatus = usePrChecksStatus(prKey); + + useEffect(() => { + if (!prKey || getPrStatusTone(state, checksStatus) !== "warning") return; + + const targetPrKey = prKey; + const targetBranch = branch; + const detailsCacheKey = cacheKey; + const detailsPrNumber = number; + let cancelled = false; + let inFlight = false; + + async function refreshPendingPr() { + if (inFlight) return; + inFlight = true; + try { + const bridge = readBridge(); + const prPromise = targetBranch + ? bridge + .ghGetPrForBranch({ projectLocation, branch: targetBranch }) + .catch(() => undefined) + : Promise.resolve(undefined); + const detailsPromise = + detailsCacheKey && detailsPrNumber + ? bridge + .ghGetPrDetails({ projectLocation, prNumber: detailsPrNumber }) + .catch(() => undefined) + : Promise.resolve(undefined); + const [pr, details] = await Promise.all([prPromise, detailsPromise]); + if (cancelled) return; + if (pr !== undefined) useGitStore.getState().setPrData(targetPrKey, pr); + if (detailsCacheKey && details) { + useGitStore.getState().setPrDetails(detailsCacheKey, details.details); + } + } finally { + inFlight = false; + } + } + + void refreshPendingPr(); + const intervalId = window.setInterval( + () => void refreshPendingPr(), + PR_PENDING_REFRESH_INTERVAL_MS, + ); + return () => { + cancelled = true; + window.clearInterval(intervalId); + }; + }, [branch, cacheKey, checksStatus, number, prKey, projectLocation, state]); +} diff --git a/src/renderer/views/GitReviewOverlay/parts/GitReviewSidebar/GitReviewSidebar.tsx b/src/renderer/views/GitReviewOverlay/parts/GitReviewSidebar/GitReviewSidebar.tsx index 511eb91..883f422 100644 --- a/src/renderer/views/GitReviewOverlay/parts/GitReviewSidebar/GitReviewSidebar.tsx +++ b/src/renderer/views/GitReviewOverlay/parts/GitReviewSidebar/GitReviewSidebar.tsx @@ -350,6 +350,8 @@ export function GitReviewSidebar(props: { = { BLOCKED: "Required reviews, conversations, or status checks not met.", @@ -45,7 +47,10 @@ const BLOCK_REASON: Record = { export function PrSection(props: { prKey: string; projectId: string; + projectLocation: ProjectLocation; + branch?: string | undefined; worktreePath?: string | undefined; + cacheKey?: string | undefined; prLoading: boolean; handleMergePr: (method: "merge" | "squash" | "rebase", admin?: boolean) => Promise; handleClosePr: () => Promise; @@ -55,7 +60,10 @@ export function PrSection(props: { const { prKey, projectId, + projectLocation, + branch, worktreePath, + cacheKey, prLoading, handleMergePr, handleClosePr, @@ -71,6 +79,8 @@ export function PrSection(props: { const mergeable = usePrMergeable(prKey); const [bypass, setBypass] = useState(false); + usePendingPrRefresh({ prKey, projectLocation, branch, ...(cacheKey ? { cacheKey } : {}) }); + const indicatorColor = PR_TONE_BG_CLASS[getPrStatusTone(state, checksStatus)]; const reasonKey = mergeable === "CONFLICTING" ? "DIRTY" : mergeStateStatus; diff --git a/src/renderer/views/PrReviewOverlay/PrReviewOverlay.tsx b/src/renderer/views/PrReviewOverlay/PrReviewOverlay.tsx index 0eada82..bb06efb 100644 --- a/src/renderer/views/PrReviewOverlay/PrReviewOverlay.tsx +++ b/src/renderer/views/PrReviewOverlay/PrReviewOverlay.tsx @@ -195,6 +195,8 @@ export function PrReviewOverlay(props: { projectId={project.id} projectLocation={effectiveLocation} prKey={prKey} + cacheKey={cacheKey} + branch={details?.headBranch} worktreePath={worktreePath} onSelectFile={(path) => { setActiveTab("changes"); diff --git a/src/renderer/views/PrReviewOverlay/parts/PrReviewSidebar.tsx b/src/renderer/views/PrReviewOverlay/parts/PrReviewSidebar.tsx index c34b472..56a85eb 100644 --- a/src/renderer/views/PrReviewOverlay/parts/PrReviewSidebar.tsx +++ b/src/renderer/views/PrReviewOverlay/parts/PrReviewSidebar.tsx @@ -29,6 +29,8 @@ export function PrReviewSidebar(props: { projectId: string; projectLocation: ProjectLocation; prKey: string; + cacheKey: string; + branch?: string | undefined; worktreePath?: string | undefined; onSelectFile: (path: string) => void; onClose: () => void; @@ -41,6 +43,8 @@ export function PrReviewSidebar(props: { projectId, projectLocation, prKey, + cacheKey, + branch, worktreePath, onSelectFile, onClose, @@ -146,7 +150,10 @@ export function PrReviewSidebar(props: { vi.fn<(...args: unknown[]) => Promise<{ stdout: string }>>(), @@ -60,6 +60,10 @@ describe("GitHubService", () => { writeFileMock.mockResolvedValue(undefined); }); + afterEach(() => { + vi.useRealTimers(); + }); + describe("checkGhAvailable", () => { it("returns available true when gh --version succeeds", async () => { execFileAsyncMock.mockResolvedValue({ stdout: "gh version 2.50.0\n" }); @@ -195,11 +199,13 @@ describe("GitHubService", () => { title: "My PR", baseRefName: "main", isDraft: false, + statusCheckRollup: [{ status: "COMPLETED", conclusion: "SUCCESS" }], updatedAt: "2026-04-03T10:00:00Z", }); - // First call: pr create (returns URL text), second call: pr view (returns JSON) + // First call: pr create, second call: viewer login, third call: pr view. execFileAsyncMock .mockResolvedValueOnce({ stdout: "https://github.com/owner/repo/pull/50\n" }) + .mockResolvedValueOnce({ stdout: "demo\n" }) .mockResolvedValueOnce({ stdout: viewJson }); const result = await new GitHubService().createPr( @@ -213,6 +219,7 @@ describe("GitHubService", () => { expect(result.number).toBe(50); expect(result.state).toBe("open"); + expect(result.checksStatus).toBe("SUCCESS"); expect(mkdtempMock).toHaveBeenCalledTimes(1); expect(writeFileMock).toHaveBeenCalledTimes(1); expect(rmMock).toHaveBeenCalledTimes(1); @@ -222,8 +229,10 @@ describe("GitHubService", () => { expect(createArgs).toContain("create"); expect(createArgs).toContain("--body-file"); expect(createArgs).not.toContain("--json"); - // Second call: pr view - const viewArgs = buildAgentCommandMock.mock.calls[1]![2] as string[]; + const viewCall = buildAgentCommandMock.mock.calls.find((call) => + (call[2] as string[]).includes("view"), + ); + const viewArgs = viewCall![2] as string[]; expect(viewArgs).toContain("pr"); expect(viewArgs).toContain("view"); expect(viewArgs).toContain("--json"); @@ -232,6 +241,7 @@ describe("GitHubService", () => { it("includes --draft flag when isDraft is true", async () => { execFileAsyncMock .mockResolvedValueOnce({ stdout: "https://github.com/owner/repo/pull/51\n" }) + .mockResolvedValueOnce({ stdout: "demo\n" }) .mockResolvedValueOnce({ stdout: JSON.stringify({ number: 51, @@ -250,6 +260,54 @@ describe("GitHubService", () => { expect(createArgs).toContain("--draft"); }); + it("polls briefly when the created PR has no checks yet", async () => { + vi.useFakeTimers(); + execFileAsyncMock + .mockResolvedValueOnce({ stdout: "https://github.com/owner/repo/pull/52\n" }) + .mockResolvedValueOnce({ stdout: "demo\n" }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 52, + url: "https://github.com/owner/repo/pull/52", + state: "OPEN", + title: "Queued PR", + baseRefName: "main", + isDraft: false, + updatedAt: "2026-04-03T10:00:00Z", + statusCheckRollup: [], + }), + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 52, + url: "https://github.com/owner/repo/pull/52", + state: "OPEN", + title: "Queued PR", + baseRefName: "main", + isDraft: false, + updatedAt: "2026-04-03T10:00:05Z", + statusCheckRollup: [{ status: "QUEUED", conclusion: "" }], + }), + }); + + const resultPromise = new GitHubService().createPr( + location, + "feature/x", + "main", + "Queued PR", + "", + false, + ); + await vi.advanceTimersByTimeAsync(1_000); + const result = await resultPromise; + + expect(result.checksStatus).toBe("PENDING"); + const viewCalls = buildAgentCommandMock.mock.calls.filter((call) => + (call[2] as string[]).includes("view"), + ); + expect(viewCalls).toHaveLength(2); + }); + it("cleans up body file even on failure", async () => { execFileAsyncMock.mockRejectedValue(new Error("gh pr create failed: some error")); diff --git a/src/supervisor/github.ts b/src/supervisor/github.ts index ffcf9be..4446a16 100644 --- a/src/supervisor/github.ts +++ b/src/supervisor/github.ts @@ -28,6 +28,14 @@ import { buildAgentCommand, parallelWslCommandsAsync, quotePosixShellArg } from const execFileAsync = promisify(execFile); const GH_TIMEOUT = 30_000; +const CREATE_PR_STATUS_WAIT_MS = 8_000; +const CREATE_PR_STATUS_POLL_MS = 1_000; +const PR_VIEW_FIELDS = + "number,url,state,title,baseRefName,isDraft,reviewDecision,statusCheckRollup,updatedAt,mergeable,mergeStateStatus,author"; + +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} // `gh` reports an unreachable remote with the GraphQL message // "Could not resolve to a Repository with the name '/'". This @@ -349,6 +357,37 @@ export class GitHubService { } } + private async viewPrData( + location: ProjectLocation, + branch: string, + viewerLogin?: string, + ): Promise { + const stdout = await runGh(location, ["pr", "view", branch, "--json", PR_VIEW_FIELDS]); + return mapPrData(JSON.parse(stdout), viewerLogin); + } + + private async waitForCreatedPrStatus( + location: ProjectLocation, + branch: string, + viewerLogin?: string, + ): Promise { + const deadline = Date.now() + CREATE_PR_STATUS_WAIT_MS; + let latest = await this.viewPrData(location, branch, viewerLogin); + while ( + !latest.isDraft && + latest.state === "open" && + !latest.checksStatus && + Date.now() < deadline + ) { + await delay(CREATE_PR_STATUS_POLL_MS); + latest = await this.viewPrData(location, branch, viewerLogin); + } + if (!latest.isDraft && latest.state === "open" && !latest.checksStatus) { + return { ...latest, checksStatus: "PENDING" }; + } + return latest; + } + async createPr( location: ProjectLocation, branch: string, @@ -377,18 +416,11 @@ export class GitHubService { ]; await runGh(location, createArgs); - // gh pr create doesn't support --json; fetch the new PR via gh pr view - const [viewStdout, viewerLogin] = await Promise.all([ - runGh(location, [ - "pr", - "view", - branch, - "--json", - "number,url,state,title,baseRefName,isDraft,reviewDecision,statusCheckRollup,updatedAt,mergeable,mergeStateStatus,author", - ]), - this.getViewerLogin(location), - ]); - return mapPrData(JSON.parse(viewStdout), viewerLogin); + // `gh pr create` doesn't support --json. GitHub can return an empty + // statusCheckRollup for a few seconds before Actions queues checks, so + // wait briefly before handing the new PR to the renderer. + const viewerLogin = await this.getViewerLogin(location); + return await this.waitForCreatedPrStatus(location, branch, viewerLogin); } catch (err) { throw classifyError(err, "pr create"); } finally { @@ -407,7 +439,7 @@ export class GitHubService { "--limit", "1", "--json", - "number,url,state,title,baseRefName,isDraft,reviewDecision,statusCheckRollup,updatedAt,mergeable,mergeStateStatus,author", + PR_VIEW_FIELDS, ]; // WSL: collapse `gh pr list` + `gh api user` (for viewerDidAuthor) into a