diff --git a/.agents/skills/ade-perf-prs/SKILL.md b/.agents/skills/ade-perf-prs/SKILL.md new file mode 100644 index 000000000..ab82edd4b --- /dev/null +++ b/.agents/skills/ade-perf-prs/SKILL.md @@ -0,0 +1,150 @@ +--- +name: ade-perf-prs +description: Performance practices for ADE's PRs tab. Read before editing + files under apps/desktop/src/renderer/components/prs/**, + apps/desktop/src/main/services/prs/**, PR IPC/preload contracts, or + PR-facing ADE actions. Preserve these patterns unless a new measured PRs UI + audit proves a better one. +metadata: + author: ade-autoresearch + version: 0.1.0 + status: active +--- + +# ade-perf-prs + +Use this as engineering guidance for keeping the PRs tab fast while adding +features. The PRs tab combines external GitHub search, local lane links, +mergeability, queue/integration workflows, Path to Merge state, review threads, +files, CI, and activity. Keep first paint local and defer expensive live GitHub +or Git operations until the visible surface needs them. + +## Measurement posture + +- Test the real Electron `/prs` route against a private `perf-pass` GitHub repo + with enough PRs to cover single, queue, integration, and rebase/merge flows. +- Drive visible UI actions with Computer Use and mark important spans with + `window.ade.perf.recordEvent({ kind: "manualStep", ... })`. +- Do not measure only one PR forever. Seed several lanes and PRs per workflow + type, then optimize the batched path users actually hit. +- Separate stale-cache/no-op refreshes from true GitHub refreshes. If the UI + returns instantly because nothing is stale, also measure the explicit preload + path for the PR set changed by the optimization. + +## Startup and GitHub tab rules + +- Opening the GitHub tab must not run conflict analysis, rebase scans, or + per-PR merge-context calls. First paint should use local PR rows and cached + GitHub snapshot data. +- Default GitHub snapshot search should fetch open external PRs only. Closed, + merged, and all-history views may opt into external closed PR history when + the user asks for that surface. +- Hydrate selected PR detail panes from `pull_request_snapshots` first, then run + live GitHub calls in the background. Detail panes should not render blank while + cached detail/files/checks/reviews/comments/commits exist. +- Keep snapshot hydration batched. Prefer `listSnapshots({ prId })` for detail + hydration and avoid separate status/checks/reviews/comments/files calls before + the cached view is visible. + +## Workflow and merge-context rules + +- Use bulk merge-context APIs for workflow surfaces. `getMergeContexts(prIds)` + should replace N calls to `getMergeContext(prId)` whenever a queue, + integration, or rebase/merge view renders multiple PRs. +- Merge-context and conflict-analysis reads should use lane metadata only: + `laneService.list({ includeArchived: false, includeStatus: false })` unless + the UI is explicitly displaying fresh Git status. +- PR workflow context should also keep lane reads status-light. Use + `window.ade.lanes.list({ includeStatus: false })` for workflow rendering and + fetch fresh lane Git status only inside flows that actually inspect dirty, + ahead, behind, or rebase-in-progress state. +- The normal GitHub list should call `listWithConflicts({ includeConflictAnalysis: false })`. + Queue, integration, and rebase/merge workflows may request conflict analysis + because their UI depends on it. +- `listWithConflicts` must batch conflict assessment with lane inputs instead of + asking the conflict service one PR at a time. + +## Refresh rules + +- Explicit PR refreshes should be bounded and parallel, not serialized one PR at + a time. Keep a conservative concurrency limit so GitHub is used efficiently + without flooding the API. +- Background refresh should stay small and stale-aware. The no-argument refresh + path is for hot or stale candidates, not a reason to sync every PR on every + tab open. +- Do not reintroduce "syncing to GitHub" as a blocking first-open state. The tab + should remain usable while refreshes run. +- Rebase diagnostics are useful workflow data, but they are not a reason to run + queue-target `git fetch` on every poll. Keep queue target tracking refreshes + best-effort and TTL-bound. + +## Proven PRs patterns + +### Keep GitHub first open-only and local-first + +- **Why it helped**: The original PRs open path spent seconds fetching external + GitHub history and doing local workflow work before the list felt usable. +- **Apply when**: Editing `GitHubTab`, GitHub snapshot fetching, or first-load + PR state. +- **Avoid**: Loading closed/merged external PRs or conflict analysis before the + user opens those surfaces. +- **Verification**: `prs-ui-baseline-20260512-051124` had + `ade.prs.getGitHubSnapshot` at `5941ms`. After the open-only snapshot and + local-first hydration, `prs-ui-lane-metadata-fast-inproc-20260512-060555` + showed first-load `getGitHubSnapshot` at `1146ms`, `listWithConflicts` at + `1ms`, and `getMergeContexts` at `52ms`. + +### Batch merge contexts and keep lane reads metadata-only + +- **Why it helped**: Workflow pages previously fanned out merge-context calls + and each one could pay for lane status work. +- **Apply when**: Queue, integration, rebase/merge, or Path to Merge surfaces + need per-PR merge context. +- **Avoid**: Looping over `getMergeContext` or using bare `laneService.list()` + from merge-context helpers. +- **Verification**: In `prs-ui-lane-metadata-fast-inproc-20260512-060555`, + workflow `getMergeContexts` calls measured `28-72ms`; the prior workflow pass + had repeated merge-context batches around `1.2-1.4s`. + +### Bound explicit refresh with parallel workers + +- **Why it helped**: Refreshing PRs one at a time made workflow refresh feel + stuck even when the UI was otherwise local-first. +- **Apply when**: Changing `prService.refresh`, refresh buttons, or explicit + refresh actions from automations. +- **Avoid**: Serial `for await` refresh of PR detail/status/check/files for + multiple PRs. +- **Verification**: Before parallel refresh, the measured workflow refresh span + had `ade.prs.refresh` at `12284ms`. After bounded parallel refresh, an + explicit all-18 preload/IPC refresh in + `prs-ui-lane-metadata-fast-inproc-20260512-060555` completed in `3800ms`. + +### Keep workflow lane reads status-light + +- **Why it helped**: Queue workflow reloads still paid full lane Git status and + auto-rebase status cleanup even though the visible workflow cards only needed + lane identity, branch, color, queue state, rebase needs, and merge context. +- **Apply when**: Editing `PrsContext`, workflow tabs, or auto-rebase status + hydration for PRs. +- **Avoid**: `window.ade.lanes.list({ includeStatus: true })` on PR workflow + startup or background PR refreshes that do not display lane dirty/ahead/behind + state. +- **Verification**: In + `prs-ui-rebase-fetch-ttl-20260512-062130`, queue reload lane reads dropped + from `1393ms` to `47-80ms`, and `listAutoRebaseStatuses` dropped from + `1404-1407ms` to `40-70ms`. + +### Keep Path to Merge start/stop local and state-first + +- **Why it helped**: Path to Merge controls should react to local pipeline + settings, convergence state, and issue inventory without waiting on a full PR + refresh or workflow sweep. +- **Apply when**: Editing `PrConvergencePanel`, pipeline settings, issue + inventory sync, or Path to Merge IPC. +- **Avoid**: Coupling the start/stop buttons to fresh detail hydration, merge + context fan-out, or agent dispatch prework that can run after the local state + transition. +- **Verification**: In `prs-ui-ptm-audit-20260512-0635`, with PLAN mode and + auto-merge off, the UI covered native Path to Merge start/stop safely: + `ade.prs.pathToMerge.start` completed in `5ms` and + `ade.prs.pathToMerge.stop` completed in `2ms`. diff --git a/apps/desktop/src/main/services/adeActions/registry.ts b/apps/desktop/src/main/services/adeActions/registry.ts index 703cdb43d..17bb8e03c 100644 --- a/apps/desktop/src/main/services/adeActions/registry.ts +++ b/apps/desktop/src/main/services/adeActions/registry.ts @@ -352,6 +352,7 @@ export const ADE_ACTION_ALLOWLIST: Partial runtime.autoRebaseService?.listStatuses() ?? [], + listAutoRebaseStatuses: () => + runtime.autoRebaseService?.listStatuses() ?? [], dismissAutoRebaseStatus: async (args?: { laneId?: string }) => { const laneId = requireNonEmptyString(args?.laneId, "laneId"); await runtime.autoRebaseService?.dismissStatus({ laneId }); diff --git a/apps/desktop/src/main/services/git/gitOperationsService.test.ts b/apps/desktop/src/main/services/git/gitOperationsService.test.ts index 6baa0a08a..9bcd2f966 100644 --- a/apps/desktop/src/main/services/git/gitOperationsService.test.ts +++ b/apps/desktop/src/main/services/git/gitOperationsService.test.ts @@ -150,7 +150,7 @@ describe("gitOperationsService stash item commands", () => { ); }); - it("pops a stash when restoring it", async () => { + it("applies then drops a stash when restoring it", async () => { mockGit.getHeadSha.mockResolvedValue("abc123"); mockGit.runGitOrThrow.mockResolvedValue(undefined); const { service, mockStart, mockFinish } = createTestGitOperationsService(); @@ -158,10 +158,14 @@ describe("gitOperationsService stash item commands", () => { const result = await service.stashPop({ laneId: "lane-1", stashRef: "stash@{1}" }); expect(mockGit.runGitOrThrow).toHaveBeenCalledWith( - ["stash", "pop", "stash@{1}"], + ["stash", "apply", "stash@{1}"], { cwd: "/tmp/ade-lane", timeoutMs: 30_000 }, ); - expect(mockGit.runGitOrThrow).toHaveBeenCalledTimes(1); + expect(mockGit.runGitOrThrow).toHaveBeenCalledWith( + ["stash", "drop", "stash@{1}"], + { cwd: "/tmp/ade-lane", timeoutMs: 30_000 }, + ); + expect(mockGit.runGitOrThrow).toHaveBeenCalledTimes(2); expect(result).toEqual({ operationId: "op-1", preHeadSha: "abc123", @@ -182,18 +186,58 @@ describe("gitOperationsService stash item commands", () => { ); }); - it("surfaces stash pop failures so git keeps the stash for manual recovery", async () => { + it("keeps the stash when restore apply fails", async () => { mockGit.getHeadSha.mockResolvedValue("abc123"); - mockGit.runGitOrThrow.mockRejectedValueOnce(new Error("pop failed")); + mockGit.runGitOrThrow.mockRejectedValueOnce(new Error("apply failed")); const { service } = createTestGitOperationsService(); - await expect(service.stashPop({ laneId: "lane-1", stashRef: "stash@{1}" })).rejects.toThrow("pop failed"); + await expect(service.stashPop({ laneId: "lane-1", stashRef: "stash@{1}" })).rejects.toThrow("apply failed"); expect(mockGit.runGitOrThrow).toHaveBeenCalledTimes(1); expect(mockGit.runGitOrThrow).toHaveBeenCalledWith( - ["stash", "pop", "stash@{1}"], + ["stash", "apply", "stash@{1}"], + { cwd: "/tmp/ade-lane", timeoutMs: 30_000 }, + ); + }); + + it("reports restore success when drop fails after apply", async () => { + mockGit.getHeadSha.mockResolvedValue("abc123"); + mockGit.runGitOrThrow + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error("drop failed")); + const { service, mockFinish, mockLogger } = createTestGitOperationsService(); + + const result = await service.stashPop({ laneId: "lane-1", stashRef: "stash@{1}" }); + + expect(mockGit.runGitOrThrow).toHaveBeenNthCalledWith( + 1, + ["stash", "apply", "stash@{1}"], + { cwd: "/tmp/ade-lane", timeoutMs: 30_000 }, + ); + expect(mockGit.runGitOrThrow).toHaveBeenNthCalledWith( + 2, + ["stash", "drop", "stash@{1}"], { cwd: "/tmp/ade-lane", timeoutMs: 30_000 }, ); + expect(result).toEqual({ + operationId: "op-1", + preHeadSha: "abc123", + postHeadSha: "abc123", + }); + expect(mockFinish).toHaveBeenCalledWith( + expect.objectContaining({ + operationId: "op-1", + status: "succeeded", + }), + ); + expect(mockLogger.warn).toHaveBeenCalledWith( + "git.stash_pop_drop_failed", + expect.objectContaining({ + laneId: "lane-1", + stashRef: "stash@{1}", + error: "drop failed", + }), + ); }); it("calls git stash drop with the lane worktree path and stash ref", async () => { diff --git a/apps/desktop/src/main/services/git/gitOperationsService.ts b/apps/desktop/src/main/services/git/gitOperationsService.ts index 737f1929a..d54d94cc3 100644 --- a/apps/desktop/src/main/services/git/gitOperationsService.ts +++ b/apps/desktop/src/main/services/git/gitOperationsService.ts @@ -977,7 +977,17 @@ export function createGitOperationsService({ reason: "stash_pop", metadata: { stashRef }, fn: async (lane) => { - await runGitOrThrow(["stash", "pop", stashRef], { cwd: lane.worktreePath, timeoutMs: 30_000 }); + await runGitOrThrow(["stash", "apply", stashRef], { cwd: lane.worktreePath, timeoutMs: 30_000 }); + try { + await runGitOrThrow(["stash", "drop", stashRef], { cwd: lane.worktreePath, timeoutMs: 30_000 }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + logger.warn("git.stash_pop_drop_failed", { + laneId: args.laneId, + stashRef, + error: message, + }); + } } }); return action; diff --git a/apps/desktop/src/main/services/ipc/registerIpc.ts b/apps/desktop/src/main/services/ipc/registerIpc.ts index 16b09c58d..baff46c85 100644 --- a/apps/desktop/src/main/services/ipc/registerIpc.ts +++ b/apps/desktop/src/main/services/ipc/registerIpc.ts @@ -8238,10 +8238,25 @@ export function registerIpc({ ipcMain.handle(IPC.prsGetMergeContext, async (_event, arg: { prId: string }): Promise => getCtx().prService.getMergeContext(arg.prId)); - ipcMain.handle(IPC.prsListWithConflicts, async () => ensurePrPolling().prService.listWithConflicts()); + ipcMain.handle(IPC.prsGetMergeContexts, async (_event, arg: { prIds?: string[] }): Promise> => + getCtx().prService.getMergeContexts(Array.isArray(arg?.prIds) ? arg.prIds : []) + ); + + ipcMain.handle(IPC.prsListWithConflicts, async (_event, arg?: { includeConflictAnalysis?: boolean }) => + ensurePrPolling().prService.listWithConflicts({ + includeConflictAnalysis: arg?.includeConflictAnalysis === true, + }) + ); + + ipcMain.handle(IPC.prsListSnapshots, async (_event, arg?: { prId?: string }) => + getCtx().prService.listSnapshots({ prId: typeof arg?.prId === "string" ? arg.prId : undefined }) + ); - ipcMain.handle(IPC.prsGetGitHubSnapshot, async (_event, arg?: { force?: boolean }): Promise => - await ensurePrPolling().prService.getGithubSnapshot({ force: arg?.force === true }) + ipcMain.handle(IPC.prsGetGitHubSnapshot, async (_event, arg?: { force?: boolean; includeExternalClosed?: boolean }): Promise => + await ensurePrPolling().prService.getGithubSnapshot({ + force: arg?.force === true, + includeExternalClosed: arg?.includeExternalClosed === true, + }) ); ipcMain.handle(IPC.prsCreateQueue, async (_event, arg: CreateQueuePrsArgs): Promise => { diff --git a/apps/desktop/src/main/services/lanes/autoRebaseService.test.ts b/apps/desktop/src/main/services/lanes/autoRebaseService.test.ts index 2c71c9ed3..f3c972730 100644 --- a/apps/desktop/src/main/services/lanes/autoRebaseService.test.ts +++ b/apps/desktop/src/main/services/lanes/autoRebaseService.test.ts @@ -247,6 +247,32 @@ describe("autoRebaseService", () => { }); }); + describe("listStatuses — caller-supplied lanes", () => { + it("does not clear auto statuses from lanes whose git status may be a lightweight stub", async () => { + const service = createService(); + laneList = [makeLane("lane-a", { + parentLaneId: "root", + status: { dirty: false, ahead: 0, behind: 0, remoteBehind: 0, rebaseInProgress: false }, + })]; + db.setJson("auto_rebase:status:lane-a", { + laneId: "lane-a", + parentLaneId: "root", + parentHeadSha: "abc", + state: "rebasePending", + updatedAt: "2026-03-25T11:59:00.000Z", + conflictCount: 0, + message: null, + source: "auto", + }); + + const statuses = await service.listStatuses({ lanes: laneList }); + + expect(statuses).toHaveLength(1); + expect(db.getJson("auto_rebase:status:lane-a")).not.toBeNull(); + expect(laneService.list).not.toHaveBeenCalled(); + }); + }); + // --------------------------------------------------------------------------- // Lanes without parent remain eligible for status display // --------------------------------------------------------------------------- @@ -379,6 +405,32 @@ describe("autoRebaseService", () => { expect(statuses).toHaveLength(0); }); + it("clears stale non-autoRebased status during listings", async () => { + const service = createService(); + + laneList = [makeLane("lane-a", { + parentLaneId: "root", + status: { dirty: false, ahead: 1, behind: 0, remoteBehind: 0, rebaseInProgress: false }, + })]; + + db.setJson("auto_rebase:status:lane-a", { + laneId: "lane-a", + parentLaneId: "root", + parentHeadSha: "abc", + state: "rebasePending", + updatedAt: "2026-03-25T11:00:00.000Z", + conflictCount: 0, + message: null, + }); + + const statuses = await service.listStatuses(); + expect(statuses).toHaveLength(0); + expect(laneService.list).toHaveBeenCalledWith({ + includeArchived: false, + includeStatus: true, + }); + }); + it("keeps non-autoRebased status when lane is behind its parent", async () => { const service = createService(); @@ -404,6 +456,32 @@ describe("autoRebaseService", () => { expect(statuses[0].laneId).toBe("lane-a"); expect(statuses[0].state).toBe("rebasePending"); }); + + it("keeps ancestor-blocked status even when the lane itself is not behind", async () => { + const service = createService(); + + const root = makeLane("root"); + const child = makeLane("lane-a", { + parentLaneId: "root", + status: { dirty: false, ahead: 0, behind: 0, remoteBehind: 0, rebaseInProgress: false }, + }); + laneList = [root, child]; + + db.setJson("auto_rebase:status:lane-a", { + laneId: "lane-a", + parentLaneId: "root", + parentHeadSha: null, + state: "rebasePending", + updatedAt: "2026-03-25T11:00:00.000Z", + conflictCount: 0, + message: "Pending: ancestor lane 'root' has unresolved rebase conflicts. Open the Rebase/Merge tab to continue.", + }); + + const statuses = await service.listStatuses(); + expect(statuses).toHaveLength(1); + expect(statuses[0].laneId).toBe("lane-a"); + expect(statuses[0].state).toBe("rebasePending"); + }); }); // --------------------------------------------------------------------------- diff --git a/apps/desktop/src/main/services/lanes/autoRebaseService.ts b/apps/desktop/src/main/services/lanes/autoRebaseService.ts index a934040a4..ec5dfac85 100644 --- a/apps/desktop/src/main/services/lanes/autoRebaseService.ts +++ b/apps/desktop/src/main/services/lanes/autoRebaseService.ts @@ -18,8 +18,8 @@ type StoredDismissal = { dismissedAt: string; }; type ListStatusesOptions = { - includeAll?: boolean; lanes?: LaneSummary[]; + preserveLaneIds?: string[]; }; type AttentionStatusInput = { laneId: string; @@ -127,6 +127,12 @@ function blockedMessage( return `Pending: ancestor lane '${laneId}' has unresolved rebase conflicts. Open the Rebase/Merge tab to continue.`; } +function isAncestorBlockedStatus(status: StoredStatus): boolean { + return status.state === "rebasePending" + && typeof status.message === "string" + && status.message.startsWith("Pending: ancestor lane "); +} + function resolveAffectedChainLaneId( laneId: string, laneById: Map, @@ -263,13 +269,19 @@ export function createAutoRebaseService(args: { const listStatuses = async (options?: ListStatusesOptions): Promise => { void maybeSweepRoots("listStatuses"); - const lanes = options?.lanes ?? await laneService.list({ includeArchived: false }); + const shouldLoadFreshLaneStatus = !options?.lanes; + const lanes = options?.lanes ?? await laneService.list({ + includeArchived: false, + includeStatus: shouldLoadFreshLaneStatus, + }); if (disposed) return []; // When a caller-supplied lane subset is provided, laneById is no longer // authoritative for the full active-lane set, so we cannot infer "parent // was deleted" from "parent missing from this slice" — skip the // missing-parent prune in that case. const hasAuthoritativeLaneSet = !options?.lanes; + const hasFreshLaneStatus = shouldLoadFreshLaneStatus; + const preserveLaneIds = new Set((options?.preserveLaneIds ?? []).map((laneId) => laneId.trim()).filter(Boolean)); const laneById = new Map(lanes.map((lane) => [lane.id, lane] as const)); const nowMs = Date.now(); @@ -287,7 +299,13 @@ export function createAutoRebaseService(args: { clearStatus(lane.id); continue; } - } else if (!options?.includeAll && lane.status.behind <= 0 && status.source !== "manual") { + } else if ( + hasFreshLaneStatus + && lane.status.behind <= 0 + && status.source !== "manual" + && !isAncestorBlockedStatus(status) + && !preserveLaneIds.has(lane.id) + ) { clearStatus(lane.id); continue; } else if (hasAuthoritativeLaneSet && status.parentLaneId && !laneById.has(status.parentLaneId)) { @@ -387,7 +405,7 @@ export function createAutoRebaseService(args: { const recordAttentionStatus = async (status: AttentionStatusInput): Promise => { setStatus(status); if (disposed) return; - await emit({ includeAll: true }); + await emit({ preserveLaneIds: [status.laneId] }); }; const dismissStatus = async (args: { laneId: string }): Promise => { @@ -401,7 +419,7 @@ export function createAutoRebaseService(args: { dismissedAt: nowIso(), }); if (disposed) return; - void emit({ includeAll: true }); + void emit(); }; const collectDescendantsDepthFirst = (rootLaneId: string, lanes: LaneSummary[]): string[] => { @@ -651,13 +669,13 @@ export function createAutoRebaseService(args: { state.pending = false; await processRoot(rootLaneId, state.reason); if (disposed) return; - await emit({ includeAll: true }); + await emit(); if (disposed) return; } } catch (error) { logger.warn("autoRebase.run_failed", { rootLaneId, error: String(error) }); if (disposed) return; - await emit({ includeAll: true }); + await emit(); } finally { state.running = false; if (state.pending && !disposed) { diff --git a/apps/desktop/src/main/services/lanes/laneService.test.ts b/apps/desktop/src/main/services/lanes/laneService.test.ts index d099db621..eb18f7eed 100644 --- a/apps/desktop/src/main/services/lanes/laneService.test.ts +++ b/apps/desktop/src/main/services/lanes/laneService.test.ts @@ -2945,10 +2945,9 @@ describe("laneService delete teardown + cancellation + streaming", () => { expect(db.get<{ lane_id: string | null }>("select lane_id from orchestrator_chat_messages where id = ?", ["message-child"])?.lane_id).toBeNull(); }); - it("honors cancelDelete before git_worktree_remove and skips destructive steps", async () => { + it("does not cancel a lane delete after it starts", async () => { const events: any[] = []; const fake = makeFakeServices(); - // Make stop_processes slow so cancelDelete races in before the worktree remove. fake.processService.stopAll.mockImplementation(async () => { await new Promise((r) => setTimeout(r, 50)); }); @@ -2960,15 +2959,15 @@ describe("laneService delete teardown + cancellation + streaming", () => { }); const deletePromise = service.delete({ laneId: "lane-child", deleteBranch: false }); - // Cancel during the slow stop_processes step. await new Promise((r) => setTimeout(r, 5)); const cancelResult = service.cancelDelete("lane-child"); - expect(cancelResult.cancelled).toBe(true); + expect(cancelResult.cancelled).toBe(false); await deletePromise; const last = events[events.length - 1]; - expect(last.progress.overallStatus).toBe("cancelled"); - expect(fake.calls).not.toContain("git_worktree_remove"); + expect(last.progress.overallStatus).toBe("completed"); + expect(last.progress.cancellable).toBe(false); + expect(last.progress.steps.find((step: any) => step.name === "git_worktree_remove")?.status).toBe("completed"); }); it("getDeleteRisk reports running processes, ptys, watchers, and unpushed commits", async () => { diff --git a/apps/desktop/src/main/services/lanes/laneService.ts b/apps/desktop/src/main/services/lanes/laneService.ts index 9c962198c..5aaf2ccd7 100644 --- a/apps/desktop/src/main/services/lanes/laneService.ts +++ b/apps/desktop/src/main/services/lanes/laneService.ts @@ -1861,9 +1861,6 @@ export function createLaneService({ return false; }; - type DeleteToken = { cancelled: boolean; cancellable: boolean }; - const deleteTokens = new Map(); - const broadcastDeleteEvent = (progress: LaneDeleteProgress): void => { if (!onDeleteEvent) return; try { @@ -3536,13 +3533,11 @@ export function createLaneService({ steps: stepNames.map((name): LaneDeleteStep => ({ name, status: "pending" })), startedAt: new Date().toISOString(), overallStatus: "running", - cancellable: true + cancellable: false }; const findStep = (name: LaneDeleteStepName): LaneDeleteStep | undefined => progress.steps.find((s) => s.name === name); - const token: DeleteToken = { cancelled: false, cancellable: true }; - deleteTokens.set(laneId, token); const nonFatalFailures: Array<{ step: LaneDeleteStepName; message: string }> = []; const runStep = async ( @@ -3596,15 +3591,6 @@ export function createLaneService({ broadcastDeleteEvent(progress); }; - const checkpoint = (): boolean => { - if (!token.cancelled) return false; - for (const step of progress.steps) { - if (step.status === "pending") step.status = "skipped"; - } - finalize("cancelled"); - return true; - }; - broadcastDeleteEvent(progress); try { @@ -3617,7 +3603,6 @@ export function createLaneService({ } return { detail: dirty ? "dirty (force enabled)" : "clean" }; }); - if (checkpoint()) return; } await runStep("cancel_auto_rebase", async () => { @@ -3628,7 +3613,6 @@ export function createLaneService({ // ignore } }); - if (checkpoint()) return; await runStep("stop_processes", async () => { const svc = teardownDeps?.processService; @@ -3642,7 +3626,6 @@ export function createLaneService({ } return { detail: `stopped ${active.length} ${active.length === 1 ? "process" : "processes"}` }; }); - if (checkpoint()) return; await runStep("stop_ptys", async () => { const svc = teardownDeps?.ptyService; @@ -3652,7 +3635,6 @@ export function createLaneService({ const disposed = svc.disposeForLane(laneId); return { detail: `closed ${disposed} ${disposed === 1 ? "session" : "sessions"}` }; }); - if (checkpoint()) return; await runStep("stop_watchers", async () => { const svc = teardownDeps?.fileWatcherService; @@ -3662,7 +3644,6 @@ export function createLaneService({ if (before === 0 && stopped === 0) return { detail: "none active" }; return { detail: `stopped ${stopped} ${stopped === 1 ? "watcher" : "watchers"}` }; }); - if (checkpoint()) return; await runStep("cleanup_env", async () => { if (!runtimeOpts?.teardownEnv) return { detail: "no env to clean" }; @@ -3674,18 +3655,10 @@ export function createLaneService({ return { detail: `warning: ${err instanceof Error ? err.message : String(err)}` }; } }); - if (checkpoint()) return; // Brief grace period so kernel-level handle releases settle on macOS before // git tries to unlink the worktree directory. await new Promise((resolve) => setTimeout(resolve, 250)); - if (checkpoint()) return; - - // Past the point of no return: any further failure leaves partial state, - // so cancellation is no longer honored. - token.cancellable = false; - progress.cancellable = false; - broadcastDeleteEvent(progress); if (hasWorktree) { await runStep("git_worktree_remove", async () => { @@ -3797,17 +3770,11 @@ export function createLaneService({ } catch (error) { finalize("failed"); throw error; - } finally { - deleteTokens.delete(laneId); } }, cancelDelete(laneId: string): { cancelled: boolean; reason?: string } { - const token = deleteTokens.get(laneId); - if (!token) return { cancelled: false, reason: "no active delete" }; - if (!token.cancellable) return { cancelled: false, reason: "past cancellation window" }; - token.cancelled = true; - return { cancelled: true }; + return { cancelled: false, reason: `Lane deletes run to completion once started: ${laneId}` }; }, async getDeleteRisk(laneId: string): Promise { diff --git a/apps/desktop/src/main/services/prs/prService.test.ts b/apps/desktop/src/main/services/prs/prService.test.ts index 942884656..d1646819d 100644 --- a/apps/desktop/src/main/services/prs/prService.test.ts +++ b/apps/desktop/src/main/services/prs/prService.test.ts @@ -185,12 +185,14 @@ interface BuildServiceOpts { githubService?: any; laneService?: any; db?: any; + conflictService?: any; } function buildService(opts: BuildServiceOpts = {}) { const db = opts.db ?? makeMockDb(); const githubService = opts.githubService ?? makeGithubService(); const laneService = opts.laneService ?? makeLaneService(); + const logger = makeLogger(); mockGit.runGit.mockImplementation(async (args: unknown[]) => { const command = Array.isArray(args) ? args[0] : null; @@ -208,17 +210,18 @@ function buildService(opts: BuildServiceOpts = {}) { const service = createPrService({ db, - logger: makeLogger(), + logger, projectId: "proj-1", projectRoot: "/tmp/test-project", laneService, operationService: makeOperationService(), githubService, + conflictService: opts.conflictService, projectConfigService: makeProjectConfigService(), openExternal: vi.fn(async () => {}), }); - return { service, db, githubService, laneService }; + return { service, db, githubService, laneService, logger }; } // --------------------------------------------------------------------------- @@ -400,7 +403,7 @@ describe("prService.getGithubSnapshot", () => { nowSpy.mockReturnValue(Date.parse("2026-01-01T00:00:00Z") + GITHUB_SNAPSHOT_TTL_MS_FOR_TEST + 1); const stale = await service.getGithubSnapshot(); expect(stale.repoPullRequests[0]?.title).toBe("Cached PR"); - await flushMicrotasks(); + await flushMicrotasks(30); expect(githubService.apiRequest).toHaveBeenCalledTimes(3); resolveRevalidation({ data: [makeGitHubPull({ title: "Fresh PR", updated_at: "2026-01-01T00:05:00Z" })] }); @@ -414,6 +417,384 @@ describe("prService.getGithubSnapshot", () => { } }); + it("fetches open external PRs by default and only includes closed external history when requested", async () => { + const githubService = makeGithubService({ + getStatus: vi.fn(async () => ({ + tokenStored: true, + repo: REPO, + userLogin: "octocat", + })), + apiRequest: vi.fn(async (args: { path: string }) => ({ + data: args.path === "/search/issues" ? { items: [] } : [], + })), + }); + const { service } = buildService({ githubService, laneService: makeLaneService([]) }); + + await service.getGithubSnapshot({ force: true }); + const defaultExternalCall = githubService.apiRequest.mock.calls.find(([args]: [{ path: string }]) => args.path === "/search/issues"); + expect(defaultExternalCall?.[0].query.q).toContain("is:open"); + + githubService.apiRequest.mockClear(); + await service.getGithubSnapshot({ force: true, includeExternalClosed: true }); + const fullHistoryExternalCall = githubService.apiRequest.mock.calls.find(([args]: [{ path: string }]) => args.path === "/search/issues"); + expect(fullHistoryExternalCall?.[0].query.q).not.toContain("is:open"); + }); + + it("does not reuse or overwrite full-history snapshots with narrower in-flight requests", async () => { + let resolveOpenRepo!: (value: unknown) => void; + const openRepoRequest = new Promise((resolve) => { + resolveOpenRepo = resolve; + }); + let repoCalls = 0; + const githubService = makeGithubService({ + getStatus: vi.fn(async () => ({ + tokenStored: true, + repo: REPO, + userLogin: "octocat", + })), + apiRequest: vi.fn(async (args: { path: string; query?: { q?: string } }) => { + if (args.path === `/repos/${REPO.owner}/${REPO.name}/pulls`) { + repoCalls += 1; + if (repoCalls === 1) return openRepoRequest; + return { data: [makeGitHubPull({ number: 2, title: "Full history PR" })] }; + } + return { + data: { + items: [ + makeGitHubPull({ + number: args.query?.q?.includes("is:open") ? 3 : 4, + title: args.query?.q?.includes("is:open") ? "Open external" : "Closed external", + pull_request: { url: "https://api.github.com/repos/elsewhere/project/pulls/4" }, + repository_url: "https://api.github.com/repos/elsewhere/project", + }), + ], + }, + }; + }), + }); + const { service } = buildService({ githubService, laneService: makeLaneService([]) }); + + const openOnly = service.getGithubSnapshot(); + await flushMicrotasks(); + + const fullHistory = await service.getGithubSnapshot({ includeExternalClosed: true }); + expect(fullHistory.repoPullRequests[0]?.title).toBe("Full history PR"); + expect(fullHistory.externalPullRequests[0]?.title).toBe("Closed external"); + + resolveOpenRepo({ data: [makeGitHubPull({ number: 1, title: "Open-only PR" })] }); + await expect(openOnly).resolves.toEqual(expect.objectContaining({ + repoPullRequests: [expect.objectContaining({ title: "Open-only PR" })], + })); + + const cachedFullHistory = await service.getGithubSnapshot({ includeExternalClosed: true }); + expect(cachedFullHistory.repoPullRequests[0]?.title).toBe("Full history PR"); + expect(cachedFullHistory.externalPullRequests[0]?.title).toBe("Closed external"); + expect(repoCalls).toBe(2); + }); + + it("keeps a superseded open-only snapshot cached when a full-history upgrade fails", async () => { + let resolveOpenRepo!: (value: unknown) => void; + const openRepoRequest = new Promise((resolve) => { + resolveOpenRepo = resolve; + }); + let rejectFullRepo!: (reason: unknown) => void; + const fullRepoRequest = new Promise((_resolve, reject) => { + rejectFullRepo = reject; + }); + let repoCalls = 0; + const githubService = makeGithubService({ + getStatus: vi.fn(async () => ({ + tokenStored: true, + repo: REPO, + userLogin: "octocat", + })), + apiRequest: vi.fn(async (args: { path: string; query?: { q?: string } }) => { + if (args.path === `/repos/${REPO.owner}/${REPO.name}/pulls`) { + repoCalls += 1; + if (repoCalls === 1) return openRepoRequest; + return fullRepoRequest; + } + return { + data: { + items: [ + makeGitHubPull({ + number: 3, + title: args.query?.q?.includes("is:open") ? "Open external" : "Closed external", + pull_request: { url: "https://api.github.com/repos/elsewhere/project/pulls/3" }, + repository_url: "https://api.github.com/repos/elsewhere/project", + }), + ], + }, + }; + }), + }); + const { service } = buildService({ githubService, laneService: makeLaneService([]) }); + + const openOnly = service.getGithubSnapshot(); + await flushMicrotasks(); + + const fullHistory = service.getGithubSnapshot({ includeExternalClosed: true }); + await flushMicrotasks(); + + resolveOpenRepo({ data: [makeGitHubPull({ number: 1, title: "Open-only PR" })] }); + await expect(openOnly).resolves.toEqual(expect.objectContaining({ + repoPullRequests: [expect.objectContaining({ title: "Open-only PR" })], + externalPullRequests: [expect.objectContaining({ title: "Open external" })], + })); + + rejectFullRepo(new Error("full history failed")); + await expect(fullHistory).rejects.toThrow("full history failed"); + + const apiCallsAfterOpenOnly = githubService.apiRequest.mock.calls.length; + const cachedOpenOnly = await service.getGithubSnapshot(); + expect(cachedOpenOnly.repoPullRequests[0]?.title).toBe("Open-only PR"); + expect(cachedOpenOnly.externalPullRequests[0]?.title).toBe("Open external"); + expect(githubService.apiRequest).toHaveBeenCalledTimes(apiCallsAfterOpenOnly); + expect(repoCalls).toBe(2); + }); + + it("keeps the existing open-only cache when a superseded refresh and full-history upgrade fail", async () => { + let resolveOpenRepo!: (value: unknown) => void; + const openRepoRequest = new Promise((resolve) => { + resolveOpenRepo = resolve; + }); + let rejectFullRepo!: (reason: unknown) => void; + const fullRepoRequest = new Promise((_resolve, reject) => { + rejectFullRepo = reject; + }); + let repoCalls = 0; + const githubService = makeGithubService({ + getStatus: vi.fn(async () => ({ + tokenStored: true, + repo: REPO, + userLogin: "octocat", + })), + apiRequest: vi.fn(async (args: { path: string; query?: { q?: string } }) => { + if (args.path === `/repos/${REPO.owner}/${REPO.name}/pulls`) { + repoCalls += 1; + if (repoCalls === 1) return { data: [makeGitHubPull({ number: 1, title: "Cached open-only PR" })] }; + if (repoCalls === 2) return openRepoRequest; + return fullRepoRequest; + } + return { + data: { + items: [ + makeGitHubPull({ + number: args.query?.q?.includes("is:open") ? 3 : 4, + title: args.query?.q?.includes("is:open") ? "Open external" : "Closed external", + pull_request: { url: "https://api.github.com/repos/elsewhere/project/pulls/4" }, + repository_url: "https://api.github.com/repos/elsewhere/project", + }), + ], + }, + }; + }), + }); + const { service } = buildService({ githubService, laneService: makeLaneService([]) }); + const cached = await service.getGithubSnapshot({ force: true }); + expect(cached.repoPullRequests[0]?.title).toBe("Cached open-only PR"); + + const openOnlyRefresh = service.getGithubSnapshot({ force: true }); + await flushMicrotasks(); + const fullHistory = service.getGithubSnapshot({ includeExternalClosed: true }); + await flushMicrotasks(); + + resolveOpenRepo({ data: [makeGitHubPull({ number: 2, title: "Superseded open-only PR" })] }); + await expect(openOnlyRefresh).resolves.toEqual(expect.objectContaining({ + repoPullRequests: [expect.objectContaining({ title: "Superseded open-only PR" })], + })); + rejectFullRepo(new Error("full history failed")); + await expect(fullHistory).rejects.toThrow("full history failed"); + + const apiCallsAfterFailure = githubService.apiRequest.mock.calls.length; + const cachedOpenOnly = await service.getGithubSnapshot(); + expect(cachedOpenOnly.repoPullRequests[0]?.title).toBe("Cached open-only PR"); + expect(githubService.apiRequest).toHaveBeenCalledTimes(apiCallsAfterFailure); + expect(repoCalls).toBe(3); + }); + + it("does not publish a fallback snapshot from a superseded full-history failure", async () => { + let resolveOpenRepo!: (value: unknown) => void; + const openRepoRequest = new Promise((resolve) => { + resolveOpenRepo = resolve; + }); + let rejectSupersededFullRepo!: (reason: unknown) => void; + const supersededFullRepoRequest = new Promise((_resolve, reject) => { + rejectSupersededFullRepo = reject; + }); + let resolveCurrentFullRepo!: (value: unknown) => void; + const currentFullRepoRequest = new Promise((resolve) => { + resolveCurrentFullRepo = resolve; + }); + let repoCalls = 0; + const githubService = makeGithubService({ + getStatus: vi.fn(async () => ({ + tokenStored: true, + repo: REPO, + userLogin: "octocat", + })), + apiRequest: vi.fn(async (args: { path: string; query?: { q?: string } }) => { + if (args.path === `/repos/${REPO.owner}/${REPO.name}/pulls`) { + repoCalls += 1; + if (repoCalls === 1) return openRepoRequest; + if (repoCalls === 2) return supersededFullRepoRequest; + return currentFullRepoRequest; + } + return { + data: { + items: [ + makeGitHubPull({ + number: args.query?.q?.includes("is:open") ? 3 : 4, + title: args.query?.q?.includes("is:open") ? "Open external" : "Closed external", + pull_request: { url: "https://api.github.com/repos/elsewhere/project/pulls/4" }, + repository_url: "https://api.github.com/repos/elsewhere/project", + }), + ], + }, + }; + }), + }); + const { service } = buildService({ githubService, laneService: makeLaneService([]) }); + + const openOnly = service.getGithubSnapshot(); + await flushMicrotasks(); + const supersededFullHistory = service.getGithubSnapshot({ includeExternalClosed: true }); + await flushMicrotasks(); + + resolveOpenRepo({ data: [makeGitHubPull({ number: 1, title: "Open-only PR" })] }); + await expect(openOnly).resolves.toEqual(expect.objectContaining({ + repoPullRequests: [expect.objectContaining({ title: "Open-only PR" })], + })); + + const currentFullHistory = service.getGithubSnapshot({ force: true, includeExternalClosed: true }); + await flushMicrotasks(); + rejectSupersededFullRepo(new Error("superseded full history failed")); + await expect(supersededFullHistory).rejects.toThrow("superseded full history failed"); + + const defaultSnapshot = service.getGithubSnapshot(); + resolveCurrentFullRepo({ data: [makeGitHubPull({ number: 2, title: "Current full-history PR" })] }); + + await expect(currentFullHistory).resolves.toEqual(expect.objectContaining({ + repoPullRequests: [expect.objectContaining({ title: "Current full-history PR" })], + })); + await expect(defaultSnapshot).resolves.toEqual(expect.objectContaining({ + repoPullRequests: [expect.objectContaining({ title: "Current full-history PR" })], + })); + expect(repoCalls).toBe(3); + }); + + it("does not let a superseded open-only snapshot overwrite a fresher cache", async () => { + let resolveStaleRepo!: (value: unknown) => void; + const staleRepoRequest = new Promise((resolve) => { + resolveStaleRepo = resolve; + }); + let resolveFreshRepo!: (value: unknown) => void; + const freshRepoRequest = new Promise((resolve) => { + resolveFreshRepo = resolve; + }); + let repoCalls = 0; + const githubService = makeGithubService({ + getStatus: vi.fn(async () => ({ + tokenStored: true, + repo: REPO, + userLogin: "octocat", + })), + apiRequest: vi.fn(async (args: { path: string }) => { + if (args.path === `/repos/${REPO.owner}/${REPO.name}/pulls`) { + repoCalls += 1; + if (repoCalls === 1) return staleRepoRequest; + return freshRepoRequest; + } + return { data: { items: [] } }; + }), + }); + const { service } = buildService({ githubService, laneService: makeLaneService([]) }); + + const staleRequest = service.getGithubSnapshot({ force: true }); + await flushMicrotasks(); + const freshRequest = service.getGithubSnapshot({ force: true }); + await flushMicrotasks(); + + resolveFreshRepo({ data: [makeGitHubPull({ number: 2, title: "Fresh open-only PR" })] }); + await expect(freshRequest).resolves.toEqual(expect.objectContaining({ + repoPullRequests: [expect.objectContaining({ title: "Fresh open-only PR" })], + })); + + resolveStaleRepo({ data: [makeGitHubPull({ number: 1, title: "Stale open-only PR" })] }); + await expect(staleRequest).resolves.toEqual(expect.objectContaining({ + repoPullRequests: [expect.objectContaining({ title: "Stale open-only PR" })], + })); + + const cachedSnapshot = await service.getGithubSnapshot(); + expect(cachedSnapshot.repoPullRequests[0]?.title).toBe("Fresh open-only PR"); + expect(repoCalls).toBe(2); + }); + + it("preserves full-history cache mode during stale open-only revalidation", async () => { + const initialNow = Date.parse("2026-01-01T00:00:00Z"); + const nowSpy = vi.spyOn(Date, "now").mockReturnValue(initialNow); + let repoCalls = 0; + const externalQueries: string[] = []; + const githubService = makeGithubService({ + getStatus: vi.fn(async () => ({ + tokenStored: true, + repo: REPO, + userLogin: "octocat", + })), + apiRequest: vi.fn(async (args: { path: string; query?: { q?: string } }) => { + if (args.path === `/repos/${REPO.owner}/${REPO.name}/pulls`) { + repoCalls += 1; + return { + data: [ + makeGitHubPull({ + number: repoCalls, + title: repoCalls === 1 ? "Cached full history" : "Fresh full history", + }), + ], + }; + } + externalQueries.push(args.query?.q ?? ""); + return { + data: { + items: [ + makeGitHubPull({ + number: externalQueries.length === 1 ? 10 : 11, + title: externalQueries.length === 1 ? "Cached closed external" : "Fresh closed external", + state: "closed", + pull_request: { url: "https://api.github.com/repos/elsewhere/project/pulls/11" }, + repository_url: "https://api.github.com/repos/elsewhere/project", + }), + ], + }, + }; + }), + }); + const { service } = buildService({ githubService, laneService: makeLaneService([]) }); + + try { + const fullHistory = await service.getGithubSnapshot({ includeExternalClosed: true }); + expect(fullHistory.externalPullRequests[0]?.title).toBe("Cached closed external"); + expect(externalQueries[0]).not.toContain("is:open"); + + nowSpy.mockReturnValue(Date.parse("2030-01-01T00:00:00Z")); + const staleOpenOnly = await service.getGithubSnapshot(); + expect(staleOpenOnly.repoPullRequests[0]?.title).toBe("Cached full history"); + + const refreshedFullHistory = await service.getGithubSnapshot({ includeExternalClosed: true }); + expect(externalQueries[1]).not.toContain("is:open"); + expect(refreshedFullHistory.repoPullRequests[0]?.title).toBe("Fresh full history"); + expect(refreshedFullHistory.externalPullRequests[0]?.title).toBe("Fresh closed external"); + + const cachedFullHistory = await service.getGithubSnapshot({ includeExternalClosed: true }); + expect(cachedFullHistory.repoPullRequests[0]?.title).toBe("Fresh full history"); + expect(cachedFullHistory.externalPullRequests[0]?.title).toBe("Fresh closed external"); + expect(repoCalls).toBe(2); + expect(externalQueries).toHaveLength(2); + } finally { + nowSpy.mockRestore(); + } + }); + it("backfills a lane PR row from GitHub when the head branch matches an active lane", async () => { const githubService = makeGithubService({ getStatus: vi.fn(async () => ({ @@ -511,6 +892,393 @@ describe("prService.getGithubSnapshot", () => { }); }); +describe("prService.listWithConflicts", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("batches conflict analysis against all active lanes, including non-PR peers", async () => { + const prLane = makeFakeLane({ id: "lane-pr", name: "PR lane" }); + const peerLane = makeFakeLane({ id: "lane-peer", name: "Peer lane" }); + const db = makeMockDb(); + db.all.mockImplementation((sql: string) => { + if (String(sql).includes("from pull_requests")) { + return [makePrRow({ id: "pr-1", lane_id: "lane-pr" })]; + } + return []; + }); + const conflictService = { + getBatchAssessment: vi.fn(async () => ({ + lanes: [ + { laneId: "lane-pr", status: "conflict-predicted", overlappingFileCount: 1 }, + { laneId: "lane-peer", status: "clean", overlappingFileCount: 1 }, + ], + matrix: [ + { laneAId: "lane-pr", laneBId: "lane-peer", riskLevel: "high", overlapCount: 1 }, + ], + overlaps: [ + { laneAId: "lane-pr", laneBId: "lane-peer", files: ["src/shared.ts"] }, + ], + })), + }; + const laneService = makeLaneService([prLane, peerLane]); + const { service } = buildService({ db, laneService, conflictService }); + + const rows = await service.listWithConflicts({ includeConflictAnalysis: true }); + + expect(laneService.list).toHaveBeenCalledWith({ includeArchived: false, includeStatus: false }); + expect(conflictService.getBatchAssessment).toHaveBeenCalledWith({ lanes: [prLane, peerLane] }); + expect(rows[0]?.conflictAnalysis).toEqual(expect.objectContaining({ + prId: "pr-1", + laneId: "lane-pr", + riskLevel: "high", + overlapCount: 1, + conflictPredicted: true, + peerConflicts: [ + { + peerId: "lane-peer", + peerName: "Peer lane", + riskLevel: "high", + overlapFiles: ["src/shared.ts"], + }, + ], + })); + }); + + it("logs when batched conflict analysis falls back to null results", async () => { + const db = makeMockDb(); + db.all.mockImplementation((sql: string) => { + if (String(sql).includes("from pull_requests")) { + return [makePrRow({ id: "pr-1", lane_id: "lane-pr" })]; + } + return []; + }); + const conflictService = { + getBatchAssessment: vi.fn(async () => { + throw new Error("batch failed"); + }), + }; + const { service, logger } = buildService({ + db, + laneService: makeLaneService([makeFakeLane({ id: "lane-pr" })]), + conflictService, + }); + + const rows = await service.listWithConflicts({ includeConflictAnalysis: true }); + + expect(rows[0]?.conflictAnalysis).toBeNull(); + expect(logger.warn).toHaveBeenCalledWith("prs.batch_conflict_analysis_failed", { error: "batch failed" }); + }); + + it("defaults to listing PRs without conflict analysis", async () => { + const db = makeMockDb(); + db.all.mockImplementation((sql: string) => { + if (String(sql).includes("from pull_requests")) { + return [makePrRow({ id: "pr-1", lane_id: "lane-pr" })]; + } + return []; + }); + const conflictService = { + getBatchAssessment: vi.fn(async () => ({ lanes: [], matrix: [], overlaps: [] })), + }; + const { service } = buildService({ + db, + laneService: makeLaneService([makeFakeLane({ id: "lane-pr" })]), + conflictService, + }); + + const rows = await service.listWithConflicts(); + + expect(rows[0]?.conflictAnalysis).toBeNull(); + expect(conflictService.getBatchAssessment).not.toHaveBeenCalled(); + }); +}); + +describe("prService.refresh", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + function makeRefreshDb(rows: ReturnType[]) { + const db = makeMockDb(); + db.get.mockImplementation((sql: string, params: unknown[]) => { + const text = String(sql); + if (text.includes("from pull_requests") && text.includes("where id = ?")) { + return rows.find((row) => row.id === params[0]) ?? null; + } + return null; + }); + return db; + } + + function makeRefreshGithubService(failingPrNumbers = new Set()) { + return makeGithubService({ + apiRequest: vi.fn(async (args: { path: string }) => { + const path = args.path; + const prMatch = path.match(/\/pulls\/(\d+)$/); + if (prMatch) { + const prNumber = Number(prMatch[1]); + if (failingPrNumbers.has(prNumber)) { + throw new Error(`refresh failed for #${prNumber}`); + } + return { + data: makeGitHubPull({ + number: prNumber, + title: `Fresh #${prNumber}`, + head: { ref: `feature/pr-${prNumber}`, sha: `sha-${prNumber}` }, + additions: 10, + deletions: 2, + }), + }; + } + if (/\/commits\/sha-\d+\/status$/.test(path)) { + return { data: { state: "success", statuses: [] } }; + } + if (/\/commits\/sha-\d+\/check-runs$/.test(path)) { + return { data: { check_runs: [] } }; + } + if (/\/pulls\/\d+\/reviews$/.test(path)) { + return { data: [] }; + } + throw new Error(`Unexpected GitHub API path: ${path}`); + }), + }); + } + + it("keeps successful explicit PR refreshes when a sibling fails", async () => { + const okRow = makePrRow({ id: "pr-ok", github_pr_number: 90 }); + const failingRow = makePrRow({ id: "pr-bad", github_pr_number: 91 }); + const { service, logger } = buildService({ + db: makeRefreshDb([okRow, failingRow]), + githubService: makeRefreshGithubService(new Set([91])), + }); + + const refreshed = await service.refresh({ prIds: ["pr-ok", "pr-bad"] }); + + expect(refreshed).toHaveLength(1); + expect(refreshed[0]).toEqual(expect.objectContaining({ + id: "pr-ok", + githubPrNumber: 90, + title: "Fresh #90", + })); + expect(logger.warn).toHaveBeenCalledWith("prs.refresh_failed", { + prId: "pr-bad", + error: "refresh failed for #91", + }); + }); + + it("rejects explicit multi-PR refreshes when every PR fails", async () => { + const firstRow = makePrRow({ id: "pr-bad-1", github_pr_number: 91 }); + const secondRow = makePrRow({ id: "pr-bad-2", github_pr_number: 92 }); + const { service, logger } = buildService({ + db: makeRefreshDb([firstRow, secondRow]), + githubService: makeRefreshGithubService(new Set([91, 92])), + }); + + await expect(service.refresh({ prIds: ["pr-bad-1", "pr-bad-2"] })).rejects.toThrow("refresh failed for #91"); + expect(logger.warn).toHaveBeenCalledWith("prs.refresh_failed", { + prId: "pr-bad-1", + error: "refresh failed for #91", + }); + expect(logger.warn).toHaveBeenCalledWith("prs.refresh_failed", { + prId: "pr-bad-2", + error: "refresh failed for #92", + }); + }); + + it("still rejects explicit single-PR refresh failures", async () => { + const failingRow = makePrRow({ id: "pr-bad", github_pr_number: 91 }); + const { service, logger } = buildService({ + db: makeRefreshDb([failingRow]), + githubService: makeRefreshGithubService(new Set([91])), + }); + + await expect(service.refresh({ prId: "pr-bad" })).rejects.toThrow("refresh failed for #91"); + expect(logger.warn).toHaveBeenCalledWith("prs.refresh_failed", { + prId: "pr-bad", + error: "refresh failed for #91", + }); + }); +}); + +describe("prService merge contexts", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("resolves target lanes by branch name when lane refs include refs/heads prefixes", async () => { + const sourceLane = makeFakeLane({ id: "lane-feature", branchRef: "refs/heads/feature/pr" }); + const targetLane = makeFakeLane({ id: "lane-main", branchRef: "refs/heads/main", name: "Main" }); + const row = makePrRow({ id: "pr-1", lane_id: sourceLane.id, base_branch: "main", head_branch: "feature/pr" }); + const db = makeMockDb(); + db.get.mockImplementation((sql: string) => { + const text = String(sql); + if (text.includes("from pull_requests")) return row; + if (text.includes("from pr_group_members")) return null; + return null; + }); + db.all.mockImplementation((sql: string) => { + if (String(sql).includes("from pull_requests")) return [row]; + return []; + }); + const { service } = buildService({ db, laneService: makeLaneService([sourceLane, targetLane]) }); + + await expect(service.getMergeContext("pr-1")).resolves.toEqual(expect.objectContaining({ + targetLaneId: "lane-main", + })); + await expect(service.getMergeContexts(["pr-1"])).resolves.toEqual({ + "pr-1": expect.objectContaining({ targetLaneId: "lane-main" }), + }); + }); + + it("uses the same group assembly for single and bulk merge-context reads", async () => { + const sourceLane = makeFakeLane({ id: "lane-source", branchRef: "refs/heads/feature/pr", name: "Feature" }); + const integrationLane = makeFakeLane({ id: "lane-integration", branchRef: "refs/heads/integration/pr", name: "Integration" }); + const targetLane = makeFakeLane({ id: "lane-main", branchRef: "refs/heads/main", name: "Main" }); + const row = makePrRow({ id: "pr-1", lane_id: sourceLane.id, base_branch: "main", head_branch: "feature/pr" }); + const group = { group_id: "group-1", group_type: "integration" as const }; + const memberRows = [ + { + group_id: group.group_id, + pr_id: "pr-1", + lane_id: sourceLane.id, + position: 0, + role: "source", + lane_name: sourceLane.name, + pr_number: 90, + }, + { + group_id: group.group_id, + pr_id: "pr-integration", + lane_id: integrationLane.id, + position: 1, + role: "integration", + lane_name: integrationLane.name, + pr_number: 91, + }, + ]; + const db = makeMockDb(); + db.get.mockImplementation((sql: string) => { + const text = String(sql); + if (text.includes("from pull_requests")) return row; + if (text.includes("from pr_group_members")) return group; + return null; + }); + db.all.mockImplementation((sql: string) => { + const text = String(sql); + if (text.includes("from pull_requests")) return [row]; + if (text.includes("from pr_group_members") && text.includes("join pr_groups")) { + return [{ ...group, pr_id: row.id }]; + } + if (text.includes("from pr_group_members")) return memberRows; + return []; + }); + const { service } = buildService({ + db, + laneService: makeLaneService([sourceLane, integrationLane, targetLane]), + }); + + const single = await service.getMergeContext("pr-1"); + const bulk = await service.getMergeContexts(["pr-1"]); + + expect(bulk["pr-1"]).toEqual(single); + const allDbCalls = db.all.mock.calls as Array<[unknown, ...unknown[]]>; + expect(allDbCalls.some(([sql]) => String(sql).includes("row_number() over"))).toBe(true); + expect(single).toEqual(expect.objectContaining({ + groupId: group.group_id, + groupType: "integration", + sourceLaneIds: [sourceLane.id], + targetLaneId: targetLane.id, + integrationLaneId: integrationLane.id, + })); + }); + + it("returns empty merge contexts for requested PR ids missing from storage", async () => { + const db = makeMockDb(); + db.get.mockReturnValue(null); + db.all.mockImplementation((sql: string) => { + if (String(sql).includes("from pull_requests")) return []; + return []; + }); + const { service } = buildService({ db, laneService: makeLaneService([]) }); + + await expect(service.getMergeContext("external-pr")).resolves.toEqual({ + prId: "external-pr", + groupId: null, + groupType: null, + sourceLaneIds: [], + targetLaneId: null, + integrationLaneId: null, + members: [], + }); + await expect(service.getMergeContexts(["external-pr"])).resolves.toEqual({ + "external-pr": { + prId: "external-pr", + groupId: null, + groupType: null, + sourceLaneIds: [], + targetLaneId: null, + integrationLaneId: null, + members: [], + }, + }); + }); + + it("chunks bulk merge-context lookups below SQLite's bind parameter limit", async () => { + const prIds = Array.from({ length: 1_005 }, (_value, index) => `pr-${index}`); + const rowsById = new Map(prIds.map((id, index) => [ + id, + makePrRow({ + id, + lane_id: `lane-${index}`, + github_pr_number: index + 1, + head_branch: `feature/${index}`, + }), + ] as const)); + const paramCounts = { + pullRequests: [] as number[], + groupLookups: [] as number[], + memberLookups: [] as number[], + }; + const db = makeMockDb(); + db.all.mockImplementation((sql: string, params: unknown[] = []) => { + const text = String(sql); + if (text.includes("from pull_requests")) { + paramCounts.pullRequests.push(params.length); + return params.slice(1).map((id) => rowsById.get(String(id))).filter(Boolean); + } + if (text.includes("from pr_group_members") && text.includes("join pr_groups")) { + paramCounts.groupLookups.push(params.length); + return params.slice(1).map((id) => ({ + pr_id: String(id), + group_id: `group-${String(id)}`, + group_type: "queue" as const, + })); + } + if (text.includes("from pr_group_members")) { + paramCounts.memberLookups.push(params.length); + return []; + } + return []; + }); + const { service } = buildService({ db, laneService: makeLaneService([]) }); + + const contexts = await service.getMergeContexts(prIds); + + expect(Object.keys(contexts)).toHaveLength(prIds.length); + expect(paramCounts.pullRequests).toHaveLength(2); + expect(paramCounts.groupLookups).toHaveLength(2); + expect(paramCounts.memberLookups).toHaveLength(2); + for (const count of [ + ...paramCounts.pullRequests, + ...paramCounts.groupLookups, + ...paramCounts.memberLookups, + ]) { + expect(count).toBeLessThanOrEqual(902); + } + }); +}); + describe("prService.createFromLane", () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/apps/desktop/src/main/services/prs/prService.ts b/apps/desktop/src/main/services/prs/prService.ts index c304bea63..77c743876 100644 --- a/apps/desktop/src/main/services/prs/prService.ts +++ b/apps/desktop/src/main/services/prs/prService.ts @@ -57,6 +57,7 @@ import type { PrState, PrStatus, PrSummary, + PrSnapshotHydration, PrWithConflicts, PrActionCapabilities, PrCreateCapabilities, @@ -176,22 +177,20 @@ type LanePrLookupRow = { archived_at: string | null; }; +const SQL_IN_CLAUSE_CHUNK_SIZE = 900; + +function chunkValues(values: readonly T[], size = SQL_IN_CLAUSE_CHUNK_SIZE): T[][] { + const chunks: T[][] = []; + for (let index = 0; index < values.length; index += size) { + chunks.push(values.slice(index, index + size)); + } + return chunks; +} + function isActivePrState(state: string | null | undefined): boolean { return state === "open" || state === "draft"; } -type PullRequestSnapshotHydration = { - prId: string; - detail: PrDetail | null; - status: PrStatus | null; - checks: PrCheck[]; - reviews: PrReview[]; - comments: PrComment[]; - files: PrFile[]; - commits: PrCommit[]; - updatedAt: string | null; -}; - type IntegrationProposalRow = { id: string; source_lane_ids_json: string; @@ -550,6 +549,7 @@ function rowToSummary(row: PullRequestRow): PrSummary { } const BACKGROUND_REFRESH_MAX_PRS = 4; +const REFRESH_CONCURRENCY = 4; const BACKGROUND_REFRESH_MIN_STALE_MS = 2 * 60_000; const BACKGROUND_REFRESH_CLOSED_STALE_MS = 15 * 60_000; @@ -1362,7 +1362,7 @@ export function createPrService({ } }; - const listSnapshotRows = (args: { prId?: string } = {}): PullRequestSnapshotHydration[] => { + const listSnapshotRows = (args: { prId?: string } = {}): PrSnapshotHydration[] => { const rows = db.all<{ pr_id: string; detail_json: string | null; @@ -2275,6 +2275,47 @@ export function createPrService({ return updated; }; + const refreshPrIds = async (prIds: string[]): Promise => { + const uniquePrIds = [...new Set(prIds.map((prId) => String(prId ?? "").trim()).filter(Boolean))]; + const refreshed: PrSummary[] = []; + const failures: Array<{ prId: string; reason: unknown }> = []; + for (let i = 0; i < uniquePrIds.length; i += REFRESH_CONCURRENCY) { + const batchPrIds = uniquePrIds.slice(i, i + REFRESH_CONCURRENCY); + const results = await Promise.allSettled(batchPrIds.map((prId) => refreshOne(prId))); + results.forEach((result, index) => { + const prId = batchPrIds[index] ?? ""; + if (result.status === "fulfilled") { + refreshed.push(result.value); + return; + } + failures.push({ prId, reason: result.reason }); + logger.warn("prs.refresh_failed", { prId, error: getErrorMessage(result.reason) }); + }); + } + if (failures[0] && refreshed.length === 0) { + throw failures[0].reason; + } + return refreshed; + }; + + const refreshRowsBestEffort = async (rows: PullRequestRow[]): Promise => { + const seen = new Set(); + const uniqueRows = rows.filter((row) => { + if (seen.has(row.id)) return false; + seen.add(row.id); + return true; + }); + for (let i = 0; i < uniqueRows.length; i += REFRESH_CONCURRENCY) { + await Promise.all(uniqueRows.slice(i, i + REFRESH_CONCURRENCY).map(async (row) => { + try { + await refreshOne(row.id); + } catch (error) { + logger.warn("prs.refresh_failed", { prId: row.id, error: getErrorMessage(error) }); + } + })); + } + }; + const computeStatus = async (summary: PrSummary): Promise => { const repo: GitHubRepoRef = { owner: summary.repoOwner, name: summary.repoName }; const pr = await fetchPr(repo, summary.githubPrNumber); @@ -3954,20 +3995,39 @@ export function createPrService({ }; }; - const getMergeContext = async (prId: string): Promise => { - const row = getRow(prId); - if (!row) throw new Error(`PR not found: ${prId}`); - - const lanes = await laneService.list({ includeArchived: false }); + const buildMergeContextLaneLookups = (lanes: LaneSummary[]) => { const laneById = new Map(lanes.map((lane) => [lane.id, lane] as const)); - const findLaneIdByBranch = (rawBranch: string): string | null => { - const normalized = normalizeBranchName(rawBranch); - if (!normalized) return null; - const byBranch = lanes.find((lane) => normalizeBranchName(lane.branchRef) === normalized); - return byBranch?.id ?? null; - }; + const laneIdByBranch = new Map( + lanes + .map((lane) => [normalizeBranchName(branchNameFromRef(lane.branchRef)), lane.id] as const) + .filter(([branch]) => Boolean(branch)), + ); + return { laneById, laneIdByBranch }; + }; + + const mapMergeContextMember = ( + member: PrGroupMemberLookupRow, + laneById: Map, + ): PrMergeContext["members"][number] => ({ + prId: member.pr_id, + laneId: member.lane_id, + laneName: member.lane_name ?? laneById.get(member.lane_id)?.name ?? member.lane_id, + prNumber: Number.isFinite(Number(member.pr_number)) ? Number(member.pr_number) : null, + position: Number(member.position), + role: normalizeGroupMemberRole(String(member.role ?? "source")), + }); - const fallbackTargetLaneId = findLaneIdByBranch(row.base_branch); + const buildMergeContextFromLookups = ( + row: PullRequestRow, + lookups: { + laneById: Map; + laneIdByBranch: Map; + groupByPrId: Map; + membersByGroupId: Map; + }, + ): PrMergeContext => { + const { laneById, laneIdByBranch, groupByPrId, membersByGroupId } = lookups; + const fallbackTargetLaneId = laneIdByBranch.get(normalizeBranchName(row.base_branch)) ?? null; const fallbackSourceLaneId = row.lane_id; const fallbackMembers: PrMergeContext["members"] = [ { @@ -3980,22 +4040,9 @@ export function createPrService({ } ]; - const group = db.get( - ` - select - g.id as group_id, - g.group_type as group_type - from pr_group_members m - join pr_groups g on g.id = m.group_id - where g.project_id = ? and m.pr_id = ? - order by g.created_at desc - limit 1 - `, - [projectId, prId] - ); - + const group = groupByPrId.get(row.id) ?? null; const baseMergeContext: PrMergeContext = { - prId, + prId: row.id, groupId: group?.group_id ?? null, groupType: null, sourceLaneIds: [fallbackSourceLaneId], @@ -4008,34 +4055,7 @@ export function createPrService({ return baseMergeContext; } - const members = db - .all( - ` - select - m.group_id as group_id, - m.pr_id as pr_id, - m.lane_id as lane_id, - m.position as position, - m.role as role, - l.name as lane_name, - p.github_pr_number as pr_number - from pr_group_members m - left join lanes l on l.id = m.lane_id and l.project_id = ? - left join pull_requests p on p.id = m.pr_id and p.project_id = ? - where m.group_id = ? - order by m.position asc - `, - [projectId, projectId, group.group_id] - ) - .map((member) => ({ - prId: member.pr_id, - laneId: member.lane_id, - laneName: member.lane_name ?? laneById.get(member.lane_id)?.name ?? member.lane_id, - prNumber: Number.isFinite(Number(member.pr_number)) ? Number(member.pr_number) : null, - position: Number(member.position), - role: normalizeGroupMemberRole(String(member.role ?? "source")) - })); - + const members = membersByGroupId.get(group.group_id) ?? []; const groupType = group.group_type === "integration" ? "integration" : "queue"; const sourceLaneIds = members .filter((member) => member.role === "source") @@ -4053,6 +4073,165 @@ export function createPrService({ }; }; + const buildMergeContextForRow = ( + row: PullRequestRow, + lanes: LaneSummary[], + ): PrMergeContext => { + const { laneById, laneIdByBranch } = buildMergeContextLaneLookups(lanes); + const group = db.get( + ` + select + g.id as group_id, + g.group_type as group_type + from pr_group_members m + join pr_groups g on g.id = m.group_id + where g.project_id = ? and m.pr_id = ? + order by g.created_at desc + limit 1 + `, + [projectId, row.id] + ); + + const groupByPrId = new Map(); + const membersByGroupId = new Map(); + if (group) { + groupByPrId.set(row.id, group); + const members = db + .all( + ` + select + m.group_id as group_id, + m.pr_id as pr_id, + m.lane_id as lane_id, + m.position as position, + m.role as role, + l.name as lane_name, + p.github_pr_number as pr_number + from pr_group_members m + left join lanes l on l.id = m.lane_id and l.project_id = ? + left join pull_requests p on p.id = m.pr_id and p.project_id = ? + where m.group_id = ? + order by m.position asc + `, + [projectId, projectId, group.group_id] + ) + .map((member) => mapMergeContextMember(member, laneById)); + membersByGroupId.set(group.group_id, members); + } + + return buildMergeContextFromLookups(row, { + laneById, + laneIdByBranch, + groupByPrId, + membersByGroupId, + }); + }; + + const emptyMergeContext = (prId: string): PrMergeContext => ({ + prId, + groupId: null, + groupType: null, + sourceLaneIds: [], + targetLaneId: null, + integrationLaneId: null, + members: [], + }); + + const getMergeContext = async (prId: string): Promise => { + const row = getRow(prId); + if (!row) return emptyMergeContext(prId); + const lanes = await laneService.list({ includeArchived: false, includeStatus: false }); + return buildMergeContextForRow(row, lanes); + }; + + const getMergeContexts = async (prIds: string[]): Promise> => { + const uniquePrIds = [...new Set(prIds.map((id) => String(id ?? "").trim()).filter(Boolean))]; + if (uniquePrIds.length === 0) return {}; + const lanes = await laneService.list({ includeArchived: false, includeStatus: false }); + const { laneById, laneIdByBranch } = buildMergeContextLaneLookups(lanes); + const rows: PullRequestRow[] = []; + for (const chunk of chunkValues(uniquePrIds)) { + const rowPlaceholders = chunk.map(() => "?").join(", "); + rows.push(...db.all( + `select ${PR_COLUMNS} + from pull_requests + where project_id = ? and id in (${rowPlaceholders})`, + [projectId, ...chunk], + )); + } + if (rows.length === 0) return Object.fromEntries(uniquePrIds.map((prId) => [prId, emptyMergeContext(prId)])); + const groupRows: Array = []; + for (const chunk of chunkValues(uniquePrIds)) { + const rowPlaceholders = chunk.map(() => "?").join(", "); + groupRows.push(...db.all( + ` + select pr_id, group_id, group_type + from ( + select + m.pr_id as pr_id, + g.id as group_id, + g.group_type as group_type, + row_number() over (partition by m.pr_id order by g.created_at desc) as group_rank + from pr_group_members m + join pr_groups g on g.id = m.group_id + where g.project_id = ? and m.pr_id in (${rowPlaceholders}) + ) + where group_rank = 1 + `, + [projectId, ...chunk], + )); + } + const groupByPrId = new Map(); + for (const group of groupRows) { + if (!groupByPrId.has(group.pr_id)) { + groupByPrId.set(group.pr_id, group); + } + } + const groupIds = [...new Set(groupRows.map((group) => group.group_id))]; + const membersByGroupId = new Map(); + if (groupIds.length > 0) { + for (const chunk of chunkValues(groupIds)) { + const groupPlaceholders = chunk.map(() => "?").join(", "); + const memberRows = db.all( + ` + select + m.group_id as group_id, + m.pr_id as pr_id, + m.lane_id as lane_id, + m.position as position, + m.role as role, + l.name as lane_name, + p.github_pr_number as pr_number + from pr_group_members m + left join lanes l on l.id = m.lane_id and l.project_id = ? + left join pull_requests p on p.id = m.pr_id and p.project_id = ? + where m.group_id in (${groupPlaceholders}) + order by m.group_id asc, m.position asc + `, + [projectId, projectId, ...chunk], + ); + for (const member of memberRows) { + const members = membersByGroupId.get(member.group_id) ?? []; + members.push(mapMergeContextMember(member, laneById)); + membersByGroupId.set(member.group_id, members); + } + } + } + const contexts: Record = {}; + for (const row of rows) { + contexts[row.id] = buildMergeContextFromLookups(row, { + laneById, + laneIdByBranch, + groupByPrId, + membersByGroupId, + }); + } + for (const prId of uniquePrIds) { + contexts[prId] ??= emptyMergeContext(prId); + } + return contexts; + }; + const extractConflictDetail = async ( treeOid: string, filePath: string, @@ -4778,12 +4957,29 @@ export function createPrService({ return result; }; + type GithubSnapshotOptions = { force?: boolean; includeExternalClosed?: boolean }; + const GITHUB_SNAPSHOT_TTL_MS = 120_000; let cachedGithubSnapshot: GitHubPrSnapshot | null = null; let cachedGithubSnapshotAt = 0; - let githubSnapshotInFlight: Promise | null = null; + let cachedGithubSnapshotIncludesExternalClosed = false; + let githubSnapshotInFlight: { request: Promise; includeExternalClosed: boolean } | null = null; + let pendingOpenOnlySnapshot: { snapshot: GitHubPrSnapshot; capturedAt: number } | null = null; + + const publishGithubSnapshot = ( + snapshot: GitHubPrSnapshot, + includesExternalClosed: boolean, + capturedAt = Date.now(), + ): void => { + cachedGithubSnapshot = snapshot; + cachedGithubSnapshotAt = capturedAt; + cachedGithubSnapshotIncludesExternalClosed = includesExternalClosed; + if (includesExternalClosed) { + pendingOpenOnlySnapshot = null; + } + }; - const getGithubSnapshotUncached = async (): Promise => { + const getGithubSnapshotUncached = async (options: GithubSnapshotOptions = {}): Promise => { const githubStatus = await githubService.getStatus(); if (!githubStatus.tokenStored) { throw new Error("GitHub token missing. Set it in Settings to sync pull requests."); @@ -4896,11 +5092,13 @@ export function createPrService({ ); } + const includeExternalClosed = options.includeExternalClosed === true; + const externalQueryState = includeExternalClosed ? "" : "is:open "; const externalPullRequestsRaw = githubStatus.userLogin ? await fetchAllPages({ path: "/search/issues", query: { - q: `is:pr involves:${githubStatus.userLogin} archived:false -repo:${repo.owner}/${repo.name}`, + q: `is:pr ${externalQueryState}involves:${githubStatus.userLogin} archived:false -repo:${repo.owner}/${repo.name}`, sort: "updated", order: "desc", }, @@ -4919,17 +5117,48 @@ export function createPrService({ }; }; - const getGithubSnapshot = async (options?: { force?: boolean }): Promise => { + const getGithubSnapshot = async (options: GithubSnapshotOptions = {}): Promise => { const force = options?.force === true; - const startSnapshotRequest = (allowStaleOnError: boolean): Promise => { + const includeExternalClosed = options?.includeExternalClosed === true; + const cachedSnapshotSatisfiesRequest = + cachedGithubSnapshot != null + && (!includeExternalClosed || cachedGithubSnapshotIncludesExternalClosed); + const startSnapshotRequest = ( + allowStaleOnError: boolean, + requestIncludeExternalClosed = includeExternalClosed, + ): Promise => { const staleFallback = cachedGithubSnapshot; - const request = getGithubSnapshotUncached() + let inFlight!: { request: Promise; includeExternalClosed: boolean }; + const request = getGithubSnapshotUncached({ includeExternalClosed: requestIncludeExternalClosed }) .then((snapshot) => { - cachedGithubSnapshot = snapshot; - cachedGithubSnapshotAt = Date.now(); + const capturedAt = Date.now(); + const hasWiderRequestInFlight = + githubSnapshotInFlight !== null + && githubSnapshotInFlight !== inFlight + && githubSnapshotInFlight.includeExternalClosed; + const canPublishSnapshot = + githubSnapshotInFlight === inFlight + || ( + !requestIncludeExternalClosed + && cachedGithubSnapshot === null + && !cachedGithubSnapshotIncludesExternalClosed + && !hasWiderRequestInFlight + ); + if (canPublishSnapshot) { + publishGithubSnapshot(snapshot, requestIncludeExternalClosed, capturedAt); + } else if (!requestIncludeExternalClosed && cachedGithubSnapshot === null) { + pendingOpenOnlySnapshot = { snapshot, capturedAt }; + } return snapshot; }) .catch((error) => { + const isCurrentRequest = githubSnapshotInFlight === inFlight; + if (isCurrentRequest && requestIncludeExternalClosed && pendingOpenOnlySnapshot) { + if (cachedGithubSnapshot === null) { + publishGithubSnapshot(pendingOpenOnlySnapshot.snapshot, false, pendingOpenOnlySnapshot.capturedAt); + } + pendingOpenOnlySnapshot = null; + } if (allowStaleOnError && staleFallback) { logger.warn("prs.github_snapshot_refresh_failed_stale_returned", { error: error instanceof Error ? error.message : String(error), @@ -4939,26 +5168,36 @@ export function createPrService({ throw error; }) .finally(() => { - if (githubSnapshotInFlight === request) { + if (githubSnapshotInFlight === inFlight) { githubSnapshotInFlight = null; } }); - githubSnapshotInFlight = request; + inFlight = { request, includeExternalClosed: requestIncludeExternalClosed }; + githubSnapshotInFlight = inFlight; return request; }; - if (!force && cachedGithubSnapshot) { + if (!force && cachedSnapshotSatisfiesRequest) { + const cachedSnapshot = cachedGithubSnapshot; + if (!cachedSnapshot) return startSnapshotRequest(false); const ageMs = Date.now() - cachedGithubSnapshotAt; if (ageMs < GITHUB_SNAPSHOT_TTL_MS) { - return cachedGithubSnapshot; + return cachedSnapshot; } - if (!githubSnapshotInFlight) { - void startSnapshotRequest(true).catch(() => {}); + const refreshIncludeExternalClosed = includeExternalClosed || cachedGithubSnapshotIncludesExternalClosed; + if (!githubSnapshotInFlight || (refreshIncludeExternalClosed && !githubSnapshotInFlight.includeExternalClosed)) { + void startSnapshotRequest(true, refreshIncludeExternalClosed).catch(() => {}); + } else if (includeExternalClosed && githubSnapshotInFlight.includeExternalClosed) { + return githubSnapshotInFlight.request; } - return cachedGithubSnapshot; + return cachedSnapshot; } - if (!force && githubSnapshotInFlight) { - return githubSnapshotInFlight; + if ( + !force + && githubSnapshotInFlight + && (!includeExternalClosed || githubSnapshotInFlight.includeExternalClosed) + ) { + return githubSnapshotInFlight.request; } return startSnapshotRequest(false); @@ -5144,20 +5383,101 @@ export function createPrService({ } }; - const listWithConflicts = async (): Promise => { - const rows = listRows(); - const results: PrWithConflicts[] = []; - for (const row of rows) { - const summary = rowToSummary(row); - let conflictAnalysis: PrConflictAnalysis | null = null; - try { - conflictAnalysis = await getConflictAnalysis(row.id); - } catch { - // Conflict analysis may fail for archived lanes; skip gracefully. + const buildConflictAnalysesForRows = async ( + rows: PullRequestRow[], + ): Promise> => { + const results = new Map(); + if (rows.length === 0) return results; + if (!conflictService) { + for (const row of rows) { + results.set(row.id, { + prId: row.id, + laneId: row.lane_id, + riskLevel: "none", + overlapCount: 0, + conflictPredicted: false, + peerConflicts: [], + analyzedAt: nowIso(), + }); } - results.push({ ...summary, conflictAnalysis }); + return results; + } + + try { + const lanes = await laneService.list({ includeArchived: false, includeStatus: false }); + const laneById = new Map(lanes.map((lane) => [lane.id, lane] as const)); + const assessment = await conflictService.getBatchAssessment({ lanes }); + const statusByLaneId = new Map(assessment.lanes.map((status) => [status.laneId, status] as const)); + const overlapFilesByPair = new Map(); + for (const overlap of assessment.overlaps) { + const key = [overlap.laneAId, overlap.laneBId].sort().join("\0"); + overlapFilesByPair.set(key, overlap.files); + } + const riskLevels = ["none", "low", "medium", "high"] as const; + const analyzedAt = nowIso(); + + for (const row of rows) { + if (!laneById.has(row.lane_id)) { + results.set(row.id, null); + continue; + } + const status = statusByLaneId.get(row.lane_id); + if (!status) { + results.set(row.id, null); + continue; + } + const peerConflicts: PrConflictAnalysis["peerConflicts"] = assessment.matrix + .filter((entry) => entry.laneAId !== entry.laneBId) + .filter((entry) => entry.laneAId === row.lane_id || entry.laneBId === row.lane_id) + .filter((entry) => entry.overlapCount > 0 || entry.riskLevel !== "none") + .map((entry) => { + const peerId = entry.laneAId === row.lane_id ? entry.laneBId : entry.laneAId; + const overlapFiles = overlapFilesByPair.get([row.lane_id, peerId].sort().join("\0")) ?? []; + return { + peerId, + peerName: laneById.get(peerId)?.name ?? peerId, + riskLevel: entry.riskLevel, + overlapFiles, + }; + }); + const highestRisk = peerConflicts.reduce( + (max, pc) => riskLevels.indexOf(pc.riskLevel) > riskLevels.indexOf(max) ? pc.riskLevel : max, + status.status === "conflict-predicted" || status.status === "conflict-active" ? "high" : "none", + ); + results.set(row.id, { + prId: row.id, + laneId: row.lane_id, + riskLevel: highestRisk, + overlapCount: status.overlappingFileCount, + conflictPredicted: status.status === "conflict-predicted" || status.status === "conflict-active", + peerConflicts, + analyzedAt, + }); + } + return results; + } catch (error) { + logger.warn("prs.batch_conflict_analysis_failed", { + error: error instanceof Error ? error.message : String(error), + }); + return results; } - return results; + }; + + const listWithConflicts = async ( + options: { includeConflictAnalysis?: boolean } = {}, + ): Promise => { + const rows = listRows(); + if (options.includeConflictAnalysis !== true) { + return rows.map((row) => ({ + ...rowToSummary(row), + conflictAnalysis: null, + })); + } + const analyses = await buildConflictAnalysesForRows(rows); + return rows.map((row) => ({ + ...rowToSummary(row), + conflictAnalysis: analyses.get(row.id) ?? null, + })); }; const listIntegrationProposals = async (): Promise => { @@ -5787,11 +6107,7 @@ export function createPrService({ ...((args.prIds ?? []).map((prId) => String(prId ?? "").trim()).filter(Boolean)), ]; if (requestedPrIds.length > 0) { - const refreshed: PrSummary[] = []; - for (const prId of [...new Set(requestedPrIds)]) { - refreshed.push(await refreshOne(prId)); - } - return refreshed; + return await refreshPrIds(requestedPrIds); } const rows = listRows(); @@ -5805,16 +6121,7 @@ export function createPrService({ .filter((row) => hotPrIds.has(row.id)) .sort(compareBackgroundRefreshPriority); const candidates = [...hotCandidates, ...staleCandidates]; - const seenCandidateIds = new Set(); - for (const row of candidates) { - if (seenCandidateIds.has(row.id)) continue; - seenCandidateIds.add(row.id); - try { - await refreshOne(row.id); - } catch (error) { - logger.warn("prs.refresh_failed", { prId: row.id, error: error instanceof Error ? error.message : String(error) }); - } - } + await refreshRowsBestEffort(candidates); return listRows().map(rowToSummary); }, @@ -5984,11 +6291,15 @@ export function createPrService({ return await getMergeContext(prId); }, - async listWithConflicts(): Promise { - return await listWithConflicts(); + async getMergeContexts(prIds: string[]): Promise> { + return await getMergeContexts(prIds); + }, + + async listWithConflicts(options?: { includeConflictAnalysis?: boolean }): Promise { + return await listWithConflicts(options); }, - async getGithubSnapshot(options?: { force?: boolean }): Promise { + async getGithubSnapshot(options?: GithubSnapshotOptions): Promise { return await getGithubSnapshot(options); }, @@ -6053,7 +6364,7 @@ export function createPrService({ return { refreshedCount: rows.length }; }, - listSnapshots(args: { prId?: string } = {}): PullRequestSnapshotHydration[] { + listSnapshots(args: { prId?: string } = {}): PrSnapshotHydration[] { return listSnapshotRows(args); }, diff --git a/apps/desktop/src/main/services/shared/queueRebase.test.ts b/apps/desktop/src/main/services/shared/queueRebase.test.ts new file mode 100644 index 000000000..fd3993b8d --- /dev/null +++ b/apps/desktop/src/main/services/shared/queueRebase.test.ts @@ -0,0 +1,131 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { AdeDb } from "../state/kvDb"; +import { fetchQueueTargetTrackingBranches, fetchRemoteTrackingBranch } from "./queueRebase"; + +const mockGit = vi.hoisted(() => ({ + runGit: vi.fn(), + runGitOrThrow: vi.fn(), +})); + +vi.mock("../git/git", () => ({ + runGit: (...args: unknown[]) => mockGit.runGit(...args), + runGitOrThrow: (...args: unknown[]) => mockGit.runGitOrThrow(...args), +})); + +const TEST_QUEUE_TARGET_FETCH_TTL_MS = 5 * 60_000; +const CACHE_BOUND_TEST_BRANCH_COUNT = 300; + +type QueueTargetDb = Pick; + +function makeDb(branches: string[]): AdeDb { + const rows = branches.map((target_branch) => ({ target_branch })); + const all: QueueTargetDb["all"] = = Record>( + _sql: string, + ) => rows as unknown as T[]; + const db: QueueTargetDb = { + all, + }; + return { + ...db, + } as AdeDb; +} + +describe("fetchQueueTargetTrackingBranches", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.spyOn(Date, "now").mockReturnValue(1_000_000); + mockGit.runGitOrThrow.mockResolvedValue(undefined); + mockGit.runGit.mockResolvedValue({ exitCode: 0, stdout: "", stderr: "" }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("bounds the queue target fetch TTL cache while preserving recent entries", async () => { + // Exceeds the production 256-entry cache cap so old queue-target fetches are evicted. + const branches = Array.from({ length: CACHE_BOUND_TEST_BRANCH_COUNT }, (_, index) => `queue-${index}`); + + await fetchQueueTargetTrackingBranches({ + db: makeDb(branches), + projectId: "project-1", + projectRoot: "/tmp/ade", + }); + expect(mockGit.runGitOrThrow).toHaveBeenCalledTimes(300); + + mockGit.runGitOrThrow.mockClear(); + await fetchQueueTargetTrackingBranches({ + db: makeDb(["queue-0", "queue-299"]), + projectId: "project-1", + projectRoot: "/tmp/ade", + }); + + expect(mockGit.runGitOrThrow).toHaveBeenCalledTimes(1); + expect(mockGit.runGitOrThrow).toHaveBeenCalledWith( + ["fetch", "--prune", "origin", "+refs/heads/queue-0:refs/remotes/origin/queue-0"], + { cwd: "/tmp/ade", timeoutMs: 120_000 }, + ); + }); + + it("refetches queue target branches after the TTL expires", async () => { + const initialNow = 2_000_000; + vi.mocked(Date.now).mockReturnValue(initialNow); + + await fetchQueueTargetTrackingBranches({ + db: makeDb(["queue-ttl"]), + projectId: "project-ttl", + projectRoot: "/tmp/ade-ttl", + }); + expect(mockGit.runGitOrThrow).toHaveBeenCalledTimes(1); + + mockGit.runGitOrThrow.mockClear(); + vi.mocked(Date.now).mockReturnValue(initialNow + TEST_QUEUE_TARGET_FETCH_TTL_MS + 1); + + await fetchQueueTargetTrackingBranches({ + db: makeDb(["queue-ttl"]), + projectId: "project-ttl", + projectRoot: "/tmp/ade-ttl", + }); + + expect(mockGit.runGitOrThrow).toHaveBeenCalledTimes(1); + }); + + it("throttles failed queue target fetch attempts", async () => { + mockGit.runGitOrThrow + .mockRejectedValueOnce(new Error("fetch failed")) + .mockResolvedValueOnce(undefined); + mockGit.runGit.mockResolvedValueOnce({ exitCode: 1, stdout: "", stderr: "network down" }); + + await fetchQueueTargetTrackingBranches({ + db: makeDb(["queue-fail"]), + projectId: "project-fail", + projectRoot: "/tmp/ade-fail", + }); + expect(mockGit.runGitOrThrow).toHaveBeenCalledTimes(1); + + mockGit.runGitOrThrow.mockClear(); + + await fetchQueueTargetTrackingBranches({ + db: makeDb(["queue-fail"]), + projectId: "project-fail", + projectRoot: "/tmp/ade-fail", + }); + + expect(mockGit.runGitOrThrow).not.toHaveBeenCalled(); + }); + + it("reports a miss when only the broad fallback fetch succeeds", async () => { + mockGit.runGitOrThrow.mockRejectedValueOnce(new Error("branch refspec failed")); + mockGit.runGit.mockResolvedValueOnce({ exitCode: 0, stdout: "", stderr: "" }); + + await expect(fetchRemoteTrackingBranch({ + projectRoot: "/tmp/ade-fallback", + targetBranch: "queue-fallback", + })).resolves.toBe(false); + + expect(mockGit.runGit).toHaveBeenCalledWith( + ["fetch", "--prune", "origin"], + { cwd: "/tmp/ade-fallback", timeoutMs: 120_000 }, + ); + }); +}); diff --git a/apps/desktop/src/main/services/shared/queueRebase.ts b/apps/desktop/src/main/services/shared/queueRebase.ts index 00bafb7fa..2045b9edf 100644 --- a/apps/desktop/src/main/services/shared/queueRebase.ts +++ b/apps/desktop/src/main/services/shared/queueRebase.ts @@ -25,6 +25,10 @@ type QueueLandingEntryState = { state: string; }; +const QUEUE_TARGET_FETCH_TTL_MS = 5 * 60_000; +const QUEUE_TARGET_FETCH_MAX_ENTRIES = 256; +const queueTargetFetchAttemptedAt = new Map(); + export type QueueRebaseOverride = { comparisonRef: string; displayBaseBranch: string; @@ -58,6 +62,21 @@ function isQueueMemberCompleted(args: { prState: string | null; queueEntryState: return args.prState === "merged"; } +function pruneQueueTargetFetchAttempts(now: number): void { + for (const [cacheKey, attemptedAt] of queueTargetFetchAttemptedAt) { + if (now - attemptedAt >= QUEUE_TARGET_FETCH_TTL_MS) { + queueTargetFetchAttemptedAt.delete(cacheKey); + } + } + if (queueTargetFetchAttemptedAt.size <= QUEUE_TARGET_FETCH_MAX_ENTRIES) return; + const staleEntries = [...queueTargetFetchAttemptedAt.entries()] + .sort(([, left], [, right]) => left - right); + const removeCount = queueTargetFetchAttemptedAt.size - QUEUE_TARGET_FETCH_MAX_ENTRIES; + for (const [cacheKey] of staleEntries.slice(0, removeCount)) { + queueTargetFetchAttemptedAt.delete(cacheKey); + } +} + async function resolveRemoteAwareTargetRef(args: { projectRoot: string; targetBranch: string; @@ -183,11 +202,11 @@ export async function fetchRemoteTrackingBranch(args: { ); return true; } catch { - const fallback = await runGit(["fetch", "--prune", "origin"], { + await runGit(["fetch", "--prune", "origin"], { cwd: args.projectRoot, timeoutMs: 120_000, }); - return fallback.exitCode === 0; + return false; } } @@ -217,13 +236,25 @@ export async function fetchQueueTargetTrackingBranches(args: { if (branch) branches.add(branch); } + const now = Date.now(); + pruneQueueTargetFetchAttempts(now); for (const branch of branches) { - await fetchRemoteTrackingBranch({ + const cacheKey = `${args.projectRoot}\0${args.projectId}\0${branch}`; + const attemptedAt = queueTargetFetchAttemptedAt.get(cacheKey) ?? 0; + if (now - attemptedAt < QUEUE_TARGET_FETCH_TTL_MS) continue; + const attemptStartedAt = Date.now(); + queueTargetFetchAttemptedAt.set(cacheKey, attemptStartedAt); + if (queueTargetFetchAttemptedAt.size > QUEUE_TARGET_FETCH_MAX_ENTRIES) { + pruneQueueTargetFetchAttempts(attemptStartedAt); + } + const fetched = await fetchRemoteTrackingBranch({ projectRoot: args.projectRoot, targetBranch: branch, - }).catch(() => { + }).catch(() => false); + if (!fetched) { // Best-effort refresh only. Rebase scans can still proceed against the // existing local tracking ref if fetch is unavailable. - }); + continue; + } } } diff --git a/apps/desktop/src/preload/global.d.ts b/apps/desktop/src/preload/global.d.ts index 3f3cda062..db5d0b1be 100644 --- a/apps/desktop/src/preload/global.d.ts +++ b/apps/desktop/src/preload/global.d.ts @@ -367,6 +367,7 @@ import type { PrReview, PrReviewThread, PrReviewThreadComment, + PrSnapshotHydration, PrStatus, PrSummary, PrWithConflicts, @@ -2038,9 +2039,18 @@ declare global { }) => Promise; getConflictAnalysis: (prId: string) => Promise; getMergeContext: (prId: string) => Promise; - listWithConflicts: () => Promise; + getMergeContexts: ( + prIds: string[], + ) => Promise>; + listWithConflicts: (args?: { + includeConflictAnalysis?: boolean; + }) => Promise; + listSnapshots: (args?: { + prId?: string; + }) => Promise; getGitHubSnapshot: (args?: { force?: boolean; + includeExternalClosed?: boolean; }) => Promise; listIntegrationWorkflows: ( args?: ListIntegrationWorkflowsArgs, diff --git a/apps/desktop/src/preload/preload.test.ts b/apps/desktop/src/preload/preload.test.ts index 567d9a103..d0aa82fab 100644 --- a/apps/desktop/src/preload/preload.test.ts +++ b/apps/desktop/src/preload/preload.test.ts @@ -948,6 +948,60 @@ describe("preload OAuth bridge", () => { expect(invoke).not.toHaveBeenCalledWith(IPC.prsCreateFromLane, expect.anything()); }); + it("routes bulk PR merge context hydration with positional runtime args", async () => { + const binding = { + kind: "remote", + key: "remote:target-1:project-1", + targetId: "target-1", + runtimeName: "Remote", + projectId: "project-1", + rootPath: "/remote/project", + displayName: "Project", + }; + const contexts = { "pr-1": { prId: "pr-1", members: [] } }; + const invoke = vi.fn(async (channel: string, payload?: unknown) => { + if (channel === IPC.appGetWindowSession) { + return { windowId: 1, project: null, binding }; + } + if (channel === IPC.remoteRuntimeCallAction) { + const request = (payload as { request?: { domain?: string; action?: string } } | undefined)?.request; + return { ok: true, domain: request?.domain, action: request?.action, result: contexts, statusHints: {} }; + } + return undefined; + }); + const on = vi.fn(); + const removeListener = vi.fn(); + const exposeInMainWorld = vi.fn((name: string, value: unknown) => { + (globalThis as any).__bridgeName = name; + (globalThis as any).__adeBridge = value; + }); + + vi.doMock("electron", () => ({ + contextBridge: { exposeInMainWorld }, + ipcRenderer: { invoke, on, removeListener }, + webFrame: { + getZoomLevel: vi.fn(() => 0), + setZoomLevel: vi.fn(), + getZoomFactor: vi.fn(() => 1), + }, + })); + + await import("./preload"); + + const bridge = (globalThis as any).__adeBridge; + await expect(bridge.prs.getMergeContexts(["pr-1", "pr-2"])).resolves.toEqual(contexts); + expect(invoke).toHaveBeenCalledWith(IPC.remoteRuntimeCallAction, { + id: "target-1", + projectId: "project-1", + request: { + domain: "pr", + action: "getMergeContexts", + argsList: [["pr-1", "pr-2"]], + }, + }); + expect(invoke).not.toHaveBeenCalledWith(IPC.prsGetMergeContexts, expect.anything()); + }); + it("routes GitHub repo metadata through a remote project runtime when bound", async () => { const binding = { kind: "remote", diff --git a/apps/desktop/src/preload/preload.ts b/apps/desktop/src/preload/preload.ts index 7726c4ba2..44434c939 100644 --- a/apps/desktop/src/preload/preload.ts +++ b/apps/desktop/src/preload/preload.ts @@ -440,6 +440,7 @@ import type { PrConflictAnalysis, PrMergeContext, PrHealth, + PrSnapshotHydration, PrWithConflicts, RebaseNeed, RebaseLaneArgs, @@ -4405,7 +4406,7 @@ contextBridge.exposeInMainWorld("ade", { }; }, listAutoRebaseStatuses: async (): Promise => - callProjectRuntimeActionOr("lane", "listAutoRebaseStatuses", {}, () => + callProjectRuntimeActionOr("lane", "listAutoRebaseStatuses", { args: {} }, () => ipcRenderer.invoke(IPC.lanesListAutoRebaseStatuses), ), dismissAutoRebaseStatus: async (args: { @@ -6957,12 +6958,27 @@ contextBridge.exposeInMainWorld("ade", { callProjectRuntimeActionOr("pr", "getMergeContext", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetMergeContext, { prId }), ), - listWithConflicts: (): Promise => - callProjectRuntimeActionOr("pr", "listWithConflicts", {}, () => - ipcRenderer.invoke(IPC.prsListWithConflicts), + getMergeContexts: (prIds: string[]): Promise> => + callProjectRuntimeActionOr( + "pr", + "getMergeContexts", + { argsList: [prIds] }, + () => ipcRenderer.invoke(IPC.prsGetMergeContexts, { prIds }), + ), + listWithConflicts: (args: { includeConflictAnalysis?: boolean } = {}): Promise => + callProjectRuntimeActionOr("pr", "listWithConflicts", { args }, () => + ipcRenderer.invoke(IPC.prsListWithConflicts, args), + ), + listSnapshots: (args: { prId?: string } = {}): Promise => + callProjectRuntimeActionOr( + "pr", + "listSnapshots", + { args }, + () => ipcRenderer.invoke(IPC.prsListSnapshots, args), ), getGitHubSnapshot: (args?: { force?: boolean; + includeExternalClosed?: boolean; }): Promise => callProjectRuntimeActionOr( "pr", diff --git a/apps/desktop/src/renderer/browserMock.ts b/apps/desktop/src/renderer/browserMock.ts index ebaa0f2fc..ca9419f19 100644 --- a/apps/desktop/src/renderer/browserMock.ts +++ b/apps/desktop/src/renderer/browserMock.ts @@ -5265,7 +5265,23 @@ if (typeof window !== "undefined" && shouldInstallBrowserMock(window)) { integrationLaneId: null, members: [], }, + getMergeContexts: async (prIds: string[]) => + Object.fromEntries( + prIds.map((prId) => [ + prId, + MOCK_MERGE_CONTEXTS[prId] ?? { + prId, + groupId: null, + groupType: null, + sourceLaneIds: [], + targetLaneId: null, + integrationLaneId: null, + members: [], + }, + ]), + ), listWithConflicts: resolved(ALL_PRS), + listSnapshots: async () => [], getGitHubSnapshot: resolvedArg(MOCK_GITHUB_SNAPSHOT), listIntegrationWorkflows: resolved(MOCK_INTEGRATION_WORKFLOWS), aiResolutionStart: async () => ({ diff --git a/apps/desktop/src/renderer/components/automations/adeActionSchemas.ts b/apps/desktop/src/renderer/components/automations/adeActionSchemas.ts index 0e411dec2..5f2ba9148 100644 --- a/apps/desktop/src/renderer/components/automations/adeActionSchemas.ts +++ b/apps/desktop/src/renderer/components/automations/adeActionSchemas.ts @@ -642,7 +642,24 @@ export const ADE_ACTION_SCHEMAS: readonly AdeActionSchema[] = [ action: "getGithubSnapshot", label: "Get GitHub PR snapshot", description: "Return the cached list of GitHub PRs visible to the viewer.", - params: [{ name: "force", type: "boolean", description: "Force refresh from GitHub." }], + params: [ + { name: "force", type: "boolean", description: "Force refresh from GitHub." }, + { name: "includeExternalClosed", type: "boolean", description: "Include closed PRs outside the active repository." }, + ], + }, + { + domain: "pr", + action: "getMergeContexts", + label: "Get PR merge contexts", + description: "Return merge workflow context for a batch of PR ids.", + params: [{ name: "prIds", type: "string-array", description: "PR ids to hydrate." }], + }, + { + domain: "pr", + action: "listSnapshots", + label: "List PR snapshots", + description: "Return locally cached PR detail snapshots.", + params: [{ name: "prId", type: "string", description: "Optional PR id to hydrate." }], }, { domain: "pr", diff --git a/apps/desktop/src/renderer/components/lanes/LanesPage.test.ts b/apps/desktop/src/renderer/components/lanes/LanesPage.test.ts index 6010fb22b..606077677 100644 --- a/apps/desktop/src/renderer/components/lanes/LanesPage.test.ts +++ b/apps/desktop/src/renderer/components/lanes/LanesPage.test.ts @@ -1,7 +1,9 @@ import { describe, expect, it } from "vitest"; import { githubPrMatchesCurrentBranch, + laneHasAncestor, lanePrMatchesCurrentBranch, + planLaneDeleteBatches, resolveLaneDeleteStartSelection, resolveCreateLaneRequest, resolveLaneIdsDeepLinkSelection, @@ -168,6 +170,47 @@ describe("resolveLaneIdsDeepLinkSelection", () => { }); }); +describe("planLaneDeleteBatches", () => { + it("runs independent lanes in the same delete batch", () => { + const batches = planLaneDeleteBatches([ + { id: "lane-a", parentLaneId: null }, + { id: "lane-b", parentLaneId: null }, + { id: "lane-c", parentLaneId: "lane-x" }, + ]); + + expect(batches.map((batch) => batch.map((lane) => lane.id))).toEqual([["lane-a", "lane-b", "lane-c"]]); + }); + + it("deletes selected child lanes before their selected parents", () => { + const batches = planLaneDeleteBatches([ + { id: "lane-parent", parentLaneId: null }, + { id: "lane-child", parentLaneId: "lane-parent" }, + { id: "lane-grandchild", parentLaneId: "lane-child" }, + { id: "lane-sibling", parentLaneId: "lane-parent" }, + ]); + + expect(batches.map((batch) => batch.map((lane) => lane.id))).toEqual([ + ["lane-grandchild", "lane-sibling"], + ["lane-child"], + ["lane-parent"], + ]); + }); +}); + +describe("laneHasAncestor", () => { + it("walks the full lane graph without hanging on corrupted cycles", () => { + const lanes = [ + { id: "child", parentLaneId: "middle" }, + { id: "middle", parentLaneId: "child" }, + { id: "root", parentLaneId: null }, + ]; + const lanesById = new Map(lanes.map((lane) => [lane.id, lane] as const)); + + expect(laneHasAncestor("child", "middle", lanesById)).toBe(true); + expect(laneHasAncestor("child", "root", lanesById)).toBe(false); + }); +}); + describe("resolveLaneDeleteStartSelection", () => { it("moves selection and active panes away from lanes that just started deleting", () => { const result = resolveLaneDeleteStartSelection({ diff --git a/apps/desktop/src/renderer/components/lanes/LanesPage.tsx b/apps/desktop/src/renderer/components/lanes/LanesPage.tsx index af2cd1ca4..8822c6d86 100644 --- a/apps/desktop/src/renderer/components/lanes/LanesPage.tsx +++ b/apps/desktop/src/renderer/components/lanes/LanesPage.tsx @@ -28,6 +28,8 @@ import { useOnboardingStore } from "../../state/onboardingStore"; import { useDialogBus } from "../../lib/useDialogBus"; import { parseLaneIdsParam, + laneHasAncestor, + planLaneDeleteBatches, resolveCreateLaneRequest, resolveLaneDeleteStartSelection, resolveLaneIdsDeepLinkSelection, @@ -180,7 +182,7 @@ function createPendingDeleteProgress(laneId: string): LaneDeleteProgress { steps: [], startedAt: new Date().toISOString(), overallStatus: "running", - cancellable: true, + cancellable: false, }; } @@ -759,7 +761,7 @@ export function LanesPage() { pendingLaneDeleteRefreshIdsRef.current.clear(); if (laneIds.length === 0) return; - void refreshLanes() + void refreshLanes({ includeStatus: false }) .then(() => { const selectedId = selectedLaneIdRef.current; const managedIds = managedLaneIdsRef.current; @@ -1437,19 +1439,47 @@ export function LanesPage() { void (async () => { const errors: string[] = []; - for (const lane of actionable) { - try { - const args = deleteArgsByLaneId.get(lane.id); - if (!args) continue; - await window.ade.lanes.delete(args); - } catch (err) { - errors.push(`${lane.name}: ${err instanceof Error ? err.message : String(err)}`); + const blockedLaneIds = new Set(); + const hasBlockedSelectedDescendant = (laneId: string): boolean => { + for (const blockedLaneId of blockedLaneIds) { + if (laneHasAncestor(blockedLaneId, laneId, lanesById)) return true; + } + return false; + }; + + for (const batch of planLaneDeleteBatches(actionable)) { + const runnable = batch.filter((lane) => { + if (!hasBlockedSelectedDescendant(lane.id)) return true; + blockedLaneIds.add(lane.id); + errors.push(`${lane.name}: skipped because a selected child lane did not delete.`); setDeleteProgressByLaneId((prev) => { const next = { ...prev }; delete next[lane.id]; return next; }); - } + return false; + }); + if (runnable.length === 0) continue; + + const results = await Promise.allSettled( + runnable.map(async (lane) => { + const args = deleteArgsByLaneId.get(lane.id); + if (!args) return; + await window.ade.lanes.delete(args); + }), + ); + results.forEach((result, index) => { + if (result.status === "fulfilled") return; + const lane = runnable[index]; + if (!lane) return; + blockedLaneIds.add(lane.id); + errors.push(`${lane.name}: ${result.reason instanceof Error ? result.reason.message : String(result.reason)}`); + setDeleteProgressByLaneId((prev) => { + const next = { ...prev }; + delete next[lane.id]; + return next; + }); + }); } if (errors.length > 0) { setLaneActionError(errors.join("\n")); diff --git a/apps/desktop/src/renderer/components/lanes/ManageLaneDialog.tsx b/apps/desktop/src/renderer/components/lanes/ManageLaneDialog.tsx index 68d5fa7e6..23b56862b 100644 --- a/apps/desktop/src/renderer/components/lanes/ManageLaneDialog.tsx +++ b/apps/desktop/src/renderer/components/lanes/ManageLaneDialog.tsx @@ -169,15 +169,6 @@ export function ManageLaneDialog({ const confirmMatch = !requiresTypeConfirm || deleteConfirmText.trim().toLowerCase() === deletePhrase.toLowerCase(); const showStaticBusy = laneActionBusy && !deleteProgress; - const handleCancelDelete = async (): Promise => { - if (!singleLane) return; - try { - await window.ade.lanes.cancelDelete({ laneId: singleLane.id }); - } catch { - // Cancellation is best-effort — surfaced via the next progress event if honored. - } - }; - return ( - - {/* Cancel — visible only while a streaming delete is running */} - {deleteProgress?.overallStatus === "running" ? ( - - ) : null} diff --git a/apps/desktop/src/renderer/components/lanes/lanePageModel.ts b/apps/desktop/src/renderer/components/lanes/lanePageModel.ts index 58d318886..2b23e6896 100644 --- a/apps/desktop/src/renderer/components/lanes/lanePageModel.ts +++ b/apps/desktop/src/renderer/components/lanes/lanePageModel.ts @@ -61,6 +61,45 @@ export function parseLaneIdsParam(value: string | null): string[] { .filter(Boolean); } +export function planLaneDeleteBatches>(lanes: T[]): T[][] { + const lanesById = new Map(lanes.map((lane) => [lane.id, lane] as const)); + const remaining = new Set(lanesById.keys()); + const batches: T[][] = []; + + while (remaining.size > 0) { + const leafIds = Array.from(remaining).filter((laneId) => { + for (const candidateId of remaining) { + if (lanesById.get(candidateId)?.parentLaneId === laneId) return false; + } + return true; + }); + const batchIds = leafIds.length > 0 ? leafIds : [remaining.values().next().value as string]; + batches.push(batchIds.map((laneId) => lanesById.get(laneId)).filter((lane): lane is T => lane != null)); + for (const laneId of batchIds) { + remaining.delete(laneId); + } + } + + return batches; +} + +export function laneHasAncestor>( + laneId: string, + ancestorLaneId: string, + lanesById: ReadonlyMap, +): boolean { + const visited = new Set([laneId]); + let cursor = lanesById.get(laneId) ?? null; + while (cursor?.parentLaneId) { + const parentLaneId = cursor.parentLaneId; + if (parentLaneId === ancestorLaneId) return true; + if (visited.has(parentLaneId)) return false; + visited.add(parentLaneId); + cursor = lanesById.get(parentLaneId) ?? null; + } + return false; +} + export function resolveLaneIdsDeepLinkSelection(args: { laneIdsRaw: string | null; inspectorTabParam?: string | null; diff --git a/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.issueResolver.test.tsx b/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.issueResolver.test.tsx index f0c5e686f..ed265da84 100644 --- a/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.issueResolver.test.tsx +++ b/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.issueResolver.test.tsx @@ -2,7 +2,7 @@ import React from "react"; import { MemoryRouter } from "react-router-dom"; -import { cleanup, render, screen, waitFor } from "@testing-library/react"; +import { act, cleanup, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { @@ -13,9 +13,12 @@ import type { PrAiResolutionEventPayload, PrActionRun, PrCheck, + PrComment, PrConvergenceState, PrConvergenceStatePatch, + PrReview, PrReviewThread, + PrSnapshotHydration, PrStatus, PrWithConflicts, TerminalSessionStatus, @@ -69,6 +72,33 @@ function makeActionRun(overrides: Partial = {}): PrActionRun { }; } +function makeComment(overrides: Partial = {}): PrComment { + return { + id: "comment-1", + author: "reviewer", + authorAvatarUrl: null, + body: "Cached comment body", + source: "issue", + url: null, + path: "src/prs.ts", + line: 18, + createdAt: "2026-03-23T12:00:00.000Z", + updatedAt: "2026-03-23T12:00:00.000Z", + ...overrides, + }; +} + +function makeReview(overrides: Partial = {}): PrReview { + return { + reviewer: "reviewer", + reviewerAvatarUrl: null, + state: "approved", + body: "Cached review body", + submittedAt: "2026-03-23T12:05:00.000Z", + ...overrides, + }; +} + function makeThread(overrides: Partial = {}): PrReviewThread { return { id: "thread-1", @@ -262,6 +292,7 @@ function renderPane(args: { prOverrides?: Partial; onNavigate?: (path: string) => void; activity?: PrActivityEvent[]; + status?: PrStatus | null; statusOverrides?: Partial; mergeMethod?: "merge" | "squash" | "rebase"; convergenceState?: PrConvergenceState | null; @@ -271,6 +302,13 @@ function renderPane(args: { items: IssueInventoryItem[]; convergence: { currentRound: number; maxRounds: number; totalNew: number; totalSentToAgent: number; isConverging: boolean }; }; + listSnapshots?: ReturnType; + getDetail?: ReturnType; + getFiles?: ReturnType; + getCommits?: ReturnType; + snapshotHydration?: PrSnapshotHydration | null; + snapshotHydrationOwnedByContext?: boolean; + liveDetailReady?: boolean; }) { const laneList = args.lanes ?? [makeLane({ status: { @@ -413,7 +451,8 @@ function renderPane(args: { Object.assign(window, { ade: { prs: { - getDetail: vi.fn().mockResolvedValue({ + listSnapshots: args.listSnapshots, + getDetail: args.getDetail ?? vi.fn().mockResolvedValue({ prId: "pr-80", body: "This PR improves GitHub PR flows.", labels: [], @@ -424,7 +463,8 @@ function renderPane(args: { milestone: null, linkedIssues: [], }), - getFiles: vi.fn().mockResolvedValue([]), + getFiles: args.getFiles ?? vi.fn().mockResolvedValue([]), + getCommits: args.getCommits, getActionRuns, getActivity: vi.fn().mockResolvedValue(args.activity ?? []), getReviewThreads, @@ -490,10 +530,13 @@ function renderPane(args: { { }); }); + it("hydrates checks and activity data from a cached snapshot", async () => { + const user = userEvent.setup(); + const listSnapshots = vi.fn().mockResolvedValue([{ + prId: "pr-80", + detail: null, + status: makeStatus({ checksStatus: "passing", reviewStatus: "approved" }), + checks: [makeCheck({ name: "Cached snapshot check", conclusion: "success" })], + reviews: [makeReview({ body: "Cached review body" })], + comments: [makeComment({ body: "Cached comment body" })], + files: [], + commits: [], + updatedAt: "2026-03-23T12:01:00.000Z", + }]); + renderPane({ + status: null, + checks: [], + reviewThreads: [], + listSnapshots, + }); + + await waitFor(() => { + expect(listSnapshots).toHaveBeenCalledWith({ prId: "pr-80" }); + }); + + await user.click(screen.getByRole("button", { name: /ci \/ checks/i })); + await waitFor(() => { + expect(screen.getByText("Cached snapshot check")).toBeTruthy(); + }); + + await user.click(screen.getByRole("button", { name: /activity/i })); + await waitFor(() => { + expect(screen.getByText("Cached comment body")).toBeTruthy(); + expect(screen.getByText("Cached review body")).toBeTruthy(); + }); + }); + + it("uses context-owned snapshot hydration without a duplicate snapshot IPC", async () => { + const user = userEvent.setup(); + const snapshot: PrSnapshotHydration = { + prId: "pr-80", + detail: null, + status: makeStatus({ checksStatus: "passing", reviewStatus: "approved" }), + checks: [makeCheck({ name: "Context snapshot check", conclusion: "success" })], + reviews: [makeReview({ body: "Context review body" })], + comments: [makeComment({ body: "Context comment body" })], + files: [], + commits: [], + updatedAt: "2026-03-23T12:01:00.000Z", + }; + const listSnapshots = vi.fn().mockResolvedValue([snapshot]); + renderPane({ + status: null, + checks: [], + reviewThreads: [], + listSnapshots, + snapshotHydration: snapshot, + snapshotHydrationOwnedByContext: true, + }); + + await user.click(screen.getByRole("button", { name: /ci \/ checks/i })); + await waitFor(() => { + expect(screen.getByText("Context snapshot check")).toBeTruthy(); + }); + expect(listSnapshots).not.toHaveBeenCalled(); + }); + + it("prefers authoritative empty live detail over cached snapshot data", async () => { + const user = userEvent.setup(); + const listSnapshots = vi.fn().mockResolvedValue([{ + prId: "pr-80", + detail: null, + status: makeStatus({ checksStatus: "passing", reviewStatus: "approved" }), + checks: [makeCheck({ name: "Cached snapshot check", conclusion: "success" })], + reviews: [makeReview({ body: "Cached review body" })], + comments: [makeComment({ body: "Cached comment body" })], + files: [], + commits: [], + updatedAt: "2026-03-23T12:01:00.000Z", + }]); + renderPane({ + status: null, + checks: [], + reviewThreads: [], + listSnapshots, + liveDetailReady: true, + }); + + await waitFor(() => { + expect(listSnapshots).toHaveBeenCalledWith({ prId: "pr-80" }); + }); + + await user.click(screen.getByRole("button", { name: /ci \/ checks/i })); + expect(screen.queryByText("Cached snapshot check")).toBeNull(); + + await user.click(screen.getByRole("button", { name: /activity/i })); + expect(screen.queryByText("Cached comment body")).toBeNull(); + expect(screen.queryByText("Cached review body")).toBeNull(); + }); + it("refreshes detail-side action runs when the selected PR status changes", async () => { const user = userEvent.setup(); const { getActionRuns, rerenderPane } = renderPane({ @@ -703,6 +845,80 @@ describe("PrDetailPane issue resolver CTA", () => { }); }); + it("does not reapply stale snapshot detail during same-PR status refreshes", async () => { + const freshDetail = { + prId: "pr-80", + body: "Fresh live body", + labels: [{ name: "fresh-label", color: "22c55e", description: null }], + assignees: [], + requestedReviewers: [], + author: { login: "octocat", avatarUrl: null }, + isDraft: false, + milestone: null, + linkedIssues: [], + }; + const staleDetail = { + ...freshDetail, + body: "Stale cached body", + labels: [{ name: "stale-label", color: "ef4444", description: null }], + }; + let resolveSecondDetail!: (value: typeof freshDetail) => void; + const secondDetail = new Promise((resolve) => { + resolveSecondDetail = resolve; + }); + const getDetail = vi.fn() + .mockResolvedValueOnce(freshDetail) + .mockReturnValueOnce(secondDetail); + const listSnapshots = vi.fn() + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ + prId: "pr-80", + detail: staleDetail, + status: null, + checks: [], + reviews: [], + comments: [], + files: [], + commits: [], + updatedAt: "2026-03-23T12:01:00.000Z", + }]); + const { rerenderPane } = renderPane({ + checks: [], + reviewThreads: [], + getDetail, + listSnapshots, + prOverrides: { + checksStatus: "none", + updatedAt: "2026-03-23T12:00:00.000Z", + }, + }); + + await waitFor(() => { + expect(screen.getByText("fresh-label")).toBeTruthy(); + }); + + rerenderPane({ + checksStatus: "pending", + updatedAt: "2026-03-23T12:01:00.000Z", + }); + + await waitFor(() => { + expect(getDetail).toHaveBeenCalledTimes(2); + }); + await act(async () => { + await Promise.resolve(); + }); + + expect(listSnapshots).toHaveBeenCalledTimes(1); + expect(screen.getByText("fresh-label")).toBeTruthy(); + expect(screen.queryByText("stale-label")).toBeNull(); + + await act(async () => { + resolveSecondDetail(freshDetail); + await secondDetail; + }); + }); + it("refreshes Path to Merge action runs during inventory sync", async () => { const user = userEvent.setup(); const { getActionRuns, issueInventorySync } = renderPane({ diff --git a/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx b/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx index 750dc2cec..8188584cf 100644 --- a/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx +++ b/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx @@ -18,6 +18,7 @@ import type { PipelineSettings, PrConvergenceState, PrConvergenceStatePatch, + PrSnapshotHydration, } from "../../../../shared/types"; import { DEFAULT_PR_TIMELINE_FILTERS, type PrTimelineFilters } from "../shared/PrTimeline"; import type { PaletteKind } from "../shared/PrCommandPalettes"; @@ -420,6 +421,9 @@ type PrDetailPaneProps = { checks: PrCheck[]; reviews: PrReview[]; comments: PrComment[]; + snapshotHydration?: PrSnapshotHydration | null; + snapshotHydrationOwnedByContext?: boolean; + liveDetailReady?: boolean; detailBusy: boolean; lanes: LaneSummary[]; mergeMethod: MergeMethod; @@ -435,10 +439,13 @@ type PrDetailPaneProps = { export function PrDetailPane({ pr, - status, - checks, - reviews, - comments, + status: liveStatus, + checks: liveChecks, + reviews: liveReviews, + comments: liveComments, + snapshotHydration = null, + snapshotHydrationOwnedByContext = false, + liveDetailReady = false, detailBusy, lanes, mergeMethod, @@ -480,10 +487,23 @@ export function PrDetailPane({ const [detail, setDetail] = React.useState(null); const [files, setFiles] = React.useState([]); const [commits, setCommits] = React.useState([]); + const [snapshotStatus, setSnapshotStatus] = React.useState(null); + const [snapshotChecks, setSnapshotChecks] = React.useState([]); + const [snapshotReviews, setSnapshotReviews] = React.useState([]); + const [snapshotComments, setSnapshotComments] = React.useState([]); const [actionRuns, setActionRuns] = React.useState([]); const [activity, setActivity] = React.useState([]); const [reviewThreads, setReviewThreads] = React.useState([]); const timelineRailsRef = React.useRef(null); + const hasSnapshotDetail = + snapshotStatus !== null + || snapshotChecks.length > 0 + || snapshotReviews.length > 0 + || snapshotComments.length > 0; + const status = liveDetailReady ? liveStatus : (hasSnapshotDetail ? snapshotStatus : liveStatus); + const checks = liveDetailReady ? liveChecks : (hasSnapshotDetail ? snapshotChecks : liveChecks); + const reviews = liveDetailReady ? liveReviews : (hasSnapshotDetail ? snapshotReviews : liveReviews); + const comments = liveDetailReady ? liveComments : (hasSnapshotDetail ? snapshotComments : liveComments); const setActiveTab = React.useCallback((tab: DetailTab) => { setActiveTabState(tab); @@ -494,6 +514,10 @@ export function PrDetailPane({ React.useEffect(() => { const next = initialDetailTab ?? readStoredDetailTab(pr.id) ?? "overview"; setActiveTabState(next); + setSnapshotStatus(null); + setSnapshotChecks([]); + setSnapshotReviews([]); + setSnapshotComments([]); if (initialDetailTab) { writeStoredDetailTab(pr.id, initialDetailTab); } @@ -743,10 +767,37 @@ export function PrDetailPane({ const detailLoadSeqRef = React.useRef(0); const detailStatusRefreshKeyRef = React.useRef(null); const inventoryLoadSeqRef = React.useRef(0); + const snapshotHydrationRef = React.useRef(snapshotHydration); + + const applySnapshotHydration = React.useCallback((cachedSnapshot: PrSnapshotHydration) => { + if (cachedSnapshot.detail) setDetail(cachedSnapshot.detail); + if (cachedSnapshot.status) setSnapshotStatus(cachedSnapshot.status); + if (cachedSnapshot.checks.length > 0) setSnapshotChecks(cachedSnapshot.checks); + if (cachedSnapshot.reviews.length > 0) setSnapshotReviews(cachedSnapshot.reviews); + if (cachedSnapshot.comments.length > 0) setSnapshotComments(cachedSnapshot.comments); + if (cachedSnapshot.files.length > 0) setFiles(cachedSnapshot.files); + if (cachedSnapshot.commits.length > 0) setCommits(cachedSnapshot.commits); + }, []); + + React.useEffect(() => { + snapshotHydrationRef.current = snapshotHydration; + if (snapshotHydration?.prId === pr.id) { + applySnapshotHydration(snapshotHydration); + } + }, [applySnapshotHydration, pr.id, snapshotHydration]); - const loadDetail = React.useCallback(async () => { + const loadDetail = React.useCallback(async (options: { hydrateSnapshot?: boolean } = {}) => { const requestId = ++detailLoadSeqRef.current; try { + if (options.hydrateSnapshot) { + const contextSnapshot = snapshotHydrationRef.current?.prId === pr.id ? snapshotHydrationRef.current : null; + const cachedSnapshot = contextSnapshot ?? (!snapshotHydrationOwnedByContext && typeof window.ade.prs.listSnapshots === "function" + ? (await window.ade.prs.listSnapshots({ prId: pr.id }).catch(() => []))[0] + : null); + if (cachedSnapshot && requestId === detailLoadSeqRef.current) { + applySnapshotHydration(cachedSnapshot); + } + } const [d, f, c, a, act] = await Promise.all([ window.ade.prs.getDetail(pr.id).catch(() => null), window.ade.prs.getFiles(pr.id).catch(() => []), @@ -763,7 +814,7 @@ export function PrDetailPane({ } catch { // silently fail - basic data still available from context } - }, [pr.id]); + }, [applySnapshotHydration, pr.id, snapshotHydrationOwnedByContext]); const refreshReviewThreads = React.useCallback(async () => { const requestId = detailLoadSeqRef.current; @@ -820,7 +871,7 @@ export function PrDetailPane({ } }); - void loadDetail(); + void loadDetail({ hydrateSnapshot: true }); return () => { detailLoadSeqRef.current += 1; inventoryLoadSeqRef.current += 1; diff --git a/apps/desktop/src/renderer/components/prs/state/PrsContext.test.tsx b/apps/desktop/src/renderer/components/prs/state/PrsContext.test.tsx index 4128d1cf4..90b913fce 100644 --- a/apps/desktop/src/renderer/components/prs/state/PrsContext.test.tsx +++ b/apps/desktop/src/renderer/components/prs/state/PrsContext.test.tsx @@ -1,10 +1,10 @@ // @vitest-environment jsdom import React from "react"; -import { cleanup, render, screen, waitFor } from "@testing-library/react"; +import { act, cleanup, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import type { AutoRebaseLaneStatus, PrConvergenceState, PrConvergenceStatePatch, PrWithConflicts, RebaseNeed } from "../../../../shared/types"; +import type { AutoRebaseLaneStatus, PrConvergenceState, PrConvergenceStatePatch, PrSnapshotHydration, PrWithConflicts, RebaseNeed } from "../../../../shared/types"; import { PrsProvider, usePrs } from "./PrsContext"; const originalAde = globalThis.window.ade; @@ -48,6 +48,46 @@ function TabSwitchHarness() { ); } +function DetailHarness() { + const { + detailBusy, + detailChecks, + detailReviews, + detailComments, + detailStatus, + loading, + selectedPrId, + setSelectedPrId, + } = usePrs(); + return ( +
+ + +
{loading ? "loading" : "idle"}
+
{detailBusy ? "busy" : "idle"}
+
{selectedPrId ?? ""}
+
{detailStatus?.state ?? ""}
+
{detailChecks.length}
+
{detailReviews.length}
+
{detailComments.length}
+
+ ); +} + +function MergeContextHarness() { + const { loading, mergeContextByPrId } = usePrs(); + return ( +
+
{loading ? "loading" : "idle"}
+
{Object.keys(mergeContextByPrId).sort().join(",")}
+
+ ); +} + describe("PrsContext refresh", () => { beforeEach(() => { window.history.replaceState(null, "", "/"); @@ -119,6 +159,10 @@ describe("PrsContext refresh", () => { it("refreshes rebase needs and auto-rebase statuses for workflow routes without waiting for events", async () => { window.location.hash = "#/prs?tab=workflows&workflow=rebase&laneId=lane-1"; + let resolveRefresh!: () => void; + vi.mocked(window.ade.prs.refresh).mockReturnValueOnce(new Promise((resolve) => { + resolveRefresh = () => resolve([]); + })); render( @@ -134,8 +178,17 @@ describe("PrsContext refresh", () => { expect(screen.getByTestId("needs-count").textContent).toBe("1"); expect(screen.getByTestId("auto-count").textContent).toBe("1"); }); - expect(window.ade.lanes.list).toHaveBeenCalledWith({ includeStatus: true }); + expect(window.ade.lanes.list).toHaveBeenCalledWith({ includeStatus: false }); expect(window.ade.prs.refresh).toHaveBeenCalledTimes(1); + const initialRebaseScanCount = vi.mocked(window.ade.rebase.scanNeeds).mock.calls.length; + const initialAutoStatusCount = vi.mocked(window.ade.lanes.listAutoRebaseStatuses).mock.calls.length; + expect(initialRebaseScanCount).toBeGreaterThanOrEqual(1); + expect(initialAutoStatusCount).toBeGreaterThanOrEqual(1); + resolveRefresh(); + await waitFor(() => { + expect(vi.mocked(window.ade.rebase.scanNeeds).mock.calls.length).toBeGreaterThan(initialRebaseScanCount); + expect(vi.mocked(window.ade.lanes.listAutoRebaseStatuses).mock.calls.length).toBeGreaterThan(initialAutoStatusCount); + }); }); it("runs a GitHub PR refresh for explicit refresh actions", async () => { @@ -181,6 +234,352 @@ describe("PrsContext refresh", () => { expect(window.ade.prs.refresh).not.toHaveBeenCalled(); }); + it("clears stale selected PR detail arrays when a cached snapshot has empty arrays", async () => { + const user = userEvent.setup(); + vi.mocked(window.ade.prs.listWithConflicts).mockResolvedValue([makeFakePr("pr-1"), makeFakePr("pr-2")]); + Object.assign(window.ade.prs, { + listSnapshots: vi.fn(async ({ prId }: { prId: string }) => [ + prId === "pr-1" + ? { + prId, + detail: null, + status: { state: "success" }, + checks: [ + { + name: "ci", + status: "completed", + conclusion: "success", + detailsUrl: null, + startedAt: null, + completedAt: null, + }, + ], + reviews: [ + { + reviewer: "octocat", + reviewerAvatarUrl: null, + state: "approved", + body: null, + submittedAt: null, + }, + ], + comments: [ + { + id: "comment-1", + author: "octocat", + authorAvatarUrl: null, + body: "looks good", + source: "issue", + url: null, + path: null, + line: null, + createdAt: null, + }, + ], + files: [], + commits: [], + updatedAt: "2026-03-24T12:00:00.000Z", + } + : { + prId, + detail: null, + status: null, + checks: [], + reviews: [], + comments: [], + files: [], + commits: [], + updatedAt: "2026-03-24T12:05:00.000Z", + }, + ]), + getStatus: vi.fn((_prId: string) => new Promise(() => {})), + getChecks: vi.fn((_prId: string) => new Promise(() => {})), + getReviews: vi.fn((_prId: string) => new Promise(() => {})), + getComments: vi.fn((_prId: string) => new Promise(() => {})), + }); + + render( + + + , + ); + + await waitFor(() => { + expect(screen.getByTestId("loading").textContent).toBe("idle"); + }); + + await user.click(screen.getByRole("button", { name: "select pr-1" })); + await waitFor(() => { + expect(screen.getByTestId("checks-count").textContent).toBe("1"); + expect(screen.getByTestId("reviews-count").textContent).toBe("1"); + expect(screen.getByTestId("comments-count").textContent).toBe("1"); + expect(screen.getByTestId("status").textContent).toBe("success"); + }); + + await user.click(screen.getByRole("button", { name: "select pr-2" })); + await waitFor(() => { + expect(screen.getByTestId("selected-pr-id").textContent).toBe("pr-2"); + expect(screen.getByTestId("checks-count").textContent).toBe("0"); + expect(screen.getByTestId("reviews-count").textContent).toBe("0"); + expect(screen.getByTestId("comments-count").textContent).toBe("0"); + expect(screen.getByTestId("status").textContent).toBe(""); + }); + }); + + it("clears stale selected PR detail when snapshot cache misses and live detail is pending", async () => { + const user = userEvent.setup(); + vi.mocked(window.ade.prs.listWithConflicts).mockResolvedValue([makeFakePr("pr-1"), makeFakePr("pr-2")]); + Object.assign(window.ade.prs, { + listSnapshots: vi.fn(async ({ prId }: { prId: string }) => prId === "pr-1" + ? [ + { + prId, + detail: null, + status: { state: "success" }, + checks: [ + { + name: "ci", + status: "completed", + conclusion: "success", + detailsUrl: null, + startedAt: null, + completedAt: null, + }, + ], + reviews: [ + { + reviewer: "octocat", + reviewerAvatarUrl: null, + state: "approved", + body: null, + submittedAt: null, + }, + ], + comments: [ + { + id: "comment-1", + author: "octocat", + authorAvatarUrl: null, + body: "looks good", + source: "issue", + url: null, + path: null, + line: null, + createdAt: null, + }, + ], + files: [], + commits: [], + updatedAt: "2026-03-24T12:00:00.000Z", + }, + ] + : []), + getStatus: vi.fn((_prId: string) => new Promise(() => {})), + getChecks: vi.fn((_prId: string) => new Promise(() => {})), + getReviews: vi.fn((_prId: string) => new Promise(() => {})), + getComments: vi.fn((_prId: string) => new Promise(() => {})), + }); + + render( + + + , + ); + + await waitFor(() => { + expect(screen.getByTestId("loading").textContent).toBe("idle"); + }); + + await user.click(screen.getByRole("button", { name: "select pr-1" })); + await waitFor(() => { + expect(screen.getByTestId("checks-count").textContent).toBe("1"); + expect(screen.getByTestId("reviews-count").textContent).toBe("1"); + expect(screen.getByTestId("comments-count").textContent).toBe("1"); + expect(screen.getByTestId("status").textContent).toBe("success"); + }); + + await user.click(screen.getByRole("button", { name: "select pr-2" })); + await waitFor(() => { + expect(screen.getByTestId("selected-pr-id").textContent).toBe("pr-2"); + expect(screen.getByTestId("checks-count").textContent).toBe("0"); + expect(screen.getByTestId("reviews-count").textContent).toBe("0"); + expect(screen.getByTestId("comments-count").textContent).toBe("0"); + expect(screen.getByTestId("status").textContent).toBe(""); + }); + }); + + it("marks detail idle when snapshot prefill is available while live detail waits", async () => { + const user = userEvent.setup(); + vi.mocked(window.ade.prs.listWithConflicts).mockResolvedValue([makeFakePr("pr-1")]); + Object.assign(window.ade.prs, { + listSnapshots: vi.fn(async ({ prId }: { prId: string }) => [ + { + prId, + detail: null, + status: { + prId, + state: "open", + checksStatus: "passing", + reviewStatus: "approved", + isMergeable: true, + mergeConflicts: false, + behindBaseBy: 0, + }, + checks: [ + { + name: "cached-ci", + status: "completed", + conclusion: "success", + detailsUrl: null, + startedAt: null, + completedAt: null, + }, + ], + reviews: [], + comments: [], + files: [], + commits: [], + updatedAt: "2026-03-24T12:00:00.000Z", + }, + ]), + getStatus: vi.fn((_prId: string) => new Promise(() => {})), + getChecks: vi.fn((_prId: string) => new Promise(() => {})), + getReviews: vi.fn((_prId: string) => new Promise(() => {})), + getComments: vi.fn((_prId: string) => new Promise(() => {})), + }); + + render( + + + , + ); + + await waitFor(() => { + expect(screen.getByTestId("loading").textContent).toBe("idle"); + }); + + await user.click(screen.getByRole("button", { name: "select pr-1" })); + await waitFor(() => { + expect(screen.getByTestId("checks-count").textContent).toBe("1"); + expect(screen.getByTestId("detail-busy").textContent).toBe("idle"); + }); + }); + + it("does not let cached snapshot hydration overwrite live detail data", async () => { + const user = userEvent.setup(); + vi.mocked(window.ade.prs.listWithConflicts).mockResolvedValue([makeFakePr("pr-1")]); + let resolveSnapshots!: (snapshots: PrSnapshotHydration[]) => void; + const snapshotsPromise = new Promise((resolve) => { + resolveSnapshots = resolve; + }); + Object.assign(window.ade.prs, { + listSnapshots: vi.fn((_args: { prId: string }) => snapshotsPromise), + getStatus: vi.fn(async (_prId: string) => ({ state: "open" })), + getChecks: vi.fn(async (_prId: string) => []), + getReviews: vi.fn(async (_prId: string) => []), + getComments: vi.fn(async (_prId: string) => []), + }); + + render( + + + , + ); + + await waitFor(() => { + expect(screen.getByTestId("loading").textContent).toBe("idle"); + }); + + await user.click(screen.getByRole("button", { name: "select pr-1" })); + await waitFor(() => { + expect(screen.getByTestId("status").textContent).toBe("open"); + expect(screen.getByTestId("checks-count").textContent).toBe("0"); + }); + + await act(async () => { + resolveSnapshots([ + { + prId: "pr-1", + detail: null, + status: { + prId: "pr-1", + state: "closed", + checksStatus: "failing", + reviewStatus: "changes_requested", + isMergeable: false, + mergeConflicts: false, + behindBaseBy: 0, + }, + checks: [ + { + name: "stale-ci", + status: "completed", + conclusion: "failure", + detailsUrl: null, + startedAt: null, + completedAt: null, + }, + ], + reviews: [], + comments: [], + files: [], + commits: [], + updatedAt: "2026-03-24T12:10:00.000Z", + }, + ]); + await snapshotsPromise; + }); + + expect(screen.getByTestId("status").textContent).toBe("open"); + expect(screen.getByTestId("checks-count").textContent).toBe("0"); + }); + + it("falls back to single merge-context hydration for IDs missing from a batch response", async () => { + vi.mocked(window.ade.prs.listWithConflicts).mockResolvedValue([makeFakePr("pr-1"), makeFakePr("pr-2")]); + Object.assign(window.ade.prs, { + getMergeContexts: vi.fn(async (prIds: string[]) => + Object.fromEntries( + prIds + .filter((prId) => prId === "pr-1") + .map((prId) => [prId, { + prId, + groupId: null, + groupType: null, + sourceLaneIds: [`lane-${prId}`], + targetLaneId: null, + integrationLaneId: null, + members: [], + }]), + ), + ), + getMergeContext: vi.fn(async (prId: string) => ({ + prId, + groupId: null, + groupType: null, + sourceLaneIds: [`lane-${prId}`], + targetLaneId: null, + integrationLaneId: null, + members: [], + })), + }); + + render( + + + , + ); + + await waitFor(() => { + expect(screen.getByTestId("loading").textContent).toBe("idle"); + }); + await waitFor(() => { + expect(screen.getByTestId("contexts").textContent).toBe("pr-1,pr-2"); + }); + expect(window.ade.prs.getMergeContexts).toHaveBeenCalledWith(["pr-1", "pr-2"]); + expect(window.ade.prs.getMergeContext).toHaveBeenCalledTimes(1); + expect(window.ade.prs.getMergeContext).toHaveBeenCalledWith("pr-2"); + }); + it("hydrates the Rebase/Merge workflow selection from the initial hash route", async () => { window.location.hash = "#/prs?tab=workflows&workflow=rebase&laneId=lane-1"; diff --git a/apps/desktop/src/renderer/components/prs/state/PrsContext.tsx b/apps/desktop/src/renderer/components/prs/state/PrsContext.tsx index 83710bf6e..89fc313f8 100644 --- a/apps/desktop/src/renderer/components/prs/state/PrsContext.tsx +++ b/apps/desktop/src/renderer/components/prs/state/PrsContext.tsx @@ -28,6 +28,7 @@ import type { PrReviewThread, PrDeployment, PrAiSummary, + PrSnapshotHydration, PrAgentPermissionMode, } from "../../../../shared/types"; import type { PrTimelineFilters } from "../shared/PrTimeline"; @@ -68,6 +69,8 @@ type PrsState = { detailReviewThreads: PrReviewThread[]; detailDeployments: PrDeployment[]; detailAiSummary: PrAiSummary | null; + detailSnapshot: PrSnapshotHydration | null; + detailLiveDataPrId: string | null; detailBusy: boolean; // Rebase state @@ -322,6 +325,18 @@ function readInitialRouteState(fallback?: PrsContextWarmCache | null): { }; } +function currentRouteRequestsPrDiagnostics(): boolean { + try { + const route = parsePrsRouteState({ + search: window.location.search, + hash: window.location.hash, + }); + return resolvePrsActiveTab(route).isWorkflowRoute || route.prId !== null; + } catch { + return false; + } +} + function requirePrId(prId: string): string { const normalized = String(prId ?? "").trim(); if (!normalized) throw new Error("PR id is required."); @@ -400,9 +415,13 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { ); const [detailDeployments, setDetailDeployments] = useState(() => warmCache?.detailDeployments ?? []); const [detailAiSummary, setDetailAiSummary] = useState(() => warmCache?.detailAiSummary ?? null); + const [detailSnapshot, setDetailSnapshot] = useState(null); + const [detailLiveDataPrId, setDetailLiveDataPrId] = useState(null); const [detailBusy, setDetailBusy] = useState(false); const [viewerLogin, setViewerLogin] = useState(() => warmCache?.viewerLogin ?? null); const detailCacheHasDataRef = React.useRef(false); + const detailSnapshotLoadedAtByPrIdRef = React.useRef>({}); + const detailSnapshotStatePrIdRef = React.useRef(null); React.useEffect(() => { detailCacheHasDataRef.current = detailStatus !== null @@ -618,6 +637,7 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { const selectedPrIdRef = React.useRef(initialRouteState.selectedPrId); React.useEffect(() => { selectedPrIdRef.current = selectedPrId; }, [selectedPrId]); const detailFetchInProgress = React.useRef(false); + const detailStatePrIdRef = React.useRef(warmCache?.selectedPrId ?? null); const detailLoadedAtByPrIdRef = React.useRef>( warmCache?.selectedPrId ? { [warmCache.selectedPrId]: warmCache.cachedAt } : {}, ); @@ -626,16 +646,26 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { const uniquePrIds = [...new Set(prIds.map((id) => String(id ?? "").trim()).filter(Boolean))]; if (uniquePrIds.length === 0) return; const contexts: Record = {}; - await Promise.all( - uniquePrIds.map(async (prId) => { - try { - const ctx = await window.ade.prs.getMergeContext(prId); - contexts[prId] = ctx; - } catch { - /* skip failures */ - } - }), - ); + if (typeof window.ade.prs.getMergeContexts === "function") { + try { + Object.assign(contexts, await window.ade.prs.getMergeContexts(uniquePrIds)); + } catch { + /* fall back to single-context hydration below */ + } + } + const missingPrIds = uniquePrIds.filter((prId) => contexts[prId] == null); + if (missingPrIds.length > 0) { + await Promise.all( + missingPrIds.map(async (prId) => { + try { + const ctx = await window.ade.prs.getMergeContext(prId); + contexts[prId] = ctx; + } catch { + /* skip failures */ + } + }), + ); + } setMergeContextByPrId((prev) => { const allowed = new Set(prsRef.current.map((pr) => pr.id)); const next = Object.fromEntries( @@ -672,12 +702,16 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { // If a refresh is requested while one is already in flight, we set a // pending flag so that once the current flight completes it immediately // kicks off another refresh instead of silently dropping the request. - const applyLocalPrState = useCallback(async () => { + const applyLocalPrState = useCallback(async (options: { + includeWorkflowDiagnostics?: boolean; + forceRebaseDiagnostics?: boolean; + } = {}) => { const shouldLoadWorkflowState = activeTabRef.current !== "normal"; - const shouldLoadRebaseState = shouldLoadWorkflowState || selectedPrIdRef.current !== null; + const shouldLoadRebaseState = (options.includeWorkflowDiagnostics ?? true) + && (options.forceRebaseDiagnostics === true || shouldLoadWorkflowState || selectedPrIdRef.current !== null); const [prList, laneList, queueStateList, refreshedRebaseNeeds, refreshedAutoRebaseStatuses] = await Promise.all([ - window.ade.prs.listWithConflicts(), - window.ade.lanes.list({ includeStatus: shouldLoadRebaseState }), + window.ade.prs.listWithConflicts({ includeConflictAnalysis: shouldLoadWorkflowState }), + window.ade.lanes.list({ includeStatus: false }), shouldLoadWorkflowState ? window.ade.prs.listQueueStates({ includeCompleted: true, limit: 50 }) : Promise.resolve([] as QueueLandingState[]), @@ -700,8 +734,10 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { // to avoid unnecessary re-render cascades in child components. setPrs((prev) => (jsonEqual(prev, prList) ? prev : prList)); setLanes((prev) => (jsonEqual(prev, laneList) ? prev : laneList)); - setRebaseNeeds((prev) => (jsonEqual(prev, refreshedRebaseNeeds) ? prev : refreshedRebaseNeeds)); - setAutoRebaseStatuses((prev) => (jsonEqual(prev, refreshedAutoRebaseStatuses) ? prev : refreshedAutoRebaseStatuses)); + if (shouldLoadRebaseState) { + setRebaseNeeds((prev) => (jsonEqual(prev, refreshedRebaseNeeds) ? prev : refreshedRebaseNeeds)); + setAutoRebaseStatuses((prev) => (jsonEqual(prev, refreshedAutoRebaseStatuses) ? prev : refreshedAutoRebaseStatuses)); + } if (shouldLoadWorkflowState) { setQueueStates((prev) => { const next = Object.fromEntries(queueStateList.map((state) => [state.groupId, state] as const)); @@ -762,11 +798,13 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { if (options.githubRefreshMode === "await") { await window.ade.prs.refresh().catch(() => {}); } - await applyLocalPrState(); + const shouldLoadWorkflowDiagnostics = + activeTabRef.current !== "normal" || selectedPrIdRef.current !== null || currentRouteRequestsPrDiagnostics(); + await applyLocalPrState({ forceRebaseDiagnostics: shouldLoadWorkflowDiagnostics }); warmCacheHydratedAtRef.current = Date.now(); if (options.githubRefreshMode === "background") { void window.ade.prs.refresh() - .then(() => applyLocalPrState()) + .then(() => applyLocalPrState({ forceRebaseDiagnostics: shouldLoadWorkflowDiagnostics })) .then(() => { warmCacheHydratedAtRef.current = Date.now(); }) @@ -837,8 +875,12 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { } } } + if (![statusResult, checksResult, reviewsResult, commentsResult].some((result) => result.status === "fulfilled")) { + return; + } // Apply successful results; keep previous value for any that failed + detailStatePrIdRef.current = prId; if (statusResult.status === "fulfilled") { setDetailStatus((prev) => (jsonEqual(prev, statusResult.value) ? prev : statusResult.value)); } else { @@ -859,6 +901,9 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { } else { console.warn("[PrsContext] Failed to refresh PR comments:", commentsResult.reason); } + if ([statusResult, checksResult, reviewsResult, commentsResult].every((result) => result.status === "fulfilled")) { + setDetailLiveDataPrId(prId); + } detailLoadedAtByPrIdRef.current[prId] = Date.now(); }) .finally(() => { @@ -875,6 +920,7 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { rateLimitedUntilRef.current = 0; if (!selectedPrId) { + detailStatePrIdRef.current = null; setDetailStatus(null); setDetailChecks([]); setDetailReviews([]); @@ -882,6 +928,8 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { setDetailReviewThreads([]); setDetailDeployments([]); setDetailAiSummary(null); + setDetailSnapshot(null); + setDetailLiveDataPrId(null); return; } @@ -891,22 +939,66 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { // URL-derived selections are not cleared before the first refresh completes. if (!initialLoadDone.current) return; if (!prsRef.current.some((p) => p.id === selectedPrId)) { + detailStatePrIdRef.current = null; setDetailStatus(null); setDetailChecks([]); setDetailReviews([]); setDetailComments([]); + setDetailReviewThreads([]); + setDetailDeployments([]); + setDetailAiSummary(null); + setDetailSnapshot(null); + setDetailLiveDataPrId(null); setSelectedPrId(null); return; } let cancelled = false; + let liveDetailApplied = false; const prId = selectedPrId; const cachedDetailAgeMs = Date.now() - (detailLoadedAtByPrIdRef.current[prId] ?? 0); const hasFreshDetailCache = cachedDetailAgeMs >= 0 && cachedDetailAgeMs < PRS_DETAIL_CACHE_TTL_MS + && detailStatePrIdRef.current === prId + && detailCacheHasDataRef.current; + const cachedSnapshotAgeMs = Date.now() - (detailSnapshotLoadedAtByPrIdRef.current[prId] ?? 0); + const hasFreshSnapshotPrefill = + cachedSnapshotAgeMs >= 0 + && cachedSnapshotAgeMs < PRS_DETAIL_CACHE_TTL_MS + && detailSnapshotStatePrIdRef.current === prId && detailCacheHasDataRef.current; + if (!hasFreshDetailCache && !hasFreshSnapshotPrefill) { + detailStatePrIdRef.current = null; + detailSnapshotStatePrIdRef.current = null; + setDetailSnapshot(null); + setDetailLiveDataPrId(null); + setDetailStatus(null); + setDetailChecks([]); + setDetailReviews([]); + setDetailComments([]); + setDetailReviewThreads([]); + setDetailDeployments([]); + setDetailAiSummary(null); + } + if (!hasFreshDetailCache && !hasFreshSnapshotPrefill && typeof window.ade.prs.listSnapshots === "function") { + void window.ade.prs.listSnapshots({ prId }).then((snapshots) => { + if (cancelled || selectedPrIdRef.current !== prId || liveDetailApplied) return; + const snapshot = snapshots[0]; + if (!snapshot) return; + detailSnapshotStatePrIdRef.current = prId; + detailSnapshotLoadedAtByPrIdRef.current[prId] = Date.now(); + setDetailSnapshot(snapshot); + setDetailStatus(snapshot.status); + setDetailChecks(snapshot.checks); + setDetailReviews(snapshot.reviews); + setDetailComments(snapshot.comments); + setDetailBusy(false); + }).catch(() => {}); + } if (hasFreshDetailCache) { + setDetailLiveDataPrId(prId); + setDetailBusy(false); const intervalId = window.setInterval(() => { refreshDetailSilently(prId); }, 60_000); @@ -928,6 +1020,7 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { ]) .then(([statusResult, checksResult, reviewsResult, commentsResult]) => { if (cancelled) return; + liveDetailApplied = true; // Check for rate-limit errors in any rejected result for (const result of [statusResult, checksResult, reviewsResult, commentsResult]) { @@ -937,15 +1030,22 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { rateLimitedUntilRef.current = Date.now() + 5 * 60_000; console.warn("[PrsContext] GitHub rate limit hit — pausing detail polling for 5 min"); // Clear stale data on rate limit + detailStatePrIdRef.current = null; + setDetailLiveDataPrId(null); setDetailStatus(null); setDetailChecks([]); setDetailReviews([]); setDetailComments([]); + setDetailReviewThreads([]); + setDetailDeployments([]); + setDetailAiSummary(null); + setDetailSnapshot(null); return; } } } + detailStatePrIdRef.current = prId; if (statusResult.status === "fulfilled") { setDetailStatus(statusResult.value ?? null); } else { @@ -970,6 +1070,11 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { console.warn("[PrsContext] Failed to load PR comments:", commentsResult.reason); setDetailComments([]); } + if ([statusResult, checksResult, reviewsResult, commentsResult].every((result) => result.status === "fulfilled")) { + setDetailLiveDataPrId(prId); + } else { + setDetailLiveDataPrId(null); + } detailLoadedAtByPrIdRef.current[prId] = Date.now(); }) .finally(() => { @@ -1260,6 +1365,8 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { detailReviewThreads, detailDeployments, detailAiSummary, + detailSnapshot, + detailLiveDataPrId, detailBusy, rebaseNeeds, autoRebaseStatuses, @@ -1318,6 +1425,8 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { detailReviewThreads, detailDeployments, detailAiSummary, + detailSnapshot, + detailLiveDataPrId, detailBusy, rebaseNeeds, autoRebaseStatuses, diff --git a/apps/desktop/src/renderer/components/prs/state/PrsContextWarmCache.test.tsx b/apps/desktop/src/renderer/components/prs/state/PrsContextWarmCache.test.tsx new file mode 100644 index 000000000..946786726 --- /dev/null +++ b/apps/desktop/src/renderer/components/prs/state/PrsContextWarmCache.test.tsx @@ -0,0 +1,155 @@ +// @vitest-environment jsdom + +import React from "react"; +import { act, cleanup, render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { afterEach, expect, it, vi } from "vitest"; + +const originalAde = globalThis.window.ade; + +function makeFakePr(id: string) { + return { + id, + laneId: `lane-${id}`, + projectId: "proj-1", + repoOwner: "octocat", + repoName: "hello-world", + githubPrNumber: 42, + githubUrl: `https://github.com/octocat/hello-world/pull/${id}`, + githubNodeId: `node-${id}`, + title: `PR ${id}`, + state: "open", + baseBranch: "main", + headBranch: `feature/${id}`, + checksStatus: "none", + reviewStatus: "none", + additions: 1, + deletions: 1, + lastSyncedAt: null, + createdAt: "2026-03-24T12:00:00.000Z", + updatedAt: "2026-03-24T12:00:00.000Z", + conflictAnalysis: null, + creationStrategy: "manual", + }; +} + +function installAde(listSnapshots: ReturnType) { + globalThis.window.ade = { + prs: { + refresh: vi.fn().mockResolvedValue(undefined), + listWithConflicts: vi.fn().mockResolvedValue([makeFakePr("pr-1")]), + listQueueStates: vi.fn().mockResolvedValue([]), + onEvent: vi.fn(() => () => {}), + listSnapshots, + getStatus: vi.fn().mockResolvedValue({ state: "open" }), + getChecks: vi.fn().mockResolvedValue([]), + getReviews: vi.fn().mockResolvedValue([]), + getComments: vi.fn().mockResolvedValue([]), + getReviewThreads: vi.fn().mockResolvedValue([]), + getDeployments: vi.fn().mockResolvedValue([]), + getAiSummary: vi.fn().mockResolvedValue(null), + getMergeContext: vi.fn().mockResolvedValue({ + prId: "pr-1", + groupId: null, + groupType: null, + sourceLaneIds: ["lane-pr-1"], + targetLaneId: null, + integrationLaneId: null, + members: [], + }), + }, + lanes: { + list: vi.fn().mockResolvedValue([]), + listAutoRebaseStatuses: vi.fn().mockResolvedValue([]), + onAutoRebaseEvent: vi.fn(() => () => {}), + }, + rebase: { + scanNeeds: vi.fn().mockResolvedValue([]), + onEvent: vi.fn(() => () => {}), + }, + } as any; +} + +afterEach(() => { + cleanup(); + vi.unstubAllEnvs(); + globalThis.window.ade = originalAde; + window.location.hash = ""; + window.history.replaceState(null, "", "/"); +}); + +it("does not let snapshot prefill overwrite a warm detail cache", async () => { + vi.stubEnv("MODE", "production"); + window.history.replaceState(null, "", "/"); + window.location.hash = ""; + + const { PrsProvider, usePrs } = await import("./PrsContext"); + + function DetailHarness() { + const { detailStatus, loading, selectedPrId, setSelectedPrId } = usePrs(); + return ( +
+ +
{loading ? "loading" : "idle"}
+
{selectedPrId ?? ""}
+
{detailStatus?.state ?? ""}
+
+ ); + } + + const coldSnapshots = vi.fn().mockResolvedValue([]); + installAde(coldSnapshots); + + const firstRender = render( + + + , + ); + + await waitFor(() => { + expect(screen.getByTestId("loading").textContent).toBe("idle"); + }); + await userEvent.click(screen.getByRole("button", { name: "select pr-1" })); + await waitFor(() => { + expect(screen.getByTestId("selected-pr-id").textContent).toBe("pr-1"); + expect(screen.getByTestId("status").textContent).toBe("open"); + }); + await act(async () => { + await Promise.resolve(); + }); + firstRender.unmount(); + + const staleSnapshots = vi.fn().mockResolvedValue([ + { + prId: "pr-1", + detail: null, + status: { state: "closed" }, + checks: [], + reviews: [], + comments: [], + files: [], + commits: [], + updatedAt: "2026-03-24T12:05:00.000Z", + }, + ]); + installAde(staleSnapshots); + + render( + + + , + ); + + await waitFor(() => { + expect(screen.getByTestId("selected-pr-id").textContent).toBe("pr-1"); + expect(screen.getByTestId("status").textContent).toBe("open"); + }); + await act(async () => { + await Promise.resolve(); + }); + + expect(staleSnapshots).not.toHaveBeenCalled(); + expect(screen.getByTestId("status").textContent).toBe("open"); +}); diff --git a/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.test.tsx b/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.test.tsx index 759c40ab8..da59f2bf6 100644 --- a/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.test.tsx +++ b/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.test.tsx @@ -413,6 +413,111 @@ describe("GitHubTab", () => { expect(window.ade.prs.getGitHubSnapshot).toHaveBeenCalledTimes(2); }); + it("keeps loaded closed history during manual sync after returning to the open filter", async () => { + const user = userEvent.setup(); + renderTab(); + + await waitFor(() => { + expect(window.ade.prs.getGitHubSnapshot).toHaveBeenCalledWith({ force: false }); + }); + + await user.click(screen.getByRole("button", { name: /^merged/i })); + await waitFor(() => { + expect(window.ade.prs.getGitHubSnapshot).toHaveBeenCalledWith({ + force: false, + includeExternalClosed: true, + }); + }); + + (window.ade.prs.getGitHubSnapshot as ReturnType).mockClear(); + await user.click(screen.getByRole("button", { name: /^open/i })); + await user.click(screen.getByRole("button", { name: /^sync$/i })); + + await waitFor(() => { + expect(window.ade.prs.getGitHubSnapshot).toHaveBeenCalledWith({ + force: true, + includeExternalClosed: true, + }); + }); + }); + + it("does not let a superseded open-only snapshot overwrite loaded closed history", async () => { + const user = userEvent.setup(); + const openOnlySnapshot: GitHubPrSnapshot = { + ...snapshot, + repoPullRequests: [makeGitHubPr()], + externalPullRequests: [], + }; + const fullHistorySnapshot: GitHubPrSnapshot = { + ...snapshot, + repoPullRequests: [ + makeGitHubPr(), + makeGitHubPr({ + id: "repo-merged", + githubPrNumber: 102, + title: "Merged PR", + state: "merged", + linkedPrId: "pr-merged", + linkedLaneId: "lane-merged", + }), + ], + }; + let rejectOpenOnly!: (error: Error) => void; + let resolveFullHistory!: (snapshot: GitHubPrSnapshot) => void; + const openOnlyRequest = new Promise((_resolve, reject) => { + rejectOpenOnly = reject; + }); + const fullHistoryRequest = new Promise((resolve) => { + resolveFullHistory = resolve; + }); + const getGitHubSnapshot = window.ade.prs.getGitHubSnapshot as ReturnType; + getGitHubSnapshot.mockReset(); + getGitHubSnapshot + .mockResolvedValueOnce(openOnlySnapshot) + .mockReturnValueOnce(openOnlyRequest) + .mockReturnValueOnce(fullHistoryRequest); + + renderTab(); + + await waitFor(() => { + expect(getGitHubSnapshot).toHaveBeenCalledWith({ force: false }); + }); + await screen.findByText("Open PR"); + + await user.click(screen.getByRole("button", { name: /^sync$/i })); + await waitFor(() => { + expect(getGitHubSnapshot).toHaveBeenCalledWith({ force: true }); + }); + await user.click(screen.getByRole("button", { name: /^merged/i })); + await waitFor(() => { + expect(getGitHubSnapshot).toHaveBeenCalledWith({ + force: false, + includeExternalClosed: true, + }); + }); + + resolveFullHistory(fullHistorySnapshot); + await screen.findByText("Merged PR"); + + rejectOpenOnly(new Error("stale open-only failed")); + await waitFor(() => { + expect(screen.getByText("Merged PR")).toBeTruthy(); + expect(screen.queryByText("stale open-only failed")).toBeNull(); + }); + + getGitHubSnapshot.mockClear(); + getGitHubSnapshot.mockResolvedValue(fullHistorySnapshot); + await user.click(screen.getByRole("button", { name: /^open/i })); + await user.click(screen.getByRole("button", { name: /^sync$/i })); + + await waitFor(() => { + expect(getGitHubSnapshot).toHaveBeenCalledWith({ + force: true, + includeExternalClosed: true, + }); + }); + }); + it("filters by ADE scope showing only linked PRs", async () => { const snapshotWithUnlinked: GitHubPrSnapshot = { ...snapshot, diff --git a/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.tsx b/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.tsx index 1a89a22e7..aadc33f00 100644 --- a/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.tsx +++ b/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.tsx @@ -36,23 +36,28 @@ type GitHubFilter = "open" | "closed" | "merged" | "all"; type ScopeFilter = "all" | "ade" | "external"; type GitHubTabWarmCache = { + projectRoot: string; snapshot: GitHubPrSnapshot | null; filter: GitHubFilter; selectedItemId: string | null; scopeFilter: ScopeFilter; searchQuery: string; + externalHistoryLoaded: boolean; cachedAt: number; }; let githubTabWarmCache: GitHubTabWarmCache | null = null; -function readGitHubTabWarmCache(): GitHubTabWarmCache | null { +function readGitHubTabWarmCache(projectRoot: string | null): GitHubTabWarmCache | null { if (GITHUB_TAB_CACHE_DISABLED) return null; + if (!projectRoot) return null; + if (githubTabWarmCache?.projectRoot !== projectRoot) return null; return githubTabWarmCache; } function writeGitHubTabWarmCache(cache: GitHubTabWarmCache): void { if (GITHUB_TAB_CACHE_DISABLED) return; + if (!cache.projectRoot) return; githubTabWarmCache = cache; } @@ -402,12 +407,15 @@ export function GitHubTab({ detailChecks, detailReviews, detailComments, + detailSnapshot, + detailLiveDataPrId, detailBusy, loading: prsContextLoading, setViewerLogin: setContextViewerLogin, } = usePrs(); + const projectRoot = useAppStore((s) => s.project?.rootPath ?? null); - const initialWarmCacheRef = React.useRef(readGitHubTabWarmCache()); + const initialWarmCacheRef = React.useRef(readGitHubTabWarmCache(projectRoot)); const [snapshot, setSnapshot] = React.useState( () => initialWarmCacheRef.current?.snapshot ?? null, ); @@ -424,6 +432,9 @@ export function GitHubTab({ const [linkingItemId, setLinkingItemId] = React.useState(null); const [syncing, setSyncing] = React.useState(false); const [searchQuery, setSearchQuery] = React.useState(() => initialWarmCacheRef.current?.searchQuery ?? ""); + const [externalHistoryLoaded, setExternalHistoryLoaded] = React.useState( + () => initialWarmCacheRef.current?.externalHistoryLoaded ?? false, + ); const lastHandledSelectedPrIdRef = React.useRef(undefined); const pendingSelectedItemIdRef = React.useRef(null); const snapshotRef = React.useRef(null); @@ -431,10 +442,16 @@ export function GitHubTab({ const lastPrFingerprintRef = React.useRef(""); const hotRefreshUntilRef = React.useRef(0); const hotRefreshTimerRef = React.useRef(null); - const inFlightSnapshotRef = React.useRef | null>(null); + const inFlightSnapshotRef = React.useRef<{ request: Promise; includeExternalClosed: boolean } | null>(null); + const loadingSnapshotRequestCountRef = React.useRef(0); const lastSnapshotLoadedAtRef = React.useRef(initialWarmCacheRef.current?.cachedAt ?? 0); + const filterRef = React.useRef(filter); + const externalHistoryLoadedRef = React.useRef(externalHistoryLoaded); + const projectRootRef = React.useRef(projectRoot); const listRef = React.useRef(null); snapshotRef.current = snapshot; + filterRef.current = filter; + externalHistoryLoadedRef.current = externalHistoryLoaded; /* Build a lookup from linkedPrId -> PrSummary for CI/review indicators */ const prsByIdMap = React.useMemo(() => { @@ -445,15 +462,36 @@ export function GitHubTab({ return map; }, [prs]); - const loadSnapshot = React.useCallback(async (options?: { force?: boolean; silent?: boolean }) => { - if (inFlightSnapshotRef.current) return inFlightSnapshotRef.current; - if (!options?.silent) { - setLoading((prev) => options?.force || snapshotRef.current == null ? true : prev); + const loadSnapshot = React.useCallback(async (options?: { + force?: boolean; + silent?: boolean; + includeExternalClosed?: boolean; + }) => { + const includeExternalClosed = options?.includeExternalClosed === true; + const inFlightSnapshot = inFlightSnapshotRef.current; + if (inFlightSnapshot && (!includeExternalClosed || inFlightSnapshot.includeExternalClosed)) { + return inFlightSnapshot.request; + } + const shouldShowLoading = !options?.silent && (options?.force === true || snapshotRef.current == null); + if (shouldShowLoading) { + loadingSnapshotRequestCountRef.current += 1; + setLoading(true); } setError(null); - const pending = window.ade.prs.getGitHubSnapshot({ force: options?.force === true }) + const requestProjectRoot = projectRootRef.current; + let pending!: Promise; + const isCurrentSnapshotRequest = () => + inFlightSnapshotRef.current?.request === pending + && inFlightSnapshotRef.current.includeExternalClosed === includeExternalClosed; + pending = window.ade.prs.getGitHubSnapshot({ + force: options?.force === true, + ...(includeExternalClosed ? { includeExternalClosed: true } : {}), + }) .then((next) => { + if (projectRootRef.current !== requestProjectRoot) return next; + if (!isCurrentSnapshotRequest()) return next; setSnapshot(next); + setExternalHistoryLoaded((prev) => prev || includeExternalClosed); lastSnapshotLoadedAtRef.current = Date.now(); if (next.viewerLogin) { setContextViewerLogin?.(next.viewerLogin); @@ -461,19 +499,47 @@ export function GitHubTab({ return next; }) .catch((err) => { - setError(err instanceof Error ? err.message : String(err)); + if (projectRootRef.current === requestProjectRoot && isCurrentSnapshotRequest()) { + setError(err instanceof Error ? err.message : String(err)); + } return snapshotRef.current as GitHubPrSnapshot; }) .finally(() => { - inFlightSnapshotRef.current = null; - if (!options?.silent) { - setLoading(false); + if (isCurrentSnapshotRequest()) { + inFlightSnapshotRef.current = null; + } + if (shouldShowLoading) { + loadingSnapshotRequestCountRef.current = Math.max(0, loadingSnapshotRequestCountRef.current - 1); + if (loadingSnapshotRequestCountRef.current === 0 && projectRootRef.current === requestProjectRoot) { + setLoading(false); + } } }); - inFlightSnapshotRef.current = pending; + inFlightSnapshotRef.current = { request: pending, includeExternalClosed }; return pending; }, [setContextViewerLogin]); + React.useEffect(() => { + if (projectRootRef.current === projectRoot) return; + projectRootRef.current = projectRoot; + inFlightSnapshotRef.current = null; + loadingSnapshotRequestCountRef.current = 0; + const warmCache = readGitHubTabWarmCache(projectRoot); + initialWarmCacheRef.current = warmCache; + setSnapshot(warmCache?.snapshot ?? null); + setLoading(!warmCache?.snapshot); + setError(null); + setFilter(warmCache?.filter ?? "open"); + setSelectedItemId(warmCache?.selectedItemId ?? null); + setScopeFilter(warmCache?.scopeFilter ?? "all"); + setSearchQuery(warmCache?.searchQuery ?? ""); + setExternalHistoryLoaded(warmCache?.externalHistoryLoaded ?? false); + lastSnapshotLoadedAtRef.current = warmCache?.cachedAt ?? 0; + if (!warmCache?.snapshot) { + void loadSnapshot({ silent: false }); + } + }, [loadSnapshot, projectRoot]); + React.useEffect(() => { if (snapshot?.viewerLogin) { setContextViewerLogin?.(snapshot.viewerLogin); @@ -486,7 +552,13 @@ export function GitHubTab({ hotRefreshTimerRef.current = window.setTimeout(() => { hotRefreshTimerRef.current = null; hotRefreshUntilRef.current = 0; - void loadSnapshot({ force: true, silent: true }); + const includeExternalClosed = + externalHistoryLoadedRef.current || filterRef.current !== "open"; + void loadSnapshot({ + force: true, + silent: true, + ...(includeExternalClosed ? { includeExternalClosed: true } : {}), + }); }, GITHUB_TAB_HOT_REFRESH_DELAY_MS); }, [loadSnapshot]); @@ -508,15 +580,26 @@ export function GitHubTab({ }, [loadSnapshot]); React.useEffect(() => { + if (!projectRoot) return; writeGitHubTabWarmCache({ + projectRoot, snapshot, filter, selectedItemId, scopeFilter, searchQuery, + externalHistoryLoaded, cachedAt: Date.now(), }); - }, [filter, scopeFilter, searchQuery, selectedItemId, snapshot]); + }, [externalHistoryLoaded, filter, projectRoot, scopeFilter, searchQuery, selectedItemId, snapshot]); + + React.useEffect(() => { + if (filter === "open" || externalHistoryLoaded) return; + void loadSnapshot({ + includeExternalClosed: true, + silent: snapshotRef.current != null, + }); + }, [externalHistoryLoaded, filter, loadSnapshot]); React.useEffect(() => { if (prsContextLoading && prs.length === 0) return; @@ -538,7 +621,13 @@ export function GitHubTab({ lastPrFingerprintRef.current = nextFingerprint; if (Date.now() - lastSnapshotLoadedAtRef.current < GITHUB_TAB_SNAPSHOT_FRESH_MS) return; startHotRefreshWindow(); - void loadSnapshot({ force: true, silent: true }); + const includeExternalClosed = + externalHistoryLoadedRef.current || filterRef.current !== "open"; + void loadSnapshot({ + force: true, + silent: true, + ...(includeExternalClosed ? { includeExternalClosed: true } : {}), + }); }, [loadSnapshot, prs, prsContextLoading, startHotRefreshWindow]); const matchesSearch = React.useCallback((item: GitHubPrListItem) => { @@ -653,9 +742,14 @@ export function GitHubTab({ setSyncing(true); startHotRefreshWindow(); try { + const includeExternalClosed = + externalHistoryLoadedRef.current || filterRef.current !== "open"; await Promise.all([ onRefreshAll().catch(() => {}), - loadSnapshot({ force: true }), + loadSnapshot({ + force: true, + ...(includeExternalClosed ? { includeExternalClosed: true } : {}), + }), ]); } finally { setSyncing(false); @@ -958,6 +1052,9 @@ export function GitHubTab({ checks={detailChecks} reviews={detailReviews} comments={detailComments} + snapshotHydration={detailSnapshot?.prId === selectedLinkedPr.id ? detailSnapshot : null} + snapshotHydrationOwnedByContext + liveDetailReady={detailLiveDataPrId === selectedLinkedPr.id} detailBusy={detailBusy} lanes={lanes} mergeMethod={mergeMethod} diff --git a/apps/desktop/src/renderer/components/prs/tabs/IntegrationTab.tsx b/apps/desktop/src/renderer/components/prs/tabs/IntegrationTab.tsx index 1d7aedc59..5c37a5c92 100644 --- a/apps/desktop/src/renderer/components/prs/tabs/IntegrationTab.tsx +++ b/apps/desktop/src/renderer/components/prs/tabs/IntegrationTab.tsx @@ -506,6 +506,8 @@ export function IntegrationTab({ prs, lanes, mergeContextByPrId, mergeMethod, se detailChecks, detailReviews, detailComments, + detailSnapshot, + detailLiveDataPrId, detailBusy, rebaseNeeds, autoRebaseStatuses, @@ -1994,6 +1996,9 @@ export function IntegrationTab({ prs, lanes, mergeContextByPrId, mergeMethod, se checks={detailChecks} reviews={detailReviews} comments={detailComments} + snapshotHydration={detailSnapshot?.prId === selectedPr.id ? detailSnapshot : null} + snapshotHydrationOwnedByContext + liveDetailReady={detailLiveDataPrId === selectedPr.id} detailBusy={detailBusy} lanes={lanes} mergeMethod={mergeMethod} diff --git a/apps/desktop/src/renderer/components/terminals/SessionContextMenu.tsx b/apps/desktop/src/renderer/components/terminals/SessionContextMenu.tsx index 88aa09807..7651f29de 100644 --- a/apps/desktop/src/renderer/components/terminals/SessionContextMenu.tsx +++ b/apps/desktop/src/renderer/components/terminals/SessionContextMenu.tsx @@ -86,7 +86,7 @@ export function SessionContextMenu({ } return { sourceX: x, sourceY: y, left, top }; }); - }, [menu]); + }, [menu, renaming]); if (!menu) return null; diff --git a/apps/desktop/src/renderer/components/terminals/WorkStartSurface.test.tsx b/apps/desktop/src/renderer/components/terminals/WorkStartSurface.test.tsx index 675ffd777..1e8c84fc6 100644 --- a/apps/desktop/src/renderer/components/terminals/WorkStartSurface.test.tsx +++ b/apps/desktop/src/renderer/components/terminals/WorkStartSurface.test.tsx @@ -9,7 +9,7 @@ const selectLane = vi.fn(); vi.mock("../../state/appStore", () => ({ useAppStore: (selector: (state: AppState) => T): T => - selector({ selectedLaneId: null, selectLane } as unknown as AppState), + selector({ selectedLaneId: null, selectLane, lanesLoading: false } as unknown as AppState), })); vi.mock("../chat/AgentChatPane", () => ({ diff --git a/apps/desktop/src/shared/ipc.ts b/apps/desktop/src/shared/ipc.ts index d7c941f44..d88304f8e 100644 --- a/apps/desktop/src/shared/ipc.ts +++ b/apps/desktop/src/shared/ipc.ts @@ -524,7 +524,9 @@ export const IPC = { prsLandStackEnhanced: "ade.prs.landStackEnhanced", prsGetConflictAnalysis: "ade.prs.getConflictAnalysis", prsGetMergeContext: "ade.prs.getMergeContext", + prsGetMergeContexts: "ade.prs.getMergeContexts", prsListWithConflicts: "ade.prs.listWithConflicts", + prsListSnapshots: "ade.prs.listSnapshots", prsGetGitHubSnapshot: "ade.prs.getGitHubSnapshot", prsCreateQueue: "ade.prs.createQueue", prsSimulateIntegration: "ade.prs.simulateIntegration", diff --git a/apps/desktop/src/shared/types/prs.ts b/apps/desktop/src/shared/types/prs.ts index ed14d2ea7..af7e94b08 100644 --- a/apps/desktop/src/shared/types/prs.ts +++ b/apps/desktop/src/shared/types/prs.ts @@ -144,6 +144,18 @@ export type GitHubPrSnapshot = { syncedAt: string; }; +export type PrSnapshotHydration = { + prId: string; + detail: PrDetail | null; + status: PrStatus | null; + checks: PrCheck[]; + reviews: PrReview[]; + comments: PrComment[]; + files: PrFile[]; + commits: PrCommit[]; + updatedAt: string | null; +}; + export type PrEventPayload = | { type: "prs-updated"; diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 527c208ce..57345c317 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -582,7 +582,7 @@ Enforced rules (from the stability overhaul): 2. New integrations are dormant-until-configured. 3. Feature pages stage data: cheapest (list/summary/topology) first, heavy (dashboard/settings/model metadata/overlays) on delay. 4. Never mount expensive trees eagerly — settings dialogs, advanced launcher sections unmount when closed. -5. Renderer polling is route-scoped; terminal attention only polls on terminal routes; lane panels only poll while live sessions exist. The plain PR list does not fire a GitHub refresh on mount and skips rebase-needs / auto-rebase polling until the user opens a workflow tab or selects a PR. The Lanes page reuses the `LaneSummary.autoRebaseStatus` snapshot already in the lane list instead of probing per-lane on `LaneGitActionsPane` mount; a fallback probe runs only when the snapshot is missing and after a visibility-gated 3.5 s delay. The Work top-bar sync chip refreshes on focus and on `sync-status` events instead of a 5 s interval. The chat composer's Cursor model inventory is fetched lazily — `ProviderModelSelector` calls `onOpen` on first open of the model catalog, and `AgentChatPane.refreshCursorModelInventory` is the only entry point that hits `cursor` with `activateRuntime: true`. +5. Renderer polling is route-scoped; terminal attention only polls on terminal routes; lane panels only poll while live sessions exist. The plain PR list does not fire a GitHub refresh on mount, fetches open external PRs before closed/all history, skips conflict analysis, and defers rebase-needs / auto-rebase polling until the user opens a workflow tab or selects a PR. Workflow PR views batch merge contexts and conflict analysis against metadata-only lane rows instead of running per-PR git/status work. The Lanes page reuses the `LaneSummary.autoRebaseStatus` snapshot already in the lane list instead of probing per-lane on `LaneGitActionsPane` mount; a fallback probe runs only when the snapshot is missing and after a visibility-gated 3.5 s delay. The Work top-bar sync chip refreshes on focus and on `sync-status` events instead of a 5 s interval. The chat composer's Cursor model inventory is fetched lazily — `ProviderModelSelector` calls `onOpen` on first open of the model catalog, and `AgentChatPane.refreshCursorModelInventory` is the only entry point that hits `cursor` with `activateRuntime: true`. 6. Shared caches for high-frequency calls (`sessionListCache`, GitHub fingerprint-based snapshots). 7. Memoize expensive renderer computations (`useMemo`, `React.memo`); isolate frequently-refreshing subtrees (e.g., budget footers). 8. `Promise.allSettled` over `Promise.all` for parallel startup — one failing service must not block others. diff --git a/docs/features/lanes/README.md b/docs/features/lanes/README.md index 8feda23a2..213335eb9 100644 --- a/docs/features/lanes/README.md +++ b/docs/features/lanes/README.md @@ -53,8 +53,8 @@ Desktop fallback services (`apps/desktop/src/main/services/lanes/`): | File | Responsibility | |------|---------------| -| `laneService.ts` | Lane CRUD, worktree creation/removal, status computation, stack chain traversal, rebase runs, reparent, mission role tagging, startup repair routines, and the multi-step lane teardown pipeline (`getDeleteRisk`, `delete`, `cancelDelete`) that streams `LaneDeleteProgress` events as it stops processes/PTYs/watchers, cancels auto-rebase, runs `git worktree remove` / `git branch -D` / optional `git push --delete origin`, and cleans the pack directory + DB rows. | -| `autoRebaseService.ts` | Auto-rebase worker for stacked lanes, attention state, head-change handlers. Consults `resolvePrRebaseMode` to determine whether a lane with a linked PR should auto-rebase (`pr_target` strategy) or only surface manual attention (`lane_base` strategy). | +| `laneService.ts` | Lane CRUD, worktree creation/removal, status computation, stack chain traversal, rebase runs, reparent, mission role tagging, startup repair routines, and the multi-step lane teardown pipeline (`getDeleteRisk`, `delete`, `cancelDelete`) that streams `LaneDeleteProgress` events as it stops processes/PTYs/watchers, cancels auto-rebase, runs `git worktree remove` / `git branch -D` / optional `git push --delete origin`, and cleans the pack directory + DB rows. Deletes now run to completion once started, so `cancelDelete` reports that no active delete can be cancelled. | +| `autoRebaseService.ts` | Auto-rebase worker for stacked lanes, attention state, head-change handlers. Consults `resolvePrRebaseMode` to determine whether a lane with a linked PR should auto-rebase (`pr_target` strategy) or only surface manual attention (`lane_base` strategy). `listStatuses({ includeAll: true })` returns stored statuses without recomputing lane git status for PR workflow views. | | `rebaseSuggestionService.ts` | Emits rebase suggestions when a parent lane advances, dismiss/defer lifecycle. Each suggestion may include up to 20 `RebaseTargetCommit` entries showing the behind commits the rebase would pull in. | | `laneEnvironmentService.ts` | Environment init pipeline: env files, docker services, dependencies, mount points, copy paths (Phase 5 W1) | | `laneTemplateService.ts` | Reusable lane init templates (Phase 5 W2) | @@ -69,8 +69,8 @@ Renderer components: | File | Responsibility | |------|---------------| -| `renderer/components/lanes/LanesPage.tsx` | 3-pane cockpit, tab management, dialog coordination. Each lane row in the lane list optionally renders a state-aware PR tag (`PR #N` / `DRAFT #N` / `MERGED #N` / `CLOSED #N`) when the lane's current branch matches an existing PR. The pure selectors in `lanePageModel.ts` prefer ADE-linked PR rows, then fall back to `prs.getGitHubSnapshot().repoPullRequests` so merged or externally created PRs stay visible by branch match; linked PRs route to the PR workspace, while unlinked GitHub-only matches open externally. Runtime activity refreshes use `refreshLanes({ includeStatus: false, includeSnapshots: true, ... })` so PTY/chat/process buckets update without recomputing git status. Expanding Git Actions suppresses the hidden inline duplicate pane via `shouldMountGitActionsPane` while keeping the fullscreen pane mounted. Lane delete kicks off optimistically: the page subscribes to `lanes.delete.event`, tracks per-lane `LaneDeleteProgress` in `deleteProgressByLaneId`, immediately closes the manage dialog, and excludes deleting lanes from the selectable lane id sets used by keyboard navigation (`selectableFilteredLaneIds`, `sortedSelectableLaneIds`). Lane tabs for deleting lanes render a non-interactive overlay with a spinning `CircleNotch` and a `Deleting` / `Deleted` / `Deleted with warnings` label; selection / pinning / context menu / split / git-actions surfaces are all suppressed for those rows. `resolveLaneDeleteStartSelection` (also used by tests) computes a fallback selection so the user is moved to the next available lane the moment delete starts, and a top-bar lane action chip surfaces failures, cancellations, and non-fatal cleanup warnings through `laneActionError`. | -| `renderer/components/lanes/lanePageModel.ts` | Pure lane-page selectors and URL/deletion helpers used by `LanesPage` and unit tests. Owns lane branch/PR matching, ADE-vs-GitHub PR tag precedence, deep-link lane selection, create-lane request normalization, and delete-start selection fallback. | +| `renderer/components/lanes/LanesPage.tsx` | 3-pane cockpit, tab management, dialog coordination. Each lane row in the lane list optionally renders a state-aware PR tag (`PR #N` / `DRAFT #N` / `MERGED #N` / `CLOSED #N`) when the lane's current branch matches an existing PR. The pure selectors in `lanePageModel.ts` prefer ADE-linked PR rows, then fall back to `prs.getGitHubSnapshot().repoPullRequests` so merged or externally created PRs stay visible by branch match; linked PRs route to the PR workspace, while unlinked GitHub-only matches open externally. Runtime activity refreshes use `refreshLanes({ includeStatus: false, includeSnapshots: true, ... })` so PTY/chat/process buckets update without recomputing git status. Expanding Git Actions suppresses the hidden inline duplicate pane via `shouldMountGitActionsPane` while keeping the fullscreen pane mounted. Lane delete kicks off optimistically: the page subscribes to `lanes.delete.event`, tracks per-lane `LaneDeleteProgress` in `deleteProgressByLaneId`, immediately closes the manage dialog, and excludes deleting lanes from the selectable lane id sets used by keyboard navigation (`selectableFilteredLaneIds`, `sortedSelectableLaneIds`). Batch deletes run selected child lanes before their selected parents, deleting independent lanes in parallel while blocking a parent if a selected descendant fails. Lane tabs for deleting lanes render a non-interactive overlay with a spinning `CircleNotch` and a `Deleting` / `Deleted` / `Deleted with warnings` label; selection / pinning / context menu / split / git-actions surfaces are all suppressed for those rows. `resolveLaneDeleteStartSelection` (also used by tests) computes a fallback selection so the user is moved to the next available lane the moment delete starts, and a top-bar lane action chip surfaces failures and non-fatal cleanup warnings through `laneActionError`. | +| `renderer/components/lanes/lanePageModel.ts` | Pure lane-page selectors and URL/deletion helpers used by `LanesPage` and unit tests. Owns lane branch/PR matching, ADE-vs-GitHub PR tag precedence, deep-link lane selection, create-lane request normalization, delete-start selection fallback, and parent-before-child-safe batch delete planning. | | `renderer/components/lanes/laneUtils.ts` | Pure lane list/filter helpers plus default pane trees, including the work-focused tiling tree used by parallel chat launch deep links. | | `renderer/components/lanes/laneColorPalette.ts` | Curated 12-swatch lane color palette (`LANE_COLOR_PALETTE`) plus helpers (`getLaneAccent`, `colorsInUse`, `nextAvailableColor`, `laneColorName`). The first 8 hexes form `LANE_FALLBACK_COLORS`, the legacy index-based fallback used for lanes that don't have an explicit color assigned. | | `renderer/components/lanes/LaneAccentDot.tsx` | Tiny accent dot used everywhere a lane is mentioned (lane list, tabs, PR rows, AppShell PR toasts). Resolves color via `getLaneAccent` so a lane without an explicit color falls back to a deterministic fallback hex. | @@ -88,7 +88,7 @@ Renderer components: | `renderer/components/lanes/LinearIssuePicker.tsx` | Filterable Linear issue picker rendered inside `CreateLaneDialog`. Loads project / state / assignee filters from `ade.cto.getLinearIssuePickerData` and pages issues through `ade.cto.searchLinearIssues`. Shared row + label helpers (`LinearIssueRow`, `linearPriorityLabel`, `issueProjectLabel`, `issueUpdatedLabel`, `toLaneLinearIssue`, `branchExistsForLinearIssue`) are reused by `LinearIssueBrowser` (top-bar quick view) and the chat composer's Linear context dialog. Also exports a `LinearIssueSummaryCard` used by the dialog's "currently connected" state. | | `renderer/components/lanes/LinearIssueBadge.tsx` | Compact lane-list badge that surfaces the lane's connected Linear issue (identifier + state + priority); clicking opens the issue in a new chat with the issue pre-attached as context, falling back to opening the issue in Linear when chat is unavailable. | | `renderer/components/lanes/linearBrand.tsx` | Linear brand tokens (`LINEAR_BRAND` colour palette) plus the icon family used everywhere ADE references Linear: `LinearMark`, `LinearStateIcon`, `LinearPriorityIcon`. | -| `renderer/components/lanes/ManageLaneDialog.tsx` | Unified delete / archive / adopt-attached dialog. Supports single-lane and batch (multi-select) modes, three delete scopes (`worktree`, `local_branch`, `remote_branch`), a typed confirmation phrase, remote-branch name input, dirty-state warnings, and a live multi-step progress strip wired to `lanes.delete.event` (`stop_processes` / `stop_ptys` / `stop_watchers` / `cancel_auto_rebase` / `cleanup_env` / `git_status` / `git_worktree_remove` / `git_branch_delete` / `git_remote_branch_delete` / `pack_dir_remove` / `database_cleanup`). Optional branch cleanup steps can finish as warnings, allowing lane-owned worktree/database cleanup to complete while still showing the branch cleanup error inline. The dialog calls `lanes.getDeleteRisk` on open to surface dirty state, unpushed commits, running processes / PTYs / watchers, and remote-branch existence before the user confirms; while a delete is running, the user can cancel each lane through `lanes.cancelDelete` until the irreversible filesystem step (`git_worktree_remove`) starts. | +| `renderer/components/lanes/ManageLaneDialog.tsx` | Unified delete / archive / adopt-attached dialog. Supports single-lane and batch (multi-select) modes, three delete scopes (`worktree`, `local_branch`, `remote_branch`), a typed confirmation phrase, remote-branch name input, dirty-state warnings, and a live multi-step progress strip wired to `lanes.delete.event` (`stop_processes` / `stop_ptys` / `stop_watchers` / `cancel_auto_rebase` / `cleanup_env` / `git_status` / `git_worktree_remove` / `git_branch_delete` / `git_remote_branch_delete` / `pack_dir_remove` / `database_cleanup`). Optional branch cleanup steps can finish as warnings, allowing lane-owned worktree/database cleanup to complete while still showing the branch cleanup error inline. The dialog calls `lanes.getDeleteRisk` on open to surface dirty state, unpushed commits, running processes / PTYs / watchers, and remote-branch existence before the user confirms; running deletes are shown as non-cancellable because teardown runs to completion once started. | | `renderer/components/lanes/MonacoDiffView.tsx` | Monaco diff editor used for editable working-tree views (invoked from `AdeDiffViewer`) | | `renderer/components/run/LaneRuntimeBar.tsx` | Compact lane runtime status bar (health, preview, port, proxy, oauth) | | `renderer/components/run/RunPage.tsx`, `RunNetworkPanel.tsx` | Runtime dashboards that consume lane runtime services | diff --git a/docs/features/pull-requests/README.md b/docs/features/pull-requests/README.md index 406f186bc..071878184 100644 --- a/docs/features/pull-requests/README.md +++ b/docs/features/pull-requests/README.md @@ -44,7 +44,7 @@ Service files (`apps/desktop/src/main/services/prs/`): | File | Responsibility | |------|---------------| -| `prService.ts` | PR CRUD, GitHub sync, merge context, draft descriptions, check/review/comment hydration, commit snapshots (`getCommits`), integration proposals, merge-into-existing-lane adoption, merge bypass, post-merge cleanup, standalone PR branch cleanup (`cleanupBranch`), deployment listing, review-thread reply/resolve/react mutations for the timeline, the aggregate `getMobileSnapshot` that powers the iOS PRs tab, and `listOpenPullRequests` — a paginated `/repos/{owner}/{name}/pulls?state=open` fetch returning `BranchPullRequest[]` for the lane-creation branch picker. `getForLane(laneId)` resolves through `getDisplayRowForCurrentLaneBranch`: it returns the most recently updated PR whose head branch matches the lane's current branch ref, ranked open/draft → merged → closed, so a freshly merged PR still shows in lane-scoped UI instead of disappearing the moment GitHub flips the state. | +| `prService.ts` | PR CRUD, GitHub sync, merge context, draft descriptions, check/review/comment hydration, cached detail snapshots (`listSnapshots`), commit snapshots (`getCommits`), integration proposals, merge-into-existing-lane adoption, merge bypass, post-merge cleanup, standalone PR branch cleanup (`cleanupBranch`), deployment listing, review-thread reply/resolve/react mutations for the timeline, the aggregate `getMobileSnapshot` that powers the iOS PRs tab, and `listOpenPullRequests` — a paginated `/repos/{owner}/{name}/pulls?state=open` fetch returning `BranchPullRequest[]` for the lane-creation branch picker. `getForLane(laneId)` resolves through `getDisplayRowForCurrentLaneBranch`: it returns the most recently updated PR whose head branch matches the lane's current branch ref, ranked open/draft → merged → closed, so a freshly merged PR still shows in lane-scoped UI instead of disappearing the moment GitHub flips the state. | | `prService.mobileSnapshot.test.ts` | Coverage for the mobile snapshot builder: stack chaining, capability gates, per-lane create eligibility, workflow-card aggregation | | `prService.mergeInto.test.ts` | Coverage for integration proposals that preview or adopt an existing merge target lane, including dirty-worktree handling and drift metadata. | | `prPollingService.ts` | 60 s polling loop, fingerprint-based change detection, notification emission. Writes `last_polled_at` per PR so callers can run delta polls on the next tick | @@ -146,7 +146,7 @@ Selected channels exposed through `preload.ts`: - `ade.prs.listAll`, `ade.prs.listProposals`, `ade.prs.listQueueStates` - `ade.prs.listOpenForRepo` — flat list of open PRs in the project's GitHub repo as `BranchPullRequest[]` (branch / number / title / state / url / author / updatedAt). Independent of `pull_requests` cache so the lane-creation branch picker can attach PR pills to branches that have no lane yet. See [features/lanes/README.md](../lanes/README.md) for the consumer. - `ade.prs.land`, `ade.prs.landStack`, `ade.prs.landStackEnhanced`, `ade.prs.landQueueNext` -- `ade.prs.getMergeContext`, `ade.prs.getStatus`, `ade.prs.getChecks`, `ade.prs.getReviews`, `ade.prs.getComments`, `ade.prs.getFiles`, `ade.prs.getCommits` +- `ade.prs.getMergeContext`, `ade.prs.getMergeContexts`, `ade.prs.listSnapshots`, `ade.prs.getStatus`, `ade.prs.getChecks`, `ade.prs.getReviews`, `ade.prs.getComments`, `ade.prs.getFiles`, `ade.prs.getCommits` - `ade.prs.cleanupBranch` — delete a merged/closed PR's local and/or remote branch without touching the lane (protected against deleting any primary-lane branch) - `ade.prs.updateDescription`, `ade.prs.updateTitle`, `ade.prs.updateBody`, `ade.prs.setLabels`, `ade.prs.requestReviewers`, `ade.prs.submitReview`, `ade.prs.close`, `ade.prs.reopen` - `ade.prs.getReviewThreads`, `ade.prs.replyToReviewThread`, `ade.prs.resolveReviewThread` @@ -160,7 +160,7 @@ Selected channels exposed through `preload.ts`: - `ade.prs.pathToMerge.start`, `ade.prs.pathToMerge.stop` — drive the Path-to-Merge orchestrator (see [`path-to-merge.md`](./path-to-merge.md)) - `ade.prs.retargetBase` — re-point a PR's base branch (used by Queue Automate Merging when stacking the chain bases before PtM picks them up) - `ade.prs.pipelineSettingsGet`, `ade.prs.pipelineSettingsSave`, `ade.prs.pipelineSettingsDelete` -- `ade.prs.getGitHubSnapshot` — merged repo + external PR snapshot +- `ade.prs.getGitHubSnapshot` — merged repo + external PR snapshot. The default fetch includes open external PRs only; closed/merged external history is opt-in with `includeExternalClosed`. - `ade.prs.simulateIntegration`, `ade.prs.createIntegrationLaneForProposal`, `ade.prs.commitIntegration`, `ade.prs.cleanupIntegrationWorkflow` Integration merge-into flow uses these existing channels with widened @@ -192,10 +192,16 @@ Caching layers: 1. **Runtime cache** — GitHub snapshot is cached for a short TTL inside `prService` on the active runtime (local daemon or remote-attached); repeated in-flight snapshot requests are - deduplicated. + deduplicated. The default snapshot fetches open external PRs only; + closed/merged external history is requested after the user switches + to a history filter or explicitly refreshes that view. 2. **Renderer cache** — `PrsContext` holds the last snapshot so - revisiting the tab renders immediately. -3. **Manual sync** — a "Refresh" action forces a fresh pull. + revisiting the tab renders immediately. Selected PR detail panes + hydrate from `listSnapshots({ prId })` before live status, check, + review, and comment requests run in the background. +3. **Manual sync** — a "Refresh" action forces a fresh pull. Explicit + multi-PR refreshes run with bounded parallelism instead of refreshing + each PR serially. Snapshot contents include `labels` (name, color, description), `isBot`, and `commentCount` fields so filters can run locally. @@ -557,7 +563,8 @@ best-effort — failures log a warning and do not abort the tick. - `PRsPage` parses URL state via `parsePrsRouteState` and writes it back with `buildPrsRouteSearch`. Active tab, workflow sub-tab, selected PR, queue group, lane, and rebase item are all encoded. -- `PrsContext` mounts cheaply on the plain GitHub PR list. The initial `refreshCore` only kicks a background GitHub refresh when the active tab is a workflow tab (`queue` / `integration` / `rebase`) or a PR is selected; otherwise `githubRefreshMode` is left undefined so the renderer paints from the existing snapshot. `applyLocalPrState` calls `lanes.list({ includeStatus: false })` and skips `rebase.scanNeeds` / `lanes.listAutoRebaseStatuses` on the plain list — those legs hydrate the moment a workflow tab opens or a PR is selected, and the periodic 60 s rebase scan + auto-rebase listener also no-op while the user is on the plain list. +- `PrsContext` mounts cheaply on the plain GitHub PR list. The initial `refreshCore` only kicks a background GitHub refresh when the active tab is a workflow tab (`queue` / `integration` / `rebase`) or a PR is selected; otherwise `githubRefreshMode` is left undefined so the renderer paints from the existing snapshot. `applyLocalPrState` calls `prs.listWithConflicts({ includeConflictAnalysis: false })` and `lanes.list({ includeStatus: false })` for the plain list, then enables conflict analysis, rebase-needs scans, and auto-rebase status reads only when a workflow tab or selected PR needs them. +- Workflow surfaces batch PR merge context through `prs.getMergeContexts(prIds)` instead of fanning out one `getMergeContext(prId)` call per card. The service builds the batch from metadata-only lane rows so queue/integration/rebase views do not pay full git status cost on render. Conflict analysis also runs as one batch over metadata-only active lanes, preserving overlap warnings against non-PR peer lanes without per-PR conflict calls. - `PrsContext` owns PR list, queue states, rebase needs, proposals, convergence runtime state, and the Timeline+Rails UI state (`prsTimelineRailsEnabled`, `timelineFiltersByPrId`, @@ -572,7 +579,9 @@ best-effort — failures log a warning and do not abort the tick. (`PrCiRunningIndicator`), merge readiness with bypass checkbox, PR markdown rendered with `rehype-sanitize` after `rehype-raw`. - `GitHubTab` renders the unified repo+external list; filter tab - counts respect the active scope. + counts respect the active scope. Open views load open external PRs + first; switching to Closed, Merged, or All asks the runtime for the + closed external history snapshot. ## CTO operator tools diff --git a/docs/perf/prs-tab-action-inventory.md b/docs/perf/prs-tab-action-inventory.md new file mode 100644 index 000000000..26e948a0b --- /dev/null +++ b/docs/perf/prs-tab-action-inventory.md @@ -0,0 +1,184 @@ +# PRs tab action inventory + +This is the audit matrix for PRs-tab autoresearch. It is deliberately not a +completion claim. A row is only `measured` when a real PRs-tab UI run has a +matching manual marker, an equivalent UI-derived probe against the perf-pass +repo, or a focused test that reproduces the exact behavior. + +Coverage states: + +- `source`: found in source, not yet driven in the current inventory pass. +- `measured`: exact row covered by a real PRs UI run, UI-derived probe, or + focused fixture test with evidence. +- `measured-partial`: driven in an earlier partial pass; must be re-driven by + this matrix before claiming full coverage. +- `fixture-needed`: safe to drive, but needs a seeded PR/lane/session state. +- `sandbox-only`: may start agents, local tools, or mutate the perf-pass repo. +- `prompt-only`: destructive or externally visible path. Open and measure the + confirmation/preflight, then cancel unless explicitly allowed. +- `external-skip`: opens GitHub, a browser, another app, or copies to clipboard. +- `not-applicable`: row came from an older/source-derived inventory item but the + current visible PRs surface no longer exposes that control. + +Evidence run ids used so far: + +- `prs-ui-baseline-20260512-051124` +- `prs-ui-lane-metadata-fast-inproc-20260512-060555` +- `prs-ui-rebase-fetch-ttl-20260512-062130` +- `prs-ui-ptm-audit-20260512-0635` +- `prs-ui-coverage-closeout-20260512-074602` + +## Route shell + +| id | action | state | evidence | +| --- | --- | --- | --- | +| prs.route.github.open | Open PRs route on GitHub surface with perf-pass project | measured | baseline and optimized run ids above | +| prs.route.workflows.open | Switch from GitHub surface to Workflows surface | measured | `prs-ui-rebase-fetch-ttl-20260512-062130` | +| prs.header.create-pr | Open Create PR modal | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.create.open` | +| prs.header.tab.github | Switch to GitHub tab | measured | `prs-ui-coverage-closeout-20260512-074602`, marker `prs.header.tab.github` | +| prs.header.tab.workflows | Switch to Workflows tab | measured | `prs-ui-rebase-fetch-ttl-20260512-062130` | + +## GitHub list + +| id | action | state | evidence | +| --- | --- | --- | --- | +| prs.github.snapshot.open | Load open PR snapshot | measured | `getGitHubSnapshot 5941ms -> 1146ms` | +| prs.github.search | Type and clear PR search | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.github.search` | +| prs.github.sync | Click Sync | measured | `prs-ui-coverage-closeout-20260512-074602`, marker `prs.github.sync`; explicit all-18 preload refresh measured at `3800ms` | +| prs.github.filter.open | Select Open filter | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.github.filters-and-scopes` | +| prs.github.filter.merged | Select Merged filter | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.github.filters-and-scopes` | +| prs.github.filter.closed | Select Closed filter | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.github.filters-and-scopes`; closed/all history fetch is intentionally opt-in | +| prs.github.filter.all | Select All filter | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.github.filters-and-scopes` | +| prs.github.scope.ade | Select ADE scope | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.github.filters-and-scopes` | +| prs.github.scope.external | Select External scope | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.github.filters-and-scopes` | +| prs.github.card.select.local | Select a local ADE PR card | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs-ptm-open-local-pr` | +| prs.github.card.select.external | Select an external PR card | measured | `prs-ui-coverage-closeout-20260512-074602`, marker `prs.github.card.select.external` | +| prs.github.card.open-github | Click PR card Open on GitHub | external-skip | `GitHubTab.tsx` | +| prs.github.card.open-queue | Click queue shortcut from PR card | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.detail.open-queue`; queue shortcut path also covered by `prs.queue.open-pr` | + +## PR detail + +| id | action | state | evidence | +| --- | --- | --- | --- | +| prs.detail.open | Open local PR detail pane | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs-ptm-open-local-pr` | +| prs.detail.open-external | Open external PR detail pane | measured | `prs-ui-coverage-closeout-20260512-074602`, marker `prs.detail.open-external` | +| prs.detail.refresh | Click detail Refresh | measured | `prs-ui-coverage-closeout-20260512-074602`, marker `prs.detail.refresh` | +| prs.detail.view-github | Click View/Open on GitHub | external-skip | `PrDetailPane.tsx` | +| prs.detail.more-menu | Open detail overflow menu | not-applicable | Current top-level `PrDetailPane.tsx` exposes direct `Refresh`, `GitHub`, `Graph`, and `Edit title` buttons; no detail overflow menu is rendered | +| prs.detail.copy-url | Copy PR URL from detail menu | not-applicable | No current top-level detail copy-url menu is rendered; comment-level copy links remain external/clipboard-skip | +| prs.detail.edit-title | Edit PR title | prompt-only | `PrDetailPane.tsx` | +| prs.detail.open-queue | Open linked queue from detail | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.detail.open-queue` | +| prs.detail.open-graph | Show lane in graph | external-skip | navigates away from PRs | +| prs.detail.overview | Open Overview tab | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.detail.overview` | +| prs.detail.path-to-merge | Open Path to Merge tab | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs-ptm-open-tab` | +| prs.detail.files | Open Files tab | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.detail.files` | +| prs.detail.files.expand | Expand a changed file diff | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.detail.files.expand` | +| prs.detail.ci | Open CI / Checks tab | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.detail.ci` | +| prs.detail.ci.open-log | Open check log drawer | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.detail.ci.open-log`; Checks-tab button opened no drawer, Overview timeline log button did | +| prs.detail.ci.open-full-log | Open full CI log | external-skip | `PrCheckLogDrawer.tsx` | +| prs.detail.ci.rerun | Rerun checks | prompt-only | `PrDetailPane.tsx` | +| prs.detail.activity | Open Activity tab | measured | `prs-ui-coverage-closeout-20260512-074602`, marker `prs.detail.activity` | +| prs.detail.comment | Add PR comment | prompt-only | `ActivityTab` | +| prs.detail.review-modal | Open review modal | fixture-needed | hidden by timeline-rails overview in the current surface; drive with flag/fixture if still supported | +| prs.detail.review-submit | Submit PR review | prompt-only | `PrDetailPane.tsx` | +| prs.detail.request-reviewers | Edit/request reviewers | fixture-needed | hidden by timeline-rails overview in the current surface | +| prs.detail.labels | Edit labels | fixture-needed | hidden by timeline-rails overview in the current surface | +| prs.detail.ai-summary | Generate AI summary | fixture-needed | hidden by timeline-rails overview in the current surface | +| prs.detail.issue-resolver | Open issue resolver modal | fixture-needed | needs actionable failed checks or unresolved review threads | +| prs.detail.issue-resolver.start | Resolve issues with agent | sandbox-only | `PrIssueResolverModal.tsx` | +| prs.detail.close-pr | Close PR | prompt-only | `PrDetailPane.tsx` | +| prs.detail.reopen-pr | Reopen PR | prompt-only | `PrDetailPane.tsx` | + +## Path to Merge + +| id | action | state | evidence | +| --- | --- | --- | --- | +| prs.ptm.panel-open | Open Path to Merge panel | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs-ptm-open-tab` | +| prs.ptm.settings | Change pipeline settings | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs-ptm-safe-settings`, PLAN mode and auto-merge off | +| prs.ptm.start | Start Path to Merge | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs-ptm-start-plan`, `ade.prs.pathToMerge.start 5ms` | +| prs.ptm.stop | Stop active Path to Merge | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs-ptm-stop-plan`, `ade.prs.pathToMerge.stop 2ms` | +| prs.ptm.force-finalize | Use force-finalize control | prompt-only | `PrConvergencePanel.tsx` | +| prs.ptm.issue-inventory | Sync/read issue inventory | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs-ptm-refresh-seeded-inventory`, seeded local inventory rendered | + +Path to Merge coverage was run with PLAN mode and auto-merge off against a +seeded local-only inventory item. It covered native UI start/stop and runtime +state changes without dispatching an agent session; dispatch still depends on +real GitHub checks or review threads becoming actionable. + +## Workflows shell + +| id | action | state | evidence | +| --- | --- | --- | --- | +| prs.workflows.active | Select Active | measured | `prs-ui-coverage-closeout-20260512-074602`, marker `prs.workflows.active` | +| prs.workflows.history | Select History | measured | `prs-ui-coverage-closeout-20260512-074602`, marker `prs.workflows.history`; empty history state rendered | +| prs.workflows.integration | Select Integration | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.integration.select` | +| prs.workflows.queue | Select Queue | measured | `prs-ui-rebase-fetch-ttl-20260512-062130` | +| prs.workflows.rebase | Select Rebase/Merge | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.queue.inspect-rebase` | +| prs.workflows.refresh | Click Workflows Refresh | measured | stale/no-op UI plus explicit all-18 refresh evidence | + +## Queue workflow + +| id | action | state | evidence | +| --- | --- | --- | --- | +| prs.queue.select | Select queue group | measured | `prs-ui-rebase-fetch-ttl-20260512-062130` | +| prs.queue.open-pr | Open a queue member PR | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.queue.open-pr` | +| prs.queue.open-github | Open queue member on GitHub | external-skip | `WorkflowsTab.tsx` | +| prs.queue.inspect-rebase | Inspect queued lane rebase need | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.queue.inspect-rebase` | +| prs.queue.scope-next | Select Next lane only | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.queue.scope-controls` | +| prs.queue.scope-all | Select All affected lanes | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.queue.scope-controls` | +| prs.queue.rebase-ai | Rebase with AI | sandbox-only | `WorkflowsTab.tsx` / `RebaseTab.tsx` | +| prs.queue.rebase-local | Rebase now local only | prompt-only | mutates perf-pass worktree | +| prs.queue.rebase-push | Rebase and push | prompt-only | externally visible GitHub push | +| prs.queue.land-current | Land current PR | prompt-only | externally visible merge | +| prs.queue.resume | Resume queue automation | prompt-only | `WorkflowsTab.tsx` | +| prs.queue.cancel | Cancel queue automation | prompt-only | `WorkflowsTab.tsx` | +| prs.queue.automate | Open automate merging modal | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.queue.automate`, opened and cancelled | + +## Integration workflow + +| id | action | state | evidence | +| --- | --- | --- | --- | +| prs.integration.select | Select integration workflow | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.integration.select` | +| prs.integration.refresh | Click integration workflow Refresh | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.integration.refresh` | +| prs.integration.open-linked-pr | Open linked PR in ADE | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.integration.open-linked-pr` | +| prs.integration.open-github | Open linked PR on GitHub | external-skip | `WorkflowsTab.tsx` | +| prs.integration.cleanup | Open cleanup controls | prompt-only | `WorkflowsTab.tsx` | +| prs.integration.dismiss-cleanup | Dismiss cleanup prompt | prompt-only | `WorkflowsTab.tsx` | +| prs.integration.simulate | Simulate integration | sandbox-only | `IntegrationTab.tsx` | +| prs.integration.create-lane | Create integration lane | sandbox-only | `IntegrationTab.tsx` | +| prs.integration.commit | Commit integration | prompt-only | mutates perf-pass/GitHub workflow | +| prs.integration.resolver | Start integration resolver | sandbox-only | `IntegrationTab.tsx` | + +## Rebase/Merge workflow + +| id | action | state | evidence | +| --- | --- | --- | --- | +| prs.rebase.select-need | Select a rebase need | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.queue.inspect-rebase` navigated to rebase need | +| prs.rebase.commit-expand | Expand commit/file details | measured | `prs-ui-coverage-closeout-20260512-074602`, marker `prs.rebase.commit-expand`; expanded `5c7d11f` file details | +| prs.rebase.dismiss | Dismiss/defer rebase need | prompt-only | `RebaseTab.tsx` | +| prs.rebase.ai | Rebase with AI | sandbox-only | `RebaseTab.tsx` | +| prs.rebase.local | Rebase now local only | prompt-only | mutates perf-pass worktree | +| prs.rebase.push | Rebase and push | prompt-only | externally visible GitHub push | +| prs.rebase.abort | Abort active rebase | prompt-only | `RebaseTab.tsx` | +| prs.rebase.rollback | Roll back completed rebase | prompt-only | `RebaseTab.tsx` | + +## Create PR modal + +| id | action | state | evidence | +| --- | --- | --- | --- | +| prs.create.open | Open Create PR modal | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.create.open` | +| prs.create.mode.normal | Select normal PR mode | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.create.mode.normal` | +| prs.create.mode.queue | Select queue workflow mode | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.create.mode.queue` | +| prs.create.mode.integration | Select integration workflow mode | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.create.mode.integration` | +| prs.create.lane-picker | Change lane/source pickers | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.create.lane-picker` | +| prs.create.rebase-warning | Read rebase warning panel | measured | `prs-ui-ptm-audit-20260512-0635`, marker `prs.create.lane-picker`; warning rendered for behind lane selection | +| prs.create.submit | Submit PR creation | prompt-only | externally visible GitHub PR | + +## Deferred and non-executed rows + +- Issue resolver and legacy overview controls remain `fixture-needed` because + they require actionable failed checks, unresolved review threads, or older + overview controls hidden by the current timeline-rails surface. +- Prompt-only, sandbox-only, and external-skip rows are intentionally not fully + executed in this audit because they mutate GitHub/perf-pass state, launch + agents/tools, copy to clipboard, or leave ADE.