From 023a6051c1b2022f9062c0fac8182ac763ccd7e0 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 2 Aug 2026 04:15:59 -0400 Subject: [PATCH 1/2] fix(prs): read a lane's PR from the machine that owns it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the global machine selector on one machine and work running in threads on another, those sessions showed no PR tab: no badge on the Work session card, no PR pill in the chat or CLI header. Switching the selector to that machine made them all appear at once. A lane's PR row lives in the `.ade` database of the machine that owns the lane, so a PR read is a per-lane fact exactly like a chat or a terminal. Chats, terminals and lanes already route through `callPinnedOrBoundRuntimeActionOr` for that reason; the whole `pr` domain went through the unpinned helper instead, so every PR read resolved against whichever machine the project tab happened to be bound to. Two renderer sites then hard-suppressed what was left: `SessionListPane` nulled `sessionPr` for any foreign row, and `WorkViewArea` nulled the PR lane id whenever a runtime pin was present. Nothing ever fetched another machine's PR rows at all. - PR reads take a pin. `callPrReadRuntimeActionOr(pin, ...)` now delegates to the pinned helper; `prs.onEvent` takes one too, or a pinned pill would listen to the wrong database and go stale. Tab-scoped call sites pass `null` explicitly — the PRs tab is the global execution context by design, and that is now stated rather than inferred from which helper was chosen. - The cross-machine union carries `prs` per machine, read at the lane cadence: in parallel with the lane and session reads on a cadence tick, and sequentially only for the off-cadence catch-up, so a slow PR read cannot hold a machine's lanes and chats behind an 8s timeout. - Map keys are namespaced — `bound:`, `:`, `any:`. Lane ids are not unique across machines (handoff copies them), so a bare id let a foreign machine's PR render on a bound-machine row and deep-link into a PRs tab that cannot resolve it. - A foreign PR opens on GitHub. A PR id only resolves on its own machine, so `openLanePr` is now the single answer to "where does this PR open" for all six call sites; the sidebar badge, the card, and the chat pane had drifted into three. Reads are pinned; writes are not. Creating a PR still belongs to the machine that owns the lane — the creator derives its branches from that machine's lane list — so a pinned pane says "Switch to to open one" instead of offering a button that would misfire. Co-Authored-By: Claude Opus 5 --- apps/desktop/src/preload/global.d.ts | 54 +++++-- apps/desktop/src/preload/preload.ts | 125 ++++++++++----- .../components/chat/AgentChatPane.test.tsx | 2 + .../components/chat/AgentChatPane.tsx | 19 ++- .../components/chat/ChatGitToolbar.test.tsx | 74 ++++++++- .../components/chat/ChatGitToolbar.tsx | 103 +++++++++---- .../components/chat/ChatPrPane.test.tsx | 4 +- .../renderer/components/chat/ChatPrPane.tsx | 65 ++++++-- .../components/chat/useChatPrAutoPop.ts | 17 ++- .../components/prs/state/PrsContext.test.tsx | 2 +- .../terminals/CliSessionWorkSurfaceHeader.tsx | 11 +- .../components/terminals/LanePrBadge.tsx | 5 - .../components/terminals/SessionCard.tsx | 19 ++- .../terminals/SessionListPane.test.tsx | 44 +++++- .../components/terminals/SessionListPane.tsx | 46 ++++-- .../terminals/WorkStartSurface.test.tsx | 1 + .../terminals/WorkViewArea.test.tsx | 15 +- .../components/terminals/WorkViewArea.tsx | 18 ++- .../components/terminals/useLanePrs.test.ts | 54 ++++++- .../components/terminals/useLanePrs.ts | 137 ++++++++++++++++- .../terminals/useWorkSessions.test.ts | 15 ++ .../components/terminals/useWorkSessions.ts | 7 +- .../components/work/WorkSurfaceHeader.tsx | 11 +- .../src/renderer/lib/lanePrBadge.test.ts | 64 +++++++- apps/desktop/src/renderer/lib/lanePrBadge.ts | 48 ++++++ apps/desktop/src/renderer/lib/prReadCache.ts | 39 +++-- apps/desktop/src/renderer/state/appStore.ts | 17 ++- ...e.test.ts => appStoreWorktreeGate.test.ts} | 0 .../renderer/state/crossMachineLanes.test.ts | 144 +++++++++++++++++- .../src/renderer/state/crossMachineLanes.ts | 97 +++++++++++- .../src/renderer/webclient/adapter/prs.ts | 43 ++++-- docs/ARCHITECTURE.md | 3 +- docs/features/chat/README.md | 4 +- docs/features/chat/composer-and-ui.md | 4 +- docs/features/pull-requests/README.md | 110 ++++++++++++- docs/features/sync-and-multi-device/README.md | 31 +++- .../features/terminals-and-sessions/README.md | 26 +++- .../terminals-and-sessions/ui-surfaces.md | 11 +- 38 files changed, 1272 insertions(+), 217 deletions(-) rename apps/desktop/src/renderer/state/{appStore.worktreeGate.test.ts => appStoreWorktreeGate.test.ts} (100%) diff --git a/apps/desktop/src/preload/global.d.ts b/apps/desktop/src/preload/global.d.ts index a81a6d4fb3..5a08403804 100644 --- a/apps/desktop/src/preload/global.d.ts +++ b/apps/desktop/src/preload/global.d.ts @@ -2365,19 +2365,46 @@ declare global { createLaneFromPrBranch: ( args: CreateLaneFromPrBranchArgs, ) => Promise; - getForLane: (laneId: string) => Promise; - syncLanePr: (laneId: string) => Promise; + /** + * `pin` routes the read to the machine that owns the lane. A PR row + * lives in that machine's database, so an unpinned read only ever + * answers for the machine the project tab is bound to. The PRs tab + * (which IS bound-machine-scoped) simply omits it. + */ + getForLane: ( + laneId: string, + pin?: OpenProjectBinding | null, + ) => Promise; + syncLanePr: ( + laneId: string, + pin?: OpenProjectBinding | null, + ) => Promise; reconcileNow: () => Promise; - listAll: () => Promise; + listAll: (pin?: OpenProjectBinding | null) => Promise; listOpenForRepo: () => Promise; - refresh: (args?: { - prId?: string; - prIds?: string[]; - }) => Promise; - getStatus: (prId: string) => Promise; - getChecks: (prId: string) => Promise; - getComments: (prId: string) => Promise; - getReviews: (prId: string) => Promise; + refresh: ( + args?: { + prId?: string; + prIds?: string[]; + }, + pin?: OpenProjectBinding | null, + ) => Promise; + getStatus: ( + prId: string, + pin?: OpenProjectBinding | null, + ) => Promise; + getChecks: ( + prId: string, + pin?: OpenProjectBinding | null, + ) => Promise; + getComments: ( + prId: string, + pin?: OpenProjectBinding | null, + ) => Promise; + getReviews: ( + prId: string, + pin?: OpenProjectBinding | null, + ) => Promise; getReviewThreads: (prId: string) => Promise; updateDescription: (args: UpdatePrDescriptionArgs) => Promise; delete: (args: DeletePrArgs) => Promise; @@ -2463,7 +2490,10 @@ declare global { listIntegrationWorkflows: ( args?: ListIntegrationWorkflowsArgs, ) => Promise; - onEvent: (cb: (ev: PrEventPayload) => void) => () => void; + onEvent: ( + cb: (ev: PrEventPayload) => void, + pin?: OpenProjectBinding | null, + ) => () => void; getDetail: (prId: string) => Promise; getFiles: (prId: string) => Promise; getCommits: (prId: string) => Promise; diff --git a/apps/desktop/src/preload/preload.ts b/apps/desktop/src/preload/preload.ts index 6c5cad57f0..c4286f30c9 100644 --- a/apps/desktop/src/preload/preload.ts +++ b/apps/desktop/src/preload/preload.ts @@ -1593,12 +1593,23 @@ function callPinnedOrBoundRuntimeActionOr( return callProjectRuntimeActionOr(domain, action, request, local); } +// A lane's PR record lives in the `.ade` database of the machine that owns the +// lane, so a PR read is a per-lane fact exactly like a chat or a terminal — and +// takes a pin for the same reason. Without one, every PR read resolves against +// whichever machine the project tab happens to be bound to, so a session on +// another machine showed no PR badge on its card and no PR pill in its header +// until the tab was rebound to that machine. +// +// `pin: null` is not "no opinion" — it means "the machine the project tab is +// bound to", which is exactly right for the PRs tab, the global execution +// context. Every call site states which of the two it wants. function callPrReadRuntimeActionOr( + pin: OpenProjectBinding | null | undefined, action: string, request: Omit, local: () => Promise, ): Promise { - return callProjectRuntimeActionOr("pr", action, request, local); + return callPinnedOrBoundRuntimeActionOr(pin, "pr", action, request, local); } async function callProjectFileRuntimeActionOr( @@ -8706,50 +8717,69 @@ contextBridge.exposeInMainWorld("ade", { callProjectRuntimeActionStrictOr("pr", "createLaneFromPrBranch", { args }, () => ipcRenderer.invoke(IPC.prsCreateLaneFromPrBranch, args), ), - getForLane: async (laneId: string): Promise => - callPrReadRuntimeActionOr("getForLane", { arg: laneId }, () => + getForLane: async ( + laneId: string, + pin?: OpenProjectBinding | null, + ): Promise => + callPrReadRuntimeActionOr(pin, "getForLane", { arg: laneId }, () => ipcRenderer.invoke(IPC.prsGetForLane, { laneId }), ), - syncLanePr: async (laneId: string): Promise => - callPrReadRuntimeActionOr("syncLanePr", { arg: laneId }, () => + syncLanePr: async ( + laneId: string, + pin?: OpenProjectBinding | null, + ): Promise => + callPrReadRuntimeActionOr(pin, "syncLanePr", { arg: laneId }, () => ipcRenderer.invoke(IPC.prsSyncLanePr, { laneId }), ), reconcileNow: async (): Promise => - callPrReadRuntimeActionOr("reconcileOnFocus", { args: { force: true } }, () => + callPrReadRuntimeActionOr(null, "reconcileOnFocus", { args: { force: true } }, () => ipcRenderer.invoke(IPC.prsReconcileNow), ), - listAll: async (): Promise => - callPrReadRuntimeActionOr("listAll", { args: {} }, () => + listAll: async (pin?: OpenProjectBinding | null): Promise => + callPrReadRuntimeActionOr(pin, "listAll", { args: {} }, () => ipcRenderer.invoke(IPC.prsListAll), ), listOpenForRepo: async (): Promise => - callPrReadRuntimeActionOr("listOpenPullRequests", {}, () => + callPrReadRuntimeActionOr(null, "listOpenPullRequests", {}, () => ipcRenderer.invoke(IPC.prsListOpenForRepo), ), refresh: async ( args: { prId?: string; prIds?: string[] } = {}, + pin?: OpenProjectBinding | null, ): Promise => - callPrReadRuntimeActionOr("refresh", { args }, () => + callPrReadRuntimeActionOr(pin, "refresh", { args }, () => ipcRenderer.invoke(IPC.prsRefresh, args), ), - getStatus: async (prId: string): Promise => - callPrReadRuntimeActionOr("getStatus", { arg: prId }, () => + getStatus: async ( + prId: string, + pin?: OpenProjectBinding | null, + ): Promise => + callPrReadRuntimeActionOr(pin, "getStatus", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetStatus, { prId }), ), - getChecks: async (prId: string): Promise => - callPrReadRuntimeActionOr("getChecks", { arg: prId }, () => + getChecks: async ( + prId: string, + pin?: OpenProjectBinding | null, + ): Promise => + callPrReadRuntimeActionOr(pin, "getChecks", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetChecks, { prId }), ), - getComments: async (prId: string): Promise => - callPrReadRuntimeActionOr("getComments", { arg: prId }, () => + getComments: async ( + prId: string, + pin?: OpenProjectBinding | null, + ): Promise => + callPrReadRuntimeActionOr(pin, "getComments", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetComments, { prId }), ), - getReviews: async (prId: string): Promise => - callPrReadRuntimeActionOr("getReviews", { arg: prId }, () => + getReviews: async ( + prId: string, + pin?: OpenProjectBinding | null, + ): Promise => + callPrReadRuntimeActionOr(pin, "getReviews", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetReviews, { prId }), ), getReviewThreads: async (prId: string): Promise => - callPrReadRuntimeActionOr("getReviewThreads", { arg: prId }, () => + callPrReadRuntimeActionOr(null, "getReviewThreads", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetReviewThreads, { prId }), ), updateDescription: async (args: UpdatePrDescriptionArgs): Promise => @@ -8850,21 +8880,23 @@ contextBridge.exposeInMainWorld("ade", { () => ipcRenderer.invoke(IPC.prsGetConflictAnalysis, { prId }), ), getMergeContext: (prId: string): Promise => - callPrReadRuntimeActionOr("getMergeContext", { arg: prId }, () => + callPrReadRuntimeActionOr(null, "getMergeContext", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetMergeContext, { prId }), ), getMergeContexts: (prIds: string[]): Promise> => callPrReadRuntimeActionOr( + null, "getMergeContexts", { argsList: [prIds] }, () => ipcRenderer.invoke(IPC.prsGetMergeContexts, { prIds }), ), listWithConflicts: (args: { includeConflictAnalysis?: boolean } = {}): Promise => - callPrReadRuntimeActionOr("listWithConflicts", { args }, () => + callPrReadRuntimeActionOr(null, "listWithConflicts", { args }, () => ipcRenderer.invoke(IPC.prsListWithConflicts, args), ), listSnapshots: (args: { prId?: string } = {}): Promise => callPrReadRuntimeActionOr( + null, "listSnapshots", { args }, () => ipcRenderer.invoke(IPC.prsListSnapshots, args), @@ -8875,6 +8907,7 @@ contextBridge.exposeInMainWorld("ade", { historyPageLimit?: number; }): Promise => callPrReadRuntimeActionOr( + null, "getGithubSnapshot", { args: args ?? {} }, () => ipcRenderer.invoke(IPC.prsGetGitHubSnapshot, args ?? {}), @@ -8999,7 +9032,17 @@ contextBridge.exposeInMainWorld("ade", { ipcRenderer.removeListener(IPC.prsAiResolutionEvent, listener); }; }, - onEvent: (cb: (ev: PrEventPayload) => void) => { + onEvent: (cb: (ev: PrEventPayload) => void, pin?: OpenProjectBinding | null) => { + // A pinned surface reads its PRs from the lane's machine, so it must hear + // that machine's `prs-updated` too — the bound runtime's feed describes a + // different database and would leave the pinned pill permanently stale. + const removePinned = subscribePinnedProjectRuntimeEvents( + pin, + (payload) => toWrappedEvent(payload, "pr_event"), + cb, + "PR event", + ); + if (removePinned) return removePinned; const unsubscribeLocal = subscribeLocalPrEvents(cb); const unsubscribeRemote = subscribeRemotePrEvents(cb); return () => { @@ -9008,71 +9051,71 @@ contextBridge.exposeInMainWorld("ade", { }; }, getDetail: async (prId: string): Promise => - callPrReadRuntimeActionOr("getDetail", { arg: prId }, () => + callPrReadRuntimeActionOr(null, "getDetail", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetDetail, { prId }), ), getFiles: async (prId: string): Promise => - callPrReadRuntimeActionOr("getFiles", { arg: prId }, () => + callPrReadRuntimeActionOr(null, "getFiles", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetFiles, { prId }), ), getCommits: async (prId: string): Promise => - callPrReadRuntimeActionOr("getCommits", { arg: prId }, () => + callPrReadRuntimeActionOr(null, "getCommits", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetCommits, { prId }), ), getActionRuns: async (prId: string): Promise => - callPrReadRuntimeActionOr("getActionRuns", { arg: prId }, () => + callPrReadRuntimeActionOr(null, "getActionRuns", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetActionRuns, { prId }), ), getActivity: async (prId: string): Promise => - callPrReadRuntimeActionOr("getActivity", { arg: prId }, () => + callPrReadRuntimeActionOr(null, "getActivity", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetActivity, { prId }), ), getWorkflowGraph: async (args: GetPrWorkflowGraphArgs): Promise => - callPrReadRuntimeActionOr("getWorkflowGraph", { args }, () => + callPrReadRuntimeActionOr(null, "getWorkflowGraph", { args }, () => ipcRenderer.invoke(IPC.prsGetWorkflowGraph, args), ), getCheckLog: async (args: GetPrCheckLogArgs): Promise => - callPrReadRuntimeActionOr("getCheckLog", { args }, () => + callPrReadRuntimeActionOr(null, "getCheckLog", { args }, () => ipcRenderer.invoke(IPC.prsGetCheckLog, args), ), getDetailByGithub: async (coords: PrGithubCoords): Promise => - callPrReadRuntimeActionOr("getDetailByGithub", { arg: coords }, () => + callPrReadRuntimeActionOr(null, "getDetailByGithub", { arg: coords }, () => ipcRenderer.invoke(IPC.prsGetDetailByGithub, coords), ), getFilesByGithub: async (coords: PrGithubCoords): Promise => - callPrReadRuntimeActionOr("getFilesByGithub", { arg: coords }, () => + callPrReadRuntimeActionOr(null, "getFilesByGithub", { arg: coords }, () => ipcRenderer.invoke(IPC.prsGetFilesByGithub, coords), ), getCommitsByGithub: async (coords: PrGithubCoords): Promise => - callPrReadRuntimeActionOr("getCommitsByGithub", { arg: coords }, () => + callPrReadRuntimeActionOr(null, "getCommitsByGithub", { arg: coords }, () => ipcRenderer.invoke(IPC.prsGetCommitsByGithub, coords), ), getActionRunsByGithub: async (coords: PrGithubCoords): Promise => - callPrReadRuntimeActionOr("getActionRunsByGithub", { arg: coords }, () => + callPrReadRuntimeActionOr(null, "getActionRunsByGithub", { arg: coords }, () => ipcRenderer.invoke(IPC.prsGetActionRunsByGithub, coords), ), getActivityByGithub: async (coords: PrGithubCoords): Promise => - callPrReadRuntimeActionOr("getActivityByGithub", { arg: coords }, () => + callPrReadRuntimeActionOr(null, "getActivityByGithub", { arg: coords }, () => ipcRenderer.invoke(IPC.prsGetActivityByGithub, coords), ), getStatusByGithub: async (coords: PrGithubCoords): Promise => - callPrReadRuntimeActionOr("getStatusByGithub", { arg: coords }, () => + callPrReadRuntimeActionOr(null, "getStatusByGithub", { arg: coords }, () => ipcRenderer.invoke(IPC.prsGetStatusByGithub, coords), ), getChecksByGithub: async (coords: PrGithubCoords): Promise => - callPrReadRuntimeActionOr("getChecksByGithub", { arg: coords }, () => + callPrReadRuntimeActionOr(null, "getChecksByGithub", { arg: coords }, () => ipcRenderer.invoke(IPC.prsGetChecksByGithub, coords), ), getReviewsByGithub: async (coords: PrGithubCoords): Promise => - callPrReadRuntimeActionOr("getReviewsByGithub", { arg: coords }, () => + callPrReadRuntimeActionOr(null, "getReviewsByGithub", { arg: coords }, () => ipcRenderer.invoke(IPC.prsGetReviewsByGithub, coords), ), getCommentsByGithub: async (coords: PrGithubCoords): Promise => - callPrReadRuntimeActionOr("getCommentsByGithub", { arg: coords }, () => + callPrReadRuntimeActionOr(null, "getCommentsByGithub", { arg: coords }, () => ipcRenderer.invoke(IPC.prsGetCommentsByGithub, coords), ), getReviewThreadsByGithub: async (coords: PrGithubCoords): Promise => - callPrReadRuntimeActionOr("getReviewThreadsByGithub", { arg: coords }, () => + callPrReadRuntimeActionOr(null, "getReviewThreadsByGithub", { arg: coords }, () => ipcRenderer.invoke(IPC.prsGetReviewThreadsByGithub, coords), ), addComment: async (args: AddPrCommentArgs): Promise => @@ -9154,11 +9197,11 @@ contextBridge.exposeInMainWorld("ade", { () => ipcRenderer.invoke(IPC.prsCleanupIntegrationWorkflow, args), ), getDeployments: async (prId: string): Promise => - callPrReadRuntimeActionOr("getDeployments", { arg: prId }, () => + callPrReadRuntimeActionOr(null, "getDeployments", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetDeployments, { prId }), ), getAiSummary: async (prId: string): Promise => - callPrReadRuntimeActionOr("getAiSummary", { arg: prId }, () => + callPrReadRuntimeActionOr(null, "getAiSummary", { arg: prId }, () => ipcRenderer.invoke(IPC.prsGetAiSummary, { prId }), ), regenerateAiSummary: async (prId: string): Promise => diff --git a/apps/desktop/src/renderer/components/chat/AgentChatPane.test.tsx b/apps/desktop/src/renderer/components/chat/AgentChatPane.test.tsx index 6279c7aa55..f21cac8084 100644 --- a/apps/desktop/src/renderer/components/chat/AgentChatPane.test.tsx +++ b/apps/desktop/src/renderer/components/chat/AgentChatPane.test.tsx @@ -5533,6 +5533,7 @@ describe("AgentChatPane submit recovery", () => { online: true, lanes: localLanes as any, sessions: [], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, @@ -9955,6 +9956,7 @@ describe("AgentChatPane per-chat runtime routing", () => { online: true, lanes: [{ id: "lane-b", name: "lane on B" }], sessions: [], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, diff --git a/apps/desktop/src/renderer/components/chat/AgentChatPane.tsx b/apps/desktop/src/renderer/components/chat/AgentChatPane.tsx index 5e9ca024f4..aa022b1e71 100644 --- a/apps/desktop/src/renderer/components/chat/AgentChatPane.tsx +++ b/apps/desktop/src/renderer/components/chat/AgentChatPane.tsx @@ -3741,13 +3741,6 @@ export function AgentChatPane({ companionStateKey === WORK_START_DRAFT_COMPANION_STATE_KEY && legacyWorkDraftLaneId ? `draft:${legacyWorkDraftLaneId}` : null; - // Left PR floating pane (ADE chats only). Auto-pops on webhook-driven PR - // changes; shared with the CLI session surface via useChatPrAutoPop. - // `persistKey` makes open/closed per chat and durable across restarts — - // declared here because it needs `companionStateKey`. - const { prPaneOpen, setPrPaneOpen, prPaneDelta } = useChatPrAutoPop(laneId, { - persistKey: companionStateKey, - }); // Measured height of the floating PR pane card, published to the minimap rail // through ChatPrPaneInsetContext so it can re-centre in the band left below. const prPaneInset = usePrPaneInsetObserver(); @@ -3943,6 +3936,16 @@ export function AgentChatPane({ ); const chatRuntimePinRef = useRef(chatRuntimePin); chatRuntimePinRef.current = chatRuntimePin; + // Left PR floating pane (ADE chats only). Auto-pops on webhook-driven PR + // changes; shared with the CLI session surface via useChatPrAutoPop. + // `persistKey` makes open/closed per chat and durable across restarts. + // Declared HERE, below `chatRuntimePin`, because a chat on another machine + // must read its PR from that machine — the pane and its auto-pop take the + // same pin every other call this chat makes already takes. + const { prPaneOpen, setPrPaneOpen, prPaneDelta } = useChatPrAutoPop(laneId, { + persistKey: companionStateKey, + runtimePin: chatRuntimePin, + }); const renderedSession = useMemo( () => ( renderedSessionId @@ -11771,6 +11774,7 @@ export function AgentChatPane({ showGitToolbar={showWorkspaceChrome} onTogglePrPane={showWorkspaceChrome && laneId ? () => setPrPaneOpen((v) => !v) : undefined} prPaneOpen={prPaneOpen} + runtimePin={chatRuntimePin} trailingActions={chatHeaderTrailingActions} onToggleSessionsPane={onToggleSessionsPane} sessionsPaneCollapsed={sessionsPaneCollapsed} @@ -13032,6 +13036,7 @@ export function AgentChatPane({ sessionTitle={selectedSession?.title ?? null} delta={prPaneDelta} onClose={() => setPrPaneOpen(false)} + runtimePin={chatRuntimePin} />, ) : null} diff --git a/apps/desktop/src/renderer/components/chat/ChatGitToolbar.test.tsx b/apps/desktop/src/renderer/components/chat/ChatGitToolbar.test.tsx index 763a8013ea..7fee07cb72 100644 --- a/apps/desktop/src/renderer/components/chat/ChatGitToolbar.test.tsx +++ b/apps/desktop/src/renderer/components/chat/ChatGitToolbar.test.tsx @@ -100,6 +100,7 @@ function LocationProbe() { function renderToolbar(props: { onTogglePrPane?: () => void; prPaneOpen?: boolean; + runtimePin?: any; } = {}) { return render( @@ -149,11 +150,76 @@ describe("ChatGitToolbar", () => { renderToolbar(); - await waitFor(() => expect(window.ade.prs.getForLane).toHaveBeenCalledWith("lane-1")); + await waitFor(() => expect(window.ade.prs.getForLane).toHaveBeenCalledWith("lane-1", null)); expect(window.ade.diff.getChanges).not.toHaveBeenCalled(); }); + // The bug: every PR read routed to the machine the project TAB is bound to, so + // a chat whose lane lives on another machine found no PR and showed the bare + // "PR" create button for a session that already had one. + it("routes PR reads to the lane's own machine when the chat is pinned", async () => { + const runtimePin = { + kind: "remote", + key: "remote:target-b:project-b", + targetId: "target-b", + projectId: "project-b", + runtimeName: "Machine B", + displayName: "Repo B", + rootPath: "/repo-b", + }; + vi.mocked(window.ade.prs.getForLane).mockResolvedValue({ + id: "pr-foreign", + laneId: "lane-1", + githubPrNumber: 91, + githubUrl: "https://github.com/acme/repo/pull/91", + state: "open", + checksStatus: "passing", + reviewStatus: "approved", + } as any); + + renderToolbar({ runtimePin }); + + await waitFor(() => expect(window.ade.prs.getForLane) + .toHaveBeenCalledWith("lane-1", runtimePin)); + // The event feed has to come from the same machine, or the pill goes stale. + expect(window.ade.prs.onEvent).toHaveBeenCalledWith(expect.any(Function), runtimePin); + // The pill renders the foreign machine's PR — before the fix this surface + // showed the bare "PR" create button for a session that already had one. + // (Where the click goes is `openLanePr`'s contract, pinned directly in + // lib/lanePrBadge.test.ts.) + expect(await screen.findByRole("button", { name: /#91/ })).toBeTruthy(); + expect(screen.queryByRole("button", { name: "PR" })).toBeNull(); + // The dirty-file read is NOT pin-aware, so for a foreign lane it would only + // ask the bound machine about a lane it does not have. + expect(window.ade.diff.getChanges).not.toHaveBeenCalled(); + }); + + // On the pinned path `prs.onEvent` is a polling pump that re-anchors to the + // live head on every subscribe, so a subscription keyed on the PR row dropped + // whatever the runtime buffered in the teardown gap. + it("keeps its PR event subscription across PR row changes", async () => { + renderToolbar(); + await waitFor(() => expect(window.ade.prs.getForLane).toHaveBeenCalled()); + const subscriptionsAfterMount = vi.mocked(window.ade.prs.onEvent).mock.calls.length; + + vi.mocked(window.ade.prs.getForLane).mockResolvedValue({ + id: "pr-1", + laneId: "lane-1", + githubPrNumber: 7, + githubUrl: "https://github.com/acme/repo/pull/7", + state: "open", + checksStatus: "passing", + reviewStatus: "approved", + } as any); + await act(async () => { + emitPrEvent({ type: "prs-updated", prs: [{ id: "pr-1", laneId: "lane-1" }] }); + }); + + expect(await screen.findByRole("button", { name: /#7/ })).toBeTruthy(); + expect(vi.mocked(window.ade.prs.onEvent).mock.calls.length).toBe(subscriptionsAfterMount); + }); + it("shows native GitHub stack position in the chat PR badge", async () => { vi.mocked(window.ade.prs.getForLane).mockResolvedValue({ id: "pr-stack", @@ -204,7 +270,7 @@ describe("ChatGitToolbar", () => { await waitFor(() => { expect(screen.getByTestId("location").textContent).toBe("/prs?tab=normal&prId=pr-1"); }); - expect(window.ade.prs.getForLane).toHaveBeenCalledWith("lane-1"); + expect(window.ade.prs.getForLane).toHaveBeenCalledWith("lane-1", null); expect(window.ade.diff.getChanges).not.toHaveBeenCalled(); }); @@ -281,7 +347,7 @@ describe("ChatGitToolbar", () => { fireEvent.click(badge.closest("button")!); await waitFor(() => { - expect(window.ade.prs.refresh).toHaveBeenCalledWith({ prIds: ["pr-1"] }); + expect(window.ade.prs.refresh).toHaveBeenCalledWith({ prIds: ["pr-1"] }, null); }); expect(await screen.findByText("MERGED #333")).toBeTruthy(); }); @@ -463,7 +529,7 @@ describe("ChatGitToolbar", () => { fireEvent.click((await screen.findByText("PR #111")).closest("button")!); await waitFor(() => { - expect(window.ade.prs.refresh).toHaveBeenCalledWith({ prIds: ["pr-lane-1"] }); + expect(window.ade.prs.refresh).toHaveBeenCalledWith({ prIds: ["pr-lane-1"] }, null); }); fireEvent.click(screen.getByRole("button", { name: "Switch lane" })); diff --git a/apps/desktop/src/renderer/components/chat/ChatGitToolbar.tsx b/apps/desktop/src/renderer/components/chat/ChatGitToolbar.tsx index 5868949c1a..ef1afeb99d 100644 --- a/apps/desktop/src/renderer/components/chat/ChatGitToolbar.tsx +++ b/apps/desktop/src/renderer/components/chat/ChatGitToolbar.tsx @@ -14,7 +14,7 @@ import { } from "@phosphor-icons/react"; import { AnimatePresence, motion } from "motion/react"; import { cn } from "../ui/cn"; -import type { DiffChanges, PrSummary, PrCheck } from "../../../shared/types"; +import type { DiffChanges, OpenProjectBinding, PrSummary, PrCheck } from "../../../shared/types"; import { armLaneBranchDriftWarning } from "../lanes/LaneBranchDrift"; import { useLaneGitActionRuntimeState } from "../lanes/LaneGitActionsPane"; import { formatPrBadgeLabel } from "../prs/shared/prFormatters"; @@ -23,6 +23,7 @@ import { useAppStore } from "../../state/appStore"; import { refreshLinkedPrCoalesced } from "../../lib/prReadCache"; import { rollupPrChecks } from "../../../shared/prChecksRollup"; import type { PrChecksStatus } from "../../../shared/types/prs"; +import { openLanePr } from "../../lib/lanePrBadge"; import { GitHubStackBadge } from "../prs/shared/GitHubStackBadge"; // --------------------------------------------------------------------------- @@ -38,6 +39,13 @@ type ChatGitToolbarProps = { */ onTogglePrPane?: () => void; prPaneOpen?: boolean; + /** + * The machine this lane lives on, when it is not the machine the project tab + * is bound to. A lane's PR record lives in its own machine's database, so + * without this the pill reads the bound machine's rows, finds nothing, and + * shows the bare "PR" create button for a session that already has one. + */ + runtimePin?: OpenProjectBinding | null; }; // --------------------------------------------------------------------------- @@ -127,6 +135,7 @@ export const ChatGitToolbar = React.memo(function ChatGitToolbar({ laneId, onTogglePrPane, prPaneOpen, + runtimePin = null, }: ChatGitToolbarProps) { const navigate = useNavigate(); const runtime = useLaneGitActionRuntimeState(laneId); @@ -144,6 +153,21 @@ export const ChatGitToolbar = React.memo(function ChatGitToolbar({ const laneIdRef = React.useRef(laneId); const refreshPrRequestRef = React.useRef(0); laneIdRef.current = laneId; + // Read inside the event handler through a ref, never as an effect dep. On the + // PINNED path `prs.onEvent` is a polling pump that re-anchors to the live head + // on every subscribe (`suppressReplay`), so making the subscription depend on + // the PR row — which every event replaces — drops whatever the runtime + // buffered in the teardown gap. ChatPrPane keeps its subscription stable the + // same way. + const linkedPrRef = React.useRef(null); + linkedPrRef.current = linkedPr; + // Effects key on the pin's KEY, never its object identity: a local pin is + // reconstructed on every cross-machine merge (~10s), and depending on the + // object made the reset effect blank the pill and the pinned event pump + // re-anchor on that timer. The object itself is read through this ref. + const runtimePinRef = React.useRef(runtimePin); + runtimePinRef.current = runtimePin; + const runtimePinKey = runtimePin?.key ?? null; // ----------------------------------------------------------------------- // Refresh git status + PR link @@ -163,13 +187,13 @@ export const ChatGitToolbar = React.memo(function ChatGitToolbar({ refreshPrRequestRef.current = requestId; const requestIsCurrent = () => laneIdRef.current === laneId && refreshPrRequestRef.current === requestId; try { - const pr = await window.ade.prs.getForLane(laneId); + const pr = await window.ade.prs.getForLane(laneId, runtimePinRef.current); if (!requestIsCurrent()) return null; setLinkedPr(pr); setPrLoaded(true); if (options.live && pr && !pr.unmapped) { try { - const refreshed = await refreshLinkedPrCoalesced(pr, { projectRoot }); + const refreshed = await refreshLinkedPrCoalesced(pr, { projectRoot, pin: runtimePinRef.current }); if (!requestIsCurrent()) return null; setLinkedPr(refreshed); return refreshed; @@ -185,7 +209,11 @@ export const ChatGitToolbar = React.memo(function ChatGitToolbar({ } return null; } - }, [laneId, projectRoot]); + // `runtimePinKey` is read through `runtimePinRef`, so the linter cannot see + // it — but a callback that reads machine A must not be reused as if it reads + // machine B, and its identity is what re-runs the effects below. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [laneId, projectRoot, runtimePinKey]); useEffect(() => { setDirtyCount(0); @@ -193,19 +221,21 @@ export const ChatGitToolbar = React.memo(function ChatGitToolbar({ setPrLoaded(false); setPrMenuOpen(false); setPrChecks(null); - if (!isRemoteProject) void refreshStatus(); + // `diff.getChanges` is not pinned, so for a lane on another machine it can + // only ask the bound machine about a lane it does not have. Skip it. + if (!isRemoteProject && !runtimePinKey) void refreshStatus(); void refreshPr(); - }, [isRemoteProject, refreshStatus, refreshPr]); + }, [isRemoteProject, refreshStatus, refreshPr, runtimePinKey]); // Re-poll after the runtime finishes an action (from either pane or toolbar) const prevBusy = React.useRef(runtime.busyAction); useEffect(() => { if (prevBusy.current && !runtime.busyAction) { - if (!isRemoteProject) void refreshStatus(); + if (!isRemoteProject && !runtimePinKey) void refreshStatus(); void refreshPr(); } prevBusy.current = runtime.busyAction; - }, [isRemoteProject, runtime.busyAction, refreshStatus, refreshPr]); + }, [isRemoteProject, runtime.busyAction, refreshStatus, refreshPr, runtimePinKey]); // Backend reconcile-on-focus, in its OWN subscription keyed only on stable // deps (laneId/projectRoot via refreshPr) — NOT linkedPr, so the idle branch's @@ -218,49 +248,58 @@ export const ChatGitToolbar = React.memo(function ChatGitToolbar({ if (event.state === "running") return; // A reconcile just healed backend state — re-read the linked PR. void refreshPr(); - }); + }, runtimePinRef.current); return () => { unsubscribe(); }; - }, [refreshPr]); + }, [refreshPr, runtimePinKey]); // Subscribe to backend PR events so the linked-PR pill reflects external // changes (PR closed, merged, checks finished, etc.) without a manual refresh. useEffect(() => { const unsubscribe = window.ade.prs.onEvent((event) => { + const current = linkedPrRef.current; if (event.type === "pr-notification") { - if (event.laneId === laneId || event.prId === linkedPr?.id) void refreshPr(); + if (event.laneId === laneId || event.prId === current?.id) void refreshPr(); return; } if (event.type !== "prs-updated") return; const eventIncludesLanePr = event.prs.some((pr) => pr.laneId === laneId); - const eventIncludesLinkedPr = linkedPr ? event.prs.some((pr) => pr.id === linkedPr.id) : false; + const eventIncludesLinkedPr = current ? event.prs.some((pr) => pr.id === current.id) : false; if (eventIncludesLanePr || eventIncludesLinkedPr) { void refreshPr(); - } else if (linkedPr) { + } else if (current) { // The linked PR vanished from the latest snapshot — clear the pill. setLinkedPr(null); } - }); + }, runtimePinRef.current); return () => { unsubscribe(); }; - }, [laneId, linkedPr, refreshPr]); + }, [laneId, refreshPr, runtimePinKey]); const handlePr = useCallback(async () => { // A PR operation is about to run against this worktree — arm the drift // warning strip so a wrong-branch PR is caught before it is opened. armLaneBranchDriftWarning(laneId); - const openPr = (prId: string) => { - navigate(`/prs${buildPrsRouteSearch({ - activeTab: "normal", - selectedPrId: prId, - selectedLaneId: laneId, - selectedRebaseItemId: null, - })}`); + const openPr = (pr: PrSummary) => { + // The PRs tab resolves a PR id against the bound machine only, so a lane + // on another machine goes to GitHub — the one destination that means the + // same thing from either machine. The local branch keeps this surface's + // richer route (it also selects the lane), so it passes its own path. + openLanePr(pr, { + foreign: Boolean(runtimePin), + navigate, + localPath: `/prs${buildPrsRouteSearch({ + activeTab: "normal", + selectedPrId: pr.id, + selectedLaneId: laneId, + selectedRebaseItemId: null, + })}`, + }); }; if (linkedPr) { - openPr(linkedPr.id); + openPr(linkedPr); return; } @@ -268,11 +307,16 @@ export const ChatGitToolbar = React.memo(function ChatGitToolbar({ setPrActionBusy(true); const latestPr = await refreshPr().finally(() => setPrActionBusy(false)); if (latestPr) { - openPr(latestPr.id); + openPr(latestPr); return; } } + // Creating a PR is a write against the lane's worktree and the create form + // derives its branches from the bound machine's lanes, so it is offered only + // on the machine that owns the lane. Reading one is not so restricted. + if (runtimePin) return; + const params = new URLSearchParams({ tab: "normal", create: "1", @@ -280,7 +324,7 @@ export const ChatGitToolbar = React.memo(function ChatGitToolbar({ target: "primary", }); navigate(`/prs?${params.toString()}`); - }, [laneId, linkedPr, navigate, prLoaded, refreshPr]); + }, [laneId, linkedPr, navigate, prLoaded, refreshPr, runtimePin]); const handlePrClick = useCallback(() => { if (prActionBusy) return; @@ -307,7 +351,7 @@ export const ChatGitToolbar = React.memo(function ChatGitToolbar({ if (!prMenuOpen || !linkedPr || linkedPr.unmapped) return; let cancelled = false; setPrChecksLoading(true); - window.ade.prs.getChecks(linkedPr.id) + window.ade.prs.getChecks(linkedPr.id, runtimePinRef.current) .then((checks) => { if (!cancelled) setPrChecks(checks); }) @@ -320,7 +364,7 @@ export const ChatGitToolbar = React.memo(function ChatGitToolbar({ return () => { cancelled = true; }; - }, [prMenuOpen, linkedPr]); + }, [prMenuOpen, linkedPr, runtimePinKey]); // Reset the copy-confirmed checkmark a moment after it's shown. useEffect(() => { @@ -332,8 +376,9 @@ export const ChatGitToolbar = React.memo(function ChatGitToolbar({ const handleOpenInAde = useCallback(() => { if (!linkedPr) return; setPrMenuOpen(false); - navigate(`/prs?tab=normal&prId=${encodeURIComponent(linkedPr.id)}`); - }, [linkedPr, navigate]); + // Same rule as the pill: a PR id only resolves on the machine that owns it. + openLanePr(linkedPr, { foreign: Boolean(runtimePin), navigate }); + }, [linkedPr, navigate, runtimePin]); const handleOpenInGitHub = useCallback(async () => { if (!linkedPr) return; diff --git a/apps/desktop/src/renderer/components/chat/ChatPrPane.test.tsx b/apps/desktop/src/renderer/components/chat/ChatPrPane.test.tsx index 9aa7ad5f6a..7d864349c3 100644 --- a/apps/desktop/src/renderer/components/chat/ChatPrPane.test.tsx +++ b/apps/desktop/src/renderer/components/chat/ChatPrPane.test.tsx @@ -246,7 +246,7 @@ describe("ChatPrPane", () => { expect(screen.queryByText("Checks running")).toBeNull(); await waitFor(() => { - expect((window.ade.prs.refresh as ReturnType)).toHaveBeenCalledWith({ prIds: ["pr-333"] }); + expect((window.ade.prs.refresh as ReturnType)).toHaveBeenCalledWith({ prIds: ["pr-333"] }, null); }); // Exactly two stable subscriptions: PR rows + reconcile-on-focus. Neither // may be torn down and re-created as PR state changes. @@ -454,7 +454,7 @@ describe("ChatPrPane title bar", () => { fireEvent.click(screen.getByRole("button", { name: "Refresh pull request" })); await waitFor(() => { - expect(window.ade.prs.syncLanePr).toHaveBeenCalledWith("lane1"); + expect(window.ade.prs.syncLanePr).toHaveBeenCalledWith("lane1", null); }); await waitFor(() => { expect((window.ade.prs.getForLane as ReturnType).mock.calls.length) diff --git a/apps/desktop/src/renderer/components/chat/ChatPrPane.tsx b/apps/desktop/src/renderer/components/chat/ChatPrPane.tsx index 545b76fa2a..1c26649cb5 100644 --- a/apps/desktop/src/renderer/components/chat/ChatPrPane.tsx +++ b/apps/desktop/src/renderer/components/chat/ChatPrPane.tsx @@ -18,13 +18,14 @@ import { XCircle, } from "@phosphor-icons/react"; import { cn } from "../ui/cn"; -import type { PrCheck, PrReview, PrState, PrStatus, PrSummary } from "../../../shared/types"; +import type { OpenProjectBinding, PrCheck, PrReview, PrState, PrStatus, PrSummary } from "../../../shared/types"; import { formatPrBadgeLabel } from "../prs/shared/prFormatters"; import { PrUserAvatar } from "../prs/shared/PrUserAvatar"; import { ChatPrInlineCreator } from "./ChatPrInlineCreator"; import { refreshLinkedPrCoalesced } from "../../lib/prReadCache"; -import { useAppStore } from "../../state/appStore"; +import { useAppStore, useRootAppStore } from "../../state/appStore"; import { pipelineStateOf } from "../../../shared/prPipelineState"; +import { openLanePr } from "../../lib/lanePrBadge"; import { GitHubStackBadge } from "../prs/shared/GitHubStackBadge"; import { NO_CI_REASON } from "../../../shared/prChecksRollup"; @@ -416,6 +417,7 @@ export const ChatPrPane = React.memo(function ChatPrPane({ sessionTitle = null, delta = null, onClose, + runtimePin = null, }: { laneId: string; branchName?: string | null; @@ -429,9 +431,22 @@ export const ChatPrPane = React.memo(function ChatPrPane({ delta?: ChatPrDelta | null; /** Closes the pane — wired to the title bar's ✕ (the header PR pill also toggles it). */ onClose?: () => void; + /** See `ChatGitToolbar.runtimePin` — the machine this lane's PR row lives on. */ + runtimePin?: OpenProjectBinding | null; }) { const navigate = useNavigate(); const projectRoot = useAppStore((s) => s.project?.rootPath ?? s.projectBinding?.rootPath ?? null); + // See `ChatGitToolbar`: a local pin is a fresh object on every cross-machine + // merge, so effects key on the stable pin key and read the object via a ref. + const runtimePinRef = useRef(runtimePin); + runtimePinRef.current = runtimePin; + const runtimePinKey = runtimePin?.key ?? null; + const machinesById = useRootAppStore((s) => s.crossMachineLanesByMachineId); + const pinMachineName = useMemo(() => { + if (!runtimePinKey) return null; + return Object.values(machinesById) + .find((machine) => machine.binding?.key === runtimePinKey)?.machineName ?? null; + }, [machinesById, runtimePinKey]); const [pr, setPr] = useState(null); const [loading, setLoading] = useState(true); const [copied, setCopied] = useState(false); @@ -463,12 +478,12 @@ export const ChatPrPane = React.memo(function ChatPrPane({ const requestIsCurrent = () => laneIdRef.current === laneId && refreshRequestRef.current === requestId; let cached: PrSummary | null = null; try { - cached = await window.ade.prs.getForLane(laneId); + cached = await window.ade.prs.getForLane(laneId, runtimePinRef.current); if (!requestIsCurrent()) return; setCurrentPr(cached); setLoading(false); if (options.live && cached && !cached.unmapped) { - const refreshed = await refreshLinkedPrCoalesced(cached, { projectRoot }); + const refreshed = await refreshLinkedPrCoalesced(cached, { projectRoot, pin: runtimePinRef.current }); if (!requestIsCurrent()) return; setCurrentPr(refreshed); } @@ -477,7 +492,10 @@ export const ChatPrPane = React.memo(function ChatPrPane({ } finally { if (requestIsCurrent()) setLoading(false); } - }, [laneId, projectRoot, setCurrentPr]); + // See ChatGitToolbar: read via ref, but the identity must still follow the + // pin so the effects keyed on it re-read from the new machine. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [laneId, projectRoot, runtimePinKey, setCurrentPr]); // The inline creator hands us the freshly-created PR the moment createFromLane // resolves — swap to the details view instantly rather than waiting for the @@ -496,7 +514,7 @@ export const ChatPrPane = React.memo(function ChatPrPane({ if (syncing) return; setSyncing(true); try { - await window.ade.prs.syncLanePr(laneId); + await window.ade.prs.syncLanePr(laneId, runtimePinRef.current); } catch { // best-effort } finally { @@ -534,11 +552,11 @@ export const ChatPrPane = React.memo(function ChatPrPane({ // A reconcile just healed backend state — re-read the lane's PR. void refresh(); } - }); + }, runtimePinRef.current); return () => { unsubscribe(); }; - }, [refresh]); + }, [refresh, runtimePinKey]); // Clear the reconcile hide timer ONLY on unmount — never on a re-subscribe — // so the debounce can't be stranded mid-flight. @@ -566,9 +584,9 @@ export const ChatPrPane = React.memo(function ChatPrPane({ } else { setCurrentPr(null); } - }); + }, runtimePinRef.current); return unsubscribe; - }, [laneId, refresh, setCurrentPr]); + }, [laneId, refresh, runtimePinKey, setCurrentPr]); // Hot-refresh enriched detail (checks / reviews / merge status) whenever this // PR's content changes — driven by the relay's `prs-updated`, not a timer. @@ -594,9 +612,9 @@ export const ChatPrPane = React.memo(function ChatPrPane({ const prId = pr.id; let cancelled = false; void Promise.allSettled([ - window.ade.prs.getChecks(prId), - window.ade.prs.getReviews(prId), - window.ade.prs.getStatus(prId), + window.ade.prs.getChecks(prId, runtimePinRef.current), + window.ade.prs.getReviews(prId, runtimePinRef.current), + window.ade.prs.getStatus(prId, runtimePinRef.current), ]).then(([c, r, s]) => { if (cancelled) return; if (c.status === "fulfilled") setChecks(c.value); @@ -604,7 +622,7 @@ export const ChatPrPane = React.memo(function ChatPrPane({ if (s.status === "fulfilled") setStatus(s.value); }); return () => { cancelled = true; }; - }, [pr]); + }, [pr, runtimePinKey]); // Best-effort: is the webhook relay actually connected for this repo? Drives // the live/stale/offline dot so the pane reflects real webhook status. @@ -637,10 +655,13 @@ export const ChatPrPane = React.memo(function ChatPrPane({ return () => window.clearTimeout(id); }, [copied]); + // Same rule the sidebar badge follows: a PR id only resolves on the machine + // that owns it, so a pinned pane's "Open in ADE" would land on an empty PRs + // tab. `openLanePr` sends a foreign PR to GitHub instead. const openInAde = useCallback(() => { if (!pr) return; - navigate(`/prs?tab=normal&prId=${encodeURIComponent(pr.id)}`); - }, [pr, navigate]); + openLanePr(pr, { foreign: Boolean(runtimePin), navigate }); + }, [pr, navigate, runtimePin]); const openInGitHub = useCallback(async () => { if (!pr) return; @@ -719,6 +740,18 @@ export const ChatPrPane = React.memo(function ChatPrPane({ onOpenGitHub={() => void openInGitHub()} onCopy={() => void copyLink()} /> + ) : runtimePin ? ( + // Reading a foreign lane's PR is now routed to its machine; CREATING + // one is not. The creator derives its branch, base and Linear link + // from `state.lanes` — the bound machine's lanes, which do not contain + // this lane — and `createFromLane` is unpinned, so it would run + // against the wrong machine. Say so instead of offering a button that + // cannot work. +

+ No pull request yet. +
+ Switch to {pinMachineName ?? "this chat's machine"} to open one. +

) : ( (runtimePin); + runtimePinRef.current = runtimePin; + const runtimePinKey = runtimePin?.key ?? null; const [prPaneOpen, setPrPaneOpen] = useState( () => (persistKey ? readChatCompanionUiState(persistKey).prPaneOpen : false), ); @@ -87,7 +96,7 @@ export function useChatPrAutoPop( if (!laneId || !prs?.getForLane || !prs?.onEvent) return; let cancelled = false; prs - .getForLane(laneId) + .getForLane(laneId, runtimePinRef.current) .then((pr) => { if (!cancelled && prevPrSigRef.current === null) prevPrSigRef.current = chatPrSignature(pr); }) @@ -102,12 +111,12 @@ export function useChatPrAutoPop( if (!change) return; setPrPaneDelta({ ...change, nonce: ++nonceRef.current }); setPrPaneOpen(true); - }); + }, runtimePinRef.current); return () => { cancelled = true; unsubscribe(); }; - }, [laneId]); + }, [laneId, runtimePinKey]); return { prPaneOpen, setPrPaneOpen, prPaneDelta }; } 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 ddcb5a242e..50e12d4be4 100644 --- a/apps/desktop/src/renderer/components/prs/state/PrsContext.test.tsx +++ b/apps/desktop/src/renderer/components/prs/state/PrsContext.test.tsx @@ -249,7 +249,7 @@ describe("PrsContext refresh", () => { expect(window.ade.rebase.scanNeeds).not.toHaveBeenCalled(); expect(window.ade.lanes.listAutoRebaseStatuses).not.toHaveBeenCalled(); expect(window.ade.lanes.list).toHaveBeenCalledWith({ includeStatus: false }); - expect(window.ade.prs.refresh).toHaveBeenCalledWith({}); + expect(window.ade.prs.refresh).toHaveBeenCalledWith({}, null); }); it("replays a hidden lane lifecycle refresh when the PRs tab becomes visible", async () => { diff --git a/apps/desktop/src/renderer/components/terminals/CliSessionWorkSurfaceHeader.tsx b/apps/desktop/src/renderer/components/terminals/CliSessionWorkSurfaceHeader.tsx index bdeae11aef..88dc148c57 100644 --- a/apps/desktop/src/renderer/components/terminals/CliSessionWorkSurfaceHeader.tsx +++ b/apps/desktop/src/renderer/components/terminals/CliSessionWorkSurfaceHeader.tsx @@ -1,7 +1,7 @@ import type { MouseEvent as ReactMouseEvent } from "react"; import { DotsThreeVertical, Info, StopCircle } from "@phosphor-icons/react"; import { useNavigate } from "react-router-dom"; -import type { LaneSummary, TerminalSessionSummary } from "../../../shared/types"; +import type { LaneSummary, OpenProjectBinding, TerminalSessionSummary } from "../../../shared/types"; import { openLaneInLanesTabPath } from "../../lib/laneNavigation"; import { shouldShowClaudeCacheTtl } from "../../lib/claudeCacheTtl"; import { formatToolTypeLabel, primarySessionLabel, truncateSessionLabel } from "../../lib/sessions"; @@ -190,16 +190,19 @@ export function GridTileSessionHeaderActions({ onInfoClick, onContextMenu, onStopRunningSession, + runtimePin = null, }: { session: TerminalSessionSummary; stopping?: boolean; onInfoClick?: SessionMouseHandler; onContextMenu?: SessionMouseHandler; onStopRunningSession?: (session: TerminalSessionSummary) => void; + /** See `ChatGitToolbar.runtimePin`. */ + runtimePin?: OpenProjectBinding | null; }) { return (
- {session.laneId ? : null} + {session.laneId ? : null} @@ -231,6 +234,7 @@ export function CliSessionWorkSurfaceHeader({ toolsPaneOpen, onTogglePrPane, prPaneOpen, + runtimePin = null, }: { session: TerminalSessionSummary; lanes: LaneSummary[]; @@ -248,6 +252,8 @@ export function CliSessionWorkSurfaceHeader({ * instead of opening the inline slide-out menu. */ onTogglePrPane?: () => void; prPaneOpen?: boolean; + /** See `ChatGitToolbar.runtimePin`. */ + runtimePin?: OpenProjectBinding | null; }) { const navigate = useNavigate(); const lane = lanes.find((entry) => entry.id === session.laneId) ?? null; @@ -282,6 +288,7 @@ export function CliSessionWorkSurfaceHeader({ showGitToolbar onTogglePrPane={onTogglePrPane} prPaneOpen={prPaneOpen} + runtimePin={runtimePin} onContextMenu={ onContextMenu ? (event) => { diff --git a/apps/desktop/src/renderer/components/terminals/LanePrBadge.tsx b/apps/desktop/src/renderer/components/terminals/LanePrBadge.tsx index 0e4c92ea18..74216c8e41 100644 --- a/apps/desktop/src/renderer/components/terminals/LanePrBadge.tsx +++ b/apps/desktop/src/renderer/components/terminals/LanePrBadge.tsx @@ -51,8 +51,3 @@ export function LanePrBadge({ pr, onOpen }: { pr: PrSummary; onOpen: () => void ); } - -/** Deep link the PR chip opens, identical from every host so the target cannot drift. */ -export function lanePrDeepLinkPath(pr: PrSummary): string { - return `/prs?tab=normal&prId=${encodeURIComponent(pr.id)}`; -} diff --git a/apps/desktop/src/renderer/components/terminals/SessionCard.tsx b/apps/desktop/src/renderer/components/terminals/SessionCard.tsx index 2e1552c291..a810fa5936 100644 --- a/apps/desktop/src/renderer/components/terminals/SessionCard.tsx +++ b/apps/desktop/src/renderer/components/terminals/SessionCard.tsx @@ -45,9 +45,9 @@ import { useSessionDelta } from "./useSessionDelta"; import { cn } from "../ui/cn"; import { MONO_FONT } from "../lanes/laneDesignTokens"; import { BranchIcon, LaneIcon } from "../ui/vcsIcons"; -import { LanePrBadge, lanePrDeepLinkPath } from "./LanePrBadge"; +import { LanePrBadge } from "./LanePrBadge"; import { branchNameFromRef } from "../prs/shared/laneBranchTargets"; -import { lanePrStateColor, lanePrStateLabel } from "../../lib/lanePrBadge"; +import { lanePrStateColor, lanePrStateLabel, openLanePr } from "../../lib/lanePrBadge"; import { SessionHoverCard, useSessionHoverCard, @@ -297,6 +297,7 @@ export const SessionCard = React.memo(function SessionCard({ githubStack = null, showLaneIdentity = false, lanePr = null, + lanePrForeign = false, machineMarker = null, suppressMachineChip = false, }: { @@ -334,6 +335,16 @@ export const SessionCard = React.memo(function SessionCard({ * divider owns the PR badge and a second copy per row would be noise. */ lanePr?: PrSummary | null; + /** + * True when this card's lane lives on another machine. The PR itself is read + * from that machine; only its click-through has to change, because the PRs tab + * cannot resolve a PR id that is not on the bound machine. See `openLanePr`. + * + * Deliberately NOT derived from `runtimePin`: an unreachable machine's row has + * a null binding and therefore no pin, while still being foreign. Deriving it + * would send exactly those rows back to a PRs tab that cannot resolve them. + */ + lanePrForeign?: boolean; /** * The machine this row's lane lives on, when that is not the Mac you're * sitting at. Resolved once by the cross-machine union and handed down, so a @@ -728,7 +739,7 @@ export const SessionCard = React.memo(function SessionCard({ ), - onActivate: () => navigate(lanePrDeepLinkPath(lanePr)), + onActivate: () => openLanePr(lanePr, { foreign: lanePrForeign, navigate }), activateLabel: `Open pull request #${lanePr.githubPrNumber}`, testId: "session-hover-pr", }); @@ -860,7 +871,7 @@ export const SessionCard = React.memo(function SessionCard({ const singletonPrBadge = showLaneIdentity && lanePr ? ( navigate(lanePrDeepLinkPath(lanePr))} + onOpen={() => openLanePr(lanePr, { foreign: lanePrForeign, navigate })} /> ) : null; diff --git a/apps/desktop/src/renderer/components/terminals/SessionListPane.test.tsx b/apps/desktop/src/renderer/components/terminals/SessionListPane.test.tsx index 5597fbbfa2..b29503c801 100644 --- a/apps/desktop/src/renderer/components/terminals/SessionListPane.test.tsx +++ b/apps/desktop/src/renderer/components/terminals/SessionListPane.test.tsx @@ -12,6 +12,7 @@ import { useAppStore, type CrossMachineMachineLanes } from "../../state/appStore import { resetCrossMachineLaneSyncForTest } from "../../state/crossMachineLanes"; import { setLaneNaming } from "../../state/laneNamingStore"; import { SessionListPane } from "./SessionListPane"; +import { laneBoundMachineKey, lanePrCompositeKey } from "./useLanePrs"; import { ADE_WORK_LANE_DND_MIME } from "./workLaneOrder"; import { EMPTY_WORK_SESSION_FILTERS } from "./workSessionFilters"; @@ -52,7 +53,10 @@ function cardPropsFor(sessionId: string): Record | undefined { const { lanePrsByLaneIdForTest } = vi.hoisted(() => ({ lanePrsByLaneIdForTest: new Map(), })); -vi.mock("./useLanePrs", () => ({ +// Only the hook is stubbed; the key/accessor helpers stay REAL so the test seeds +// its map with the same key discipline the component reads it back with. +vi.mock("./useLanePrs", async (importOriginal) => ({ + ...(await importOriginal()), useLanePrsByLaneId: () => lanePrsByLaneIdForTest, })); @@ -1247,6 +1251,7 @@ describe("SessionListPane", () => { title: "Chat on the other machine", }), ], + prs: [], lastSyncedAtMs: 1, error: null, ...overrides, @@ -1378,6 +1383,34 @@ describe("SessionListPane", () => { ); }); + // The bug: a foreign row's PR was looked up against the machine the project + // TAB is bound to, so a session doing work on another machine showed no PR + // badge until you switched the global machine selector to that machine. + it("renders a foreign lane's PR badge from its own machine's answer", () => { + seedForeignMachine(); + lanePrsByLaneIdForTest.set( + lanePrCompositeKey("target-studio", "lane-elsewhere"), + [makePr({ laneId: "lane-elsewhere", githubPrNumber: 91, headBranch: "feature/elsewhere" })], + ); + renderPane(); + + expect(screen.getByText("#91")).toBeTruthy(); + }); + + // The other half of the same bug: the bound machine's rows must not answer + // for a foreign lane either. Cross-machine handoff copies a lane id, so a + // bare-lane-id lookup would render the wrong machine's PR here. + it("does not borrow the bound machine's PR for a foreign lane", () => { + seedForeignMachine(); + lanePrsByLaneIdForTest.set( + laneBoundMachineKey("lane-elsewhere"), + [makePr({ laneId: "lane-elsewhere", githubPrNumber: 77, headBranch: "feature/elsewhere" })], + ); + renderPane(); + + expect(screen.queryByText("#77")).toBeNull(); + }); + it("keeps a foreign lane's divider, and its menu, once it holds two chats", () => { seedForeignMachine({ sessions: [ @@ -2008,7 +2041,10 @@ describe("SessionListPane singleton lanes and shelves", () => { it("hands the lane's PR badge to the lone card once the divider is gone", () => { // PR state has to survive everywhere the lane header is minimized or // absent; the singleton form was the last hole. - lanePrsByLaneIdForTest.set("lane-solo", [makePr({ laneId: "lane-solo", headBranch: "solo-lane" })]); + lanePrsByLaneIdForTest.set( + laneBoundMachineKey("lane-solo"), + [makePr({ laneId: "lane-solo", headBranch: "solo-lane" })], + ); const session = soloSession(); const { container } = renderPane({ lanes: [soloLane], @@ -2495,7 +2531,7 @@ describe("SessionListPane header shape", () => { }); it("keeps the PR badge on a collapsed quiet lane header", () => { - lanePrsByLaneIdForTest.set("lane-known", [makePr()]); + lanePrsByLaneIdForTest.set(laneBoundMachineKey("lane-known"), [makePr()]); const snoozed = makeSession({ id: "session-pr-snoozed", laneId: "lane-known", @@ -2749,6 +2785,7 @@ describe("SessionListPane visual hierarchy", () => { laneName: "Primary", title: "Primary chat elsewhere", })], + prs: [], lastSyncedAtMs: 1, error: null, }, @@ -2873,6 +2910,7 @@ describe("SessionListPane machine chip suppression", () => { online: true, lanes: [lane], sessions, + prs: [], lastSyncedAtMs: 1, error: null, }, diff --git a/apps/desktop/src/renderer/components/terminals/SessionListPane.tsx b/apps/desktop/src/renderer/components/terminals/SessionListPane.tsx index 2bc6bcfffb..0d10bc7e9a 100644 --- a/apps/desktop/src/renderer/components/terminals/SessionListPane.tsx +++ b/apps/desktop/src/renderer/components/terminals/SessionListPane.tsx @@ -4,10 +4,10 @@ import { ArrowClockwise, CaretDown, CaretRight, CircleNotch, Desktop, Funnel, Ma import { AnimatePresence, motion } from "motion/react"; import { BranchIcon, LaneIcon } from "../ui/vcsIcons"; import type { LaneSummary, OpenProjectBinding, PrSummary, TerminalSessionSummary } from "../../../shared/types"; -import { selectPrimaryLanePr } from "../../lib/lanePrBadge"; -import { LanePrBadge, lanePrDeepLinkPath } from "./LanePrBadge"; +import { openLanePr, selectPrimaryLanePr } from "../../lib/lanePrBadge"; +import { LanePrBadge } from "./LanePrBadge"; import type { SessionContextMenuLaneActions } from "./SessionContextMenu"; -import { useLanePrsByLaneId } from "./useLanePrs"; +import { boundMachineLanePrs, laneHasAnyPr, lanePrsForMachine, useLanePrsByLaneId } from "./useLanePrs"; import { canonicalInputFromSummary, sessionFilingBucket, @@ -1051,7 +1051,7 @@ export const SessionListPane = React.memo(function SessionListPane({ workSessionFilters.tool.length > 0 && !workSessionFilters.tool.includes(workToolFamily(job.targetToolType)) ) return false; - if (workSessionFilters.hasPr && (prsByLaneId.get(job.laneId)?.length ?? 0) === 0) return false; + if (workSessionFilters.hasPr && !laneHasAnyPr(prsByLaneId, job.laneId)) return false; if (workSessionFilters.dirtyLane && !lanes.find((lane) => lane.id === job.laneId)?.status.dirty) { return false; } @@ -1778,8 +1778,17 @@ export const SessionListPane = React.memo(function SessionListPane({ if (isFirst) sessionItemAnchorEmitted = true; const foreignRow = options?.foreignRow; const sessionLane = foreignRow?.lane ?? laneById.get(session.laneId) ?? null; - const sessionPr = !foreignRow && sessionLane - ? selectPrimaryLanePr(sessionLane, prsByLaneId.get(session.laneId) ?? []) + // A PR belongs to the lane, and the lane belongs to a machine — so the PR + // is read from that machine and filed under its composite key. A foreign + // card is no longer excluded: it was only ever blank because the lookup + // could not reach past the tab's own binding. + const sessionPr = sessionLane + ? selectPrimaryLanePr( + sessionLane, + foreignRow + ? lanePrsForMachine(prsByLaneId, foreignRow.machineId, session.laneId) + : boundMachineLanePrs(prsByLaneId, session.laneId), + ) : null; // A card on an unreachable machine is shown as last reported and every // action on it would fail, so it is inert and says which machine is gone. @@ -1839,6 +1848,7 @@ export const SessionListPane = React.memo(function SessionListPane({ compact={options?.compact} showLaneIdentity={options?.showLaneIdentity} lanePr={options?.lanePr} + lanePrForeign={Boolean(foreignRow)} gridBadge={foreignRow ? null : gridBadgeFor(session.id)} runtimePin={foreignRow?.binding} machineMarker={options?.machineMarker ?? null} @@ -2215,9 +2225,12 @@ export const SessionListPane = React.memo(function SessionListPane({ {lane.icon ? iconGlyph(lane.icon) : } ); - const primaryPr = selectPrimaryLanePr(lane, prsByLaneId.get(lane.id) ?? []); + const primaryPr = selectPrimaryLanePr(lane, boundMachineLanePrs(prsByLaneId, lane.id)); const prBadge = primaryPr ? ( - navigate(lanePrDeepLinkPath(primaryPr))} /> + openLanePr(primaryPr, { foreign: false, navigate })} + /> ) : null; // Never populated for a lane on the Mac you're sitting at — the marker says // exactly one thing, "this work isn't here", and no lane type is exempt. @@ -2331,10 +2344,14 @@ export const SessionListPane = React.memo(function SessionListPane({ : workCollapsedLaneIds.includes(compositeLaneId); // A shelved lane shows nothing but its header, so the header has to keep // carrying everything that identifies the lane — the machine marker below, - // and the PR badge here. PR records are local to this runtime, so a lane - // that exists only elsewhere simply has none; when the local runtime does - // know the lane's PR, it is the same lane and the same badge. - const primaryPr = selectPrimaryLanePr(row.lane, prsByLaneId.get(row.lane.id) ?? []); + // and the PR badge here. The lookup is keyed by the row's OWN machine: a + // bare lane id would answer out of whichever machine happened to claim it, + // and cross-machine handoff makes that a real collision, not a theoretical + // one. + const primaryPr = selectPrimaryLanePr( + row.lane, + lanePrsForMachine(prsByLaneId, row.machineId, row.lane.id), + ); // A group WITH a header names the machine there, so its rows never repeat // it. A headerless group has no such header, so its lone card takes the // marker instead — the same trade `renderLaneGroup` makes. @@ -2372,7 +2389,10 @@ export const SessionListPane = React.memo(function SessionListPane({ headerless={headerless} accentColor={row.lane.color ?? null} prBadge={primaryPr ? ( - navigate(lanePrDeepLinkPath(primaryPr))} /> + openLanePr(primaryPr, { foreign: true, navigate })} + /> ) : null} machineMarker={headerMarker ? : null} quietCounts={laneQuiet && collapsed diff --git a/apps/desktop/src/renderer/components/terminals/WorkStartSurface.test.tsx b/apps/desktop/src/renderer/components/terminals/WorkStartSurface.test.tsx index 54b060e6d1..4e31c63cf5 100644 --- a/apps/desktop/src/renderer/components/terminals/WorkStartSurface.test.tsx +++ b/apps/desktop/src/renderer/components/terminals/WorkStartSurface.test.tsx @@ -137,6 +137,7 @@ describe("WorkStartSurface", () => { online: true, lanes: [{ id: "lane-studio", name: "Studio lane" } as any], sessions: [], + prs: [], lastSyncedAtMs: 1, error: null, }, diff --git a/apps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsx b/apps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsx index ea8308a4dd..53e4694758 100644 --- a/apps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsx +++ b/apps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsx @@ -561,7 +561,7 @@ describe("WorkViewArea", () => { expect(terminals.map((terminal) => terminal.getAttribute("data-session-id"))).toContain("session-1"); }); - it("suppresses the PR pane and auto-pop reads for a foreign running CLI", async () => { + it("pins the PR pane and auto-pop reads to the owning machine for a foreign running CLI", async () => { const session = { ...makeRunningSession("session-foreign", "pty-foreign"), toolType: "codex" as const }; const runtimePin = { kind: "remote", @@ -591,11 +591,16 @@ describe("WorkViewArea", () => { ); const local = within(view.container); - expect(local.queryByRole("button", { name: "Toggle PR pane" })).toBeNull(); - expect(local.queryByTestId("chat-pr-pane")).toBeNull(); + // A lane's PR lives in ITS machine's database, so a foreign session gets the + // same PR affordance as a local one — the reads just carry the pin. Before + // this, both the pane and its auto-pop were suppressed outright, which is + // what made a remote session's PR invisible until the tab was rebound. await waitFor(() => { - for (const mock of Object.values(prsMocks)) expect(mock).not.toHaveBeenCalled(); + expect(prsMocks.getForLane).toHaveBeenCalledWith("lane-1", runtimePin); }); + expect(prsMocks.onEvent).toHaveBeenCalledWith(expect.any(Function), runtimePin); + fireEvent.click(local.getByRole("button", { name: "Toggle PR pane" })); + expect((await local.findByTestId("chat-pr-pane")).getAttribute("data-lane-id")).toBe("lane-1"); }); it("keeps PR auto-pop and pane controls enabled for a local running CLI", async () => { @@ -619,7 +624,7 @@ describe("WorkViewArea", () => { const local = within(view.container); await waitFor(() => { - expect(prsMocks.getForLane).toHaveBeenCalledWith("lane-1"); + expect(prsMocks.getForLane).toHaveBeenCalledWith("lane-1", null); expect(prsMocks.onEvent).toHaveBeenCalledTimes(1); }); fireEvent.click(local.getByRole("button", { name: "Toggle PR pane" })); diff --git a/apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx b/apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx index effaff46b2..c674480fc4 100644 --- a/apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx +++ b/apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx @@ -795,14 +795,14 @@ function CliSessionSurface({ }) { // Persist the pane per CLI session so reopening the surface restores it, the // same way the ADE chat pane keys its companion UI state. - // PR data lives on the owning machine, but the prs preload surface is - // unpinned. Foreign CLI sessions therefore expose neither auto-pop nor pane. - const prLaneId = runtimePin ? null : session.laneId; - const { prPaneOpen, setPrPaneOpen, prPaneDelta } = useChatPrAutoPop(prLaneId, { + // PR reads follow the lane's machine now, so a foreign CLI session gets the + // same pill, auto-pop and pane as a local one — the pin just routes them. + const { prPaneOpen, setPrPaneOpen, prPaneDelta } = useChatPrAutoPop(session.laneId, { persistKey: session.id, + runtimePin, }); const supportsSplit = layoutVariant !== "grid-tile"; - const prFloating = prPaneOpen && Boolean(prLaneId) && supportsSplit; + const prFloating = prPaneOpen && Boolean(session.laneId) && supportsSplit; return (
{layoutVariant !== "grid-tile" ? ( @@ -818,8 +818,9 @@ function CliSessionSurface({ sessionsPaneCount={sessionsPaneCount} onToggleToolsPane={onToggleToolsPane} toolsPaneOpen={toolsPaneOpen} - onTogglePrPane={prLaneId ? () => setPrPaneOpen((v) => !v) : undefined} + onTogglePrPane={session.laneId ? () => setPrPaneOpen((v) => !v) : undefined} prPaneOpen={prPaneOpen} + runtimePin={runtimePin} /> ) : null}
@@ -834,7 +835,7 @@ function CliSessionSurface({ className="h-full w-full" /> - {prFloating && prLaneId ? ( + {prFloating && session.laneId ? (
setPrPaneOpen(false)} + runtimePin={runtimePin} />
diff --git a/apps/desktop/src/renderer/components/terminals/useLanePrs.test.ts b/apps/desktop/src/renderer/components/terminals/useLanePrs.test.ts index 007db359f2..ca6b96a8de 100644 --- a/apps/desktop/src/renderer/components/terminals/useLanePrs.test.ts +++ b/apps/desktop/src/renderer/components/terminals/useLanePrs.test.ts @@ -1,6 +1,14 @@ import { describe, expect, it } from "vitest"; import type { GitHubPrListItem, LaneSummary, PrSummary } from "../../../shared/types"; -import { buildLanePrsByLaneId } from "./useLanePrs"; +import { + boundMachineLanePrs, + buildLanePrsByLaneId, + laneAnyMachineKey, + laneBoundMachineKey, + laneHasAnyPr, + lanePrCompositeKey, + lanePrsForMachine, +} from "./useLanePrs"; function lane(overrides: Partial = {}): LaneSummary { return { @@ -133,3 +141,47 @@ describe("buildLanePrsByLaneId", () => { expect(result.has("lane-1")).toBe(false); }); }); + +/* + * Key discipline. These pin the invariant that made a remote machine's PRs + * invisible: a lookup has to name the machine, because a lane id does not. + */ +describe("lane PR key namespaces", () => { + it("never collides across the three namespaces for one lane id", () => { + const keys = [ + lanePrCompositeKey("machine-a", "lane-1"), + laneBoundMachineKey("lane-1"), + laneAnyMachineKey("lane-1"), + ]; + + expect(new Set(keys).size).toBe(3); + expect(keys).not.toContain("lane-1"); + expect(lanePrCompositeKey("machine-a", "lane-1")) + .not.toBe(lanePrCompositeKey("machine-b", "lane-1")); + }); + + // The regression: cross-machine handoff copies a lane id, so a foreign + // machine's PR could answer a bound-machine lookup and render a badge that + // deep-links into a PRs tab which cannot resolve it. + it("keeps a foreign machine's PR out of the bound machine's answer", () => { + const byLane = new Map([ + [lanePrCompositeKey("machine-b", "lane-1"), [mappedPr({ id: "pr-foreign" })]], + [laneAnyMachineKey("lane-1"), [mappedPr({ id: "pr-foreign" })]], + ]); + + expect(boundMachineLanePrs(byLane, "lane-1")).toEqual([]); + expect(lanePrsForMachine(byLane, "machine-b", "lane-1")[0]?.id).toBe("pr-foreign"); + expect(lanePrsForMachine(byLane, "machine-a", "lane-1")).toEqual([]); + }); + + it("answers the filter chip from any machine, including a foreign-only lane", () => { + const byLane = new Map([ + [lanePrCompositeKey("machine-b", "lane-1"), [mappedPr()]], + [laneAnyMachineKey("lane-1"), [mappedPr()]], + ]); + + expect(laneHasAnyPr(byLane, "lane-1")).toBe(true); + expect(laneHasAnyPr(byLane, "lane-unknown")).toBe(false); + expect(boundMachineLanePrs(byLane, "lane-unknown")).toEqual([]); + }); +}); diff --git a/apps/desktop/src/renderer/components/terminals/useLanePrs.ts b/apps/desktop/src/renderer/components/terminals/useLanePrs.ts index ee5ff9e612..65c5d31b76 100644 --- a/apps/desktop/src/renderer/components/terminals/useLanePrs.ts +++ b/apps/desktop/src/renderer/components/terminals/useLanePrs.ts @@ -1,7 +1,8 @@ -import { useEffect, useMemo, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import type { GitHubPrListItem, LaneSummary, PrSummary } from "../../../shared/types"; import { getGitHubSnapshotCoalesced, listPrsCoalesced } from "../../lib/prReadCache"; -import { selectActiveProjectRoot, useAppStore } from "../../state/appStore"; +import { selectActiveProjectRoot, useAppStore, useRootAppStore } from "../../state/appStore"; +import { THIS_MACHINE_ID } from "../../../shared/machineIdentity"; import { lanePrMatchesCurrentBranch, selectGithubLanePrTag, @@ -36,6 +37,32 @@ function githubItemToLanePr(item: GitHubPrListItem, laneId: string): PrSummary { }; } +/** + * Key vocabulary for the lane→PR map. + * + * The invariant behind all three: lane ids are NOT unique across machines — + * cross-machine handoff copies a lane, id included — so "which machine" is part + * of the identity of a PR lookup. Every key is namespaced, so a key can only + * ever mean one thing and no precedence rule has to be remembered. Nothing is + * ever filed under a bare lane id; when the bare id doubled as "the bound + * machine's answer", a foreign machine's PR could fill an id the bound machine + * had not claimed and render on a bound-machine row, whose badge deep-links + * into a PRs tab that cannot resolve it. + */ +export function lanePrCompositeKey(machineId: string, laneId: string): string { + return `${machineId}:${laneId}`; +} + +/** Alias for the bound machine, so purely-local render paths need no machine id. */ +export function laneBoundMachineKey(laneId: string): string { + return `bound:${laneId}`; +} + +/** The union across every machine. */ +export function laneAnyMachineKey(laneId: string): string { + return `any:${laneId}`; +} + export function buildLanePrsByLaneId(args: { lanes: LaneSummary[]; prs: PrSummary[]; @@ -61,6 +88,31 @@ export function buildLanePrsByLaneId(args: { return byLane; } +/** The only lookup that may ignore machine identity — the "has PR" filter chip. */ +export function laneHasAnyPr( + byLane: Map, + laneId: string, +): boolean { + return (byLane.get(laneAnyMachineKey(laneId))?.length ?? 0) > 0; +} + +/** What every row on the tab's own machine renders. */ +export function boundMachineLanePrs( + byLane: Map, + laneId: string, +): PrSummary[] { + return byLane.get(laneBoundMachineKey(laneId)) ?? []; +} + +/** What a foreign row renders — its own machine's answer, never another's. */ +export function lanePrsForMachine( + byLane: Map, + machineId: string, + laneId: string, +): PrSummary[] { + return byLane.get(lanePrCompositeKey(machineId, laneId)) ?? []; +} + /** * Canonical current-branch PRs grouped by lane id. A coalesced mapped-PR read * and GitHub snapshot read cover both ADE-linked and GitHub-only PRs without @@ -71,10 +123,18 @@ export function buildLanePrsByLaneId(args: { * underlying read is coalesced and the event subscription is idempotent, so * more than one caller costs a listener and nothing else. * + * Cross-machine: a mapped PR row lives in the `.ade` database of the machine + * that owns the lane, so the active binding's `listAll` can only ever answer for + * its own machine — which is why a session on another machine used to show no PR + * until the project tab was rebound to it. Each machine's rows arrive with its + * union slice and are folded in here under `lanePrCompositeKey`. The GitHub + * snapshot is deliberately NOT re-read per machine: it describes the repo, not a + * machine, so the same snapshot joins correctly against every machine's lanes. */ export function useLanePrsByLaneId(): Map { const projectRoot = useAppStore(selectActiveProjectRoot); const lanes = useAppStore((state) => state.lanes); + const machines = useRootAppStore((state) => state.crossMachineLanesByMachineId); const [prs, setPrs] = useState([]); const [githubPrs, setGithubPrs] = useState([]); useEffect(() => { @@ -107,8 +167,79 @@ export function useLanePrsByLaneId(): Map { unsubscribe(); }; }, [projectRoot]); - return useMemo( + // Derived, not searched. `isEligibleMachineOption` excludes the tab's own + // machine from the union, so looking it up in `machines` would always miss and + // the composite key would never be filled — leaving correctness resting on + // every bound-machine render path happening to use the alias. Deriving it + // enforces the intent instead of relying on it. + const boundMachineId = useAppStore((state) => ( + state.projectBinding?.kind === "remote" + ? state.projectBinding.targetId + : THIS_MACHINE_ID + )); + // The tab's own machine is answered by the read above, which is event-driven + // and therefore fresher than the 30s union slice. It is never double-read: + // `readMachine` is never called for this machine at all — `isEligibleMachineOption` + // excludes it from the union. Memoized separately so chat churn (a new + // `machines` identity every ~10s) does not rebuild it. + const boundBuilt = useMemo( () => buildLanePrsByLaneId({ lanes, prs, githubPrs }), [githubPrs, lanes, prs], ); + // Per-machine memo. `mergeCrossMachineLanes` keeps `lanes`/`prs` reference- + // stable across ticks that do not change them, so a machine whose CHATS + // churned re-uses its built map instead of paying the rebuild. Without this + // the whole union rebuilds on chat churn, on a hook two Work hot-path + // consumers share. + const perMachineCache = useRef(new Map; + }>()); + return useMemo(() => { + const byLane = new Map(); + const file = ( + built: Map, + machineId: string, + alias = false, + ) => { + for (const [laneId, list] of built) { + byLane.set(lanePrCompositeKey(machineId, laneId), list); + // The bound machine is filed twice: under its real id, and under a fixed + // alias so purely-local render paths need no machine id at all. + if (alias) byLane.set(laneBoundMachineKey(laneId), list); + // First machine to answer a lane id owns the union key. Which one that + // is does not matter — its only consumer is the "has a PR at all" + // filter chip, and every rendering path reads a machine-scoped key. + const anyKey = laneAnyMachineKey(laneId); + if (!byLane.has(anyKey)) byLane.set(anyKey, list); + } + }; + + file(boundBuilt, boundMachineId, true); + + const cache = perMachineCache.current; + for (const machine of Object.values(machines)) { + if (!machine.lanes.length) continue; + const cached = cache.get(machine.machineId); + const built = cached + && cached.lanes === machine.lanes + && cached.prs === machine.prs + && cached.githubPrs === githubPrs + ? cached.built + : buildLanePrsByLaneId({ lanes: machine.lanes, prs: machine.prs, githubPrs }); + cache.set(machine.machineId, { + lanes: machine.lanes, + prs: machine.prs, + githubPrs, + built, + }); + file(built, machine.machineId); + } + for (const machineId of [...cache.keys()]) { + if (!machines[machineId]) cache.delete(machineId); + } + return byLane; + }, [boundBuilt, boundMachineId, githubPrs, machines]); } diff --git a/apps/desktop/src/renderer/components/terminals/useWorkSessions.test.ts b/apps/desktop/src/renderer/components/terminals/useWorkSessions.test.ts index c29dd6be5c..b0706170c2 100644 --- a/apps/desktop/src/renderer/components/terminals/useWorkSessions.test.ts +++ b/apps/desktop/src/renderer/components/terminals/useWorkSessions.test.ts @@ -808,6 +808,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { online: true, lanes: [], sessions: [foreign], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, @@ -884,6 +885,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { binding: bindingA, lanes: [{ id: "lane-a" }], sessions: [foreignA], + prs: [], }, "target-b": { machineId: "target-b", @@ -891,6 +893,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { binding: bindingB, lanes: [{ id: "lane-b" }], sessions: [foreignB], + prs: [], }, }, }; @@ -919,6 +922,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { binding: bindingA, lanes: [{ id: "lane-a" }], sessions: [{ ...foreignA, lastOutputPreview: "fresh-a" }], + prs: [], }, }, }; @@ -941,6 +945,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { binding: bindingA, lanes: [{ id: "lane-a" }], sessions: [{ ...foreignA, lastOutputPreview: "fresher-a" }], + prs: [], }, }, }; @@ -994,6 +999,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { binding: bindingA, lanes: [{ id: "lane-a" }], sessions: [foreignA], + prs: [], }, "target-b": { machineId: "target-b", @@ -1001,6 +1007,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { binding: bindingB, lanes: [{ id: "lane-b" }], sessions: [foreignB], + prs: [], }, }, }; @@ -1031,6 +1038,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { binding: bindingA, lanes: [{ id: "lane-a" }], sessions: [foreignA], + prs: [], }, }, }; @@ -1064,6 +1072,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { machineName: "Machine B", lanes: [{ id: "foreign-lane" }], sessions: [foreign], + prs: [], }, }, }; @@ -1079,6 +1088,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { machineName: "Machine B", lanes: [{ id: "foreign-lane" }], sessions: [], + prs: [], }, }, }; @@ -1170,6 +1180,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { binding: foreignBinding, lanes: [{ id: "lane-b" }], sessions: [foreignSession], + prs: [], }, }, }; @@ -1254,6 +1265,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { binding: foreignBinding, lanes: [{ id: "lane-b" }], sessions: [foreignSession], + prs: [], }, }, }; @@ -1310,6 +1322,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { online: true, lanes: [{ id: "lane-1" }], sessions: [foreignSession], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, @@ -1385,6 +1398,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { binding: foreignBinding, lanes: [{ id: "lane-b" }], sessions: [foreignSession], + prs: [], }, }, }; @@ -1805,6 +1819,7 @@ describe("useWorkSessions — refresh-before-focus ordering", () => { machineName: "Machine B", lanes: [{ id: foreign.laneId }], sessions: [foreign], + prs: [], }, }, }; diff --git a/apps/desktop/src/renderer/components/terminals/useWorkSessions.ts b/apps/desktop/src/renderer/components/terminals/useWorkSessions.ts index 4de18a8907..91e6588923 100644 --- a/apps/desktop/src/renderer/components/terminals/useWorkSessions.ts +++ b/apps/desktop/src/renderer/components/terminals/useWorkSessions.ts @@ -26,7 +26,7 @@ import { } from "../../lib/terminalAttention"; import type { CanonicalStatusBucket } from "../../../shared/sessionCanonicalState"; import { nextSnoozeDeadlineMs } from "../../lib/sessionSnooze"; -import { useLanePrsByLaneId } from "./useLanePrs"; +import { laneHasAnyPr, useLanePrsByLaneId } from "./useLanePrs"; import { applyWorkLaneManualMove, type WorkLaneSortMode } from "./workLaneOrder"; import { EMPTY_WORK_SESSION_FILTERS, @@ -1561,7 +1561,10 @@ export function useWorkSessions({ active = true }: UseWorkSessionsOptions = {}) if (isWorkSessionFilterEmpty(workSessionFilters)) return filtered; const ctx = { nowMs: Date.now(), - laneHasPr: (laneId: string) => (prsByLaneId.get(laneId)?.length ?? 0) > 0, + // Union answer on purpose: the chip asks "does this lane have a PR at + // all", and a filtered session row carries no machine to key on. Badges + // use the machine-scoped lookups instead — see `lanePrsForMachine`. + laneHasPr: (laneId: string) => laneHasAnyPr(prsByLaneId, laneId), laneIsDirty: (laneId: string) => laneStatusById.get(laneId)?.status.dirty === true, }; return filtered.filter((session) => matchesWorkSessionFilters(session, workSessionFilters, ctx)); diff --git a/apps/desktop/src/renderer/components/work/WorkSurfaceHeader.tsx b/apps/desktop/src/renderer/components/work/WorkSurfaceHeader.tsx index dab33a23ae..a244d4a609 100644 --- a/apps/desktop/src/renderer/components/work/WorkSurfaceHeader.tsx +++ b/apps/desktop/src/renderer/components/work/WorkSurfaceHeader.tsx @@ -7,6 +7,7 @@ import { SessionLifecycleChips } from "./SessionLifecycleChips"; import { ClaudeCacheTtlBadge } from "../shared/ClaudeCacheTtlBadge"; import { useFloatingPaneEmbeddedChrome } from "../ui/FloatingPane"; import { cn } from "../ui/cn"; +import type { OpenProjectBinding } from "../../../shared/types"; // Provider default chat titles — mirrors DEFAULT_SESSION_TITLES in // agentChatService.ts. When a chat's title transitions FROM one of these TO a @@ -199,6 +200,8 @@ export type WorkSurfaceHeaderProps = { */ onTogglePrPane?: () => void; prPaneOpen?: boolean; + /** See `ChatGitToolbar.runtimePin`: the machine this lane's PR is read from. */ + runtimePin?: OpenProjectBinding | null; /** Surface-specific trailing actions (right side of the row). */ trailingActions?: ReactNode; /** @@ -242,6 +245,7 @@ export function WorkSurfaceHeader({ showGitToolbar = false, onTogglePrPane, prPaneOpen, + runtimePin = null, trailingActions, onToggleSessionsPane, sessionsPaneCollapsed = false, @@ -289,7 +293,12 @@ export function WorkSurfaceHeader({
{showGitToolbar && laneId ? ( - + ) : null} {trailingActions || onToggleToolsPane ? ( diff --git a/apps/desktop/src/renderer/lib/lanePrBadge.test.ts b/apps/desktop/src/renderer/lib/lanePrBadge.test.ts index 3d9a26580c..339d42d553 100644 --- a/apps/desktop/src/renderer/lib/lanePrBadge.test.ts +++ b/apps/desktop/src/renderer/lib/lanePrBadge.test.ts @@ -1,6 +1,6 @@ -import { describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import type { PrState } from "../../shared/types"; -import { pickPrimaryPr, primaryPrStateRank } from "./lanePrBadge"; +import { openLanePr, pickPrimaryPr, primaryPrStateRank } from "./lanePrBadge"; type TestPr = { id: string; state: PrState; updatedAt: string; githubPrNumber: number }; @@ -69,3 +69,63 @@ describe("pickPrimaryPr", () => { }); } }); + +describe("openLanePr", () => { + const foreignPr = { + id: "pr-on-other-machine", + githubUrl: "https://github.com/arul28/ADE/pull/91", + } as unknown as Parameters[0]; + + beforeEach(() => { + (globalThis as unknown as { window: unknown }).window = { + ade: { app: { openExternal: vi.fn(async () => {}) } }, + open: vi.fn(), + }; + }); + + it("deep-links a local PR into the PRs tab", () => { + const navigate = vi.fn(); + openLanePr(foreignPr, { foreign: false, navigate }); + + expect(navigate).toHaveBeenCalledTimes(1); + expect(navigate).toHaveBeenCalledWith("/prs?tab=normal&prId=pr-on-other-machine"); + expect(window.ade.app.openExternal).not.toHaveBeenCalled(); + }); + + it("honours a caller's richer local route over the default deep link", () => { + const navigate = vi.fn(); + openLanePr(foreignPr, { foreign: false, navigate, localPath: "/prs?tab=normal&laneId=lane-1" }); + + expect(navigate).toHaveBeenCalledWith("/prs?tab=normal&laneId=lane-1"); + }); + + // The regression: a PR id only resolves on the machine that owns it, so + // deep-linking a foreign PR landed on an empty PRs tab. GitHub is the one + // destination that means the same thing from either machine. + it("sends a foreign PR to GitHub instead of the machine-scoped PRs tab", () => { + const navigate = vi.fn(); + openLanePr(foreignPr, { foreign: true, navigate }); + + expect(navigate).not.toHaveBeenCalled(); + expect(window.ade.app.openExternal) + .toHaveBeenCalledWith("https://github.com/arul28/ADE/pull/91"); + }); + + it("falls back to window.open when the external open is refused", async () => { + const navigate = vi.fn(); + (window.ade.app.openExternal as ReturnType) + .mockRejectedValueOnce(new Error("blocked scheme")); + + openLanePr(foreignPr, { foreign: true, navigate }); + // The fallback runs in the rejected promise's catch; flush the microtask + // queue rather than guessing a tick count. + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(navigate).not.toHaveBeenCalled(); + expect(window.open).toHaveBeenCalledWith( + "https://github.com/arul28/ADE/pull/91", + "_blank", + "noopener,noreferrer", + ); + }); +}); diff --git a/apps/desktop/src/renderer/lib/lanePrBadge.ts b/apps/desktop/src/renderer/lib/lanePrBadge.ts index 7233db76b1..7445e54128 100644 --- a/apps/desktop/src/renderer/lib/lanePrBadge.ts +++ b/apps/desktop/src/renderer/lib/lanePrBadge.ts @@ -82,3 +82,51 @@ export function lanePrStateColor(state: PrState): string { return COLORS.textSecondary; // closed } } + +/** Deep link the PR chip opens, identical from every host so the target cannot drift. */ +export function lanePrDeepLinkPath(pr: PrSummary): string { + return `/prs?tab=normal&prId=${encodeURIComponent(pr.id)}`; +} + +/** + * Opens a lane's PR from any Work surface. + * + * The PRs tab reads the machine the project tab is bound to, and a PR id is only + * resolvable on the machine that owns it — so deep-linking a PR that lives on + * another machine lands on an empty tab. GitHub resolves identically from every + * machine, so a foreign PR opens there instead of pretending to navigate. + * + * Canonical on purpose: the sidebar badge, the session card, and the chat PR + * pane all answer "where does this PR open" the same way, and a fourth caller + * getting it wrong is exactly how the foreign-PR dead end appeared twice. + */ +export function openLanePr( + pr: PrSummary, + options: { + foreign?: boolean; + navigate: (path: string) => void; + /** + * Overrides the in-app destination for the local case. The chat toolbar + * uses a richer route that also selects the lane; everything else wants the + * plain deep link. + */ + localPath?: string; + }, +): void { + if (!options.foreign) { + options.navigate(options.localPath ?? lanePrDeepLinkPath(pr)); + return; + } + if (!pr.githubUrl) return; + // `githubUrl` on a foreign row is data the paired machine sent us, so + // `openExternal` can reject (the main process enforces an http/https/mailto + // allowlist). Swallow it into the same in-app fallback the PR pane uses + // rather than raising an unhandled rejection on a click handler. + void window.ade.app.openExternal(pr.githubUrl).catch(() => { + try { + window.open(pr.githubUrl, "_blank", "noopener,noreferrer"); + } catch { + /* nothing left to try */ + } + }); +} diff --git a/apps/desktop/src/renderer/lib/prReadCache.ts b/apps/desktop/src/renderer/lib/prReadCache.ts index f03ade1c33..d0db111d72 100644 --- a/apps/desktop/src/renderer/lib/prReadCache.ts +++ b/apps/desktop/src/renderer/lib/prReadCache.ts @@ -1,4 +1,4 @@ -import type { GitHubPrSnapshot, PrSummary } from "../../shared/types"; +import type { GitHubPrSnapshot, OpenProjectBinding, PrSummary } from "../../shared/types"; type InFlightEntry = { promise: Promise; @@ -13,7 +13,19 @@ const linkedPrRecentRefresh = new Map( return promise; } -export function listPrsCoalesced(options?: { projectRoot?: string | null }): Promise { +export function listPrsCoalesced(options?: { + projectRoot?: string | null; + pin?: OpenProjectBinding | null; +}): Promise { return coalesceInFlight( prListInFlight, - projectKey(options?.projectRoot), - () => window.ade.prs.listAll(), + projectKey(options?.projectRoot, options?.pin), + () => window.ade.prs.listAll(options?.pin ?? null), ); } @@ -72,17 +87,17 @@ export function getGitHubSnapshotCoalesced( export function refreshPrsCoalesced( args: { prId?: string; prIds?: string[] } = {}, - options?: { projectRoot?: string | null }, + options?: { projectRoot?: string | null; pin?: OpenProjectBinding | null }, ): Promise { const prIds = args.prIds?.filter(Boolean).sort() ?? []; return coalesceInFlight( prRefreshInFlight, JSON.stringify({ - projectRoot: projectKey(options?.projectRoot), + projectRoot: projectKey(options?.projectRoot, options?.pin), prId: args.prId ?? null, prIds, }), - () => window.ade.prs.refresh(args), + () => window.ade.prs.refresh(args, options?.pin ?? null), ); } @@ -90,6 +105,7 @@ export function refreshLinkedPrCoalesced( pr: PrSummary, options?: { projectRoot?: string | null; + pin?: OpenProjectBinding | null; force?: boolean; cooldownMs?: number; }, @@ -98,7 +114,7 @@ export function refreshLinkedPrCoalesced( if (!prId) return Promise.resolve(null); const key = JSON.stringify({ - projectRoot: projectKey(options?.projectRoot), + projectRoot: projectKey(options?.projectRoot, options?.pin), prId, }); const cooldownMs = Math.max(0, options?.cooldownMs ?? LINKED_PR_LIVE_REFRESH_COOLDOWN_MS); @@ -112,7 +128,10 @@ export function refreshLinkedPrCoalesced( key, async () => { try { - const refreshed = await refreshPrsCoalesced({ prIds: [prId] }, { projectRoot: options?.projectRoot }); + const refreshed = await refreshPrsCoalesced( + { prIds: [prId] }, + { projectRoot: options?.projectRoot, pin: options?.pin }, + ); const result = refreshed.find((next) => next.id === prId) ?? null; linkedPrRecentRefresh.set(key, { refreshedAt: Date.now(), result }); return result; diff --git a/apps/desktop/src/renderer/state/appStore.ts b/apps/desktop/src/renderer/state/appStore.ts index 2b7c083264..bb86714ed2 100644 --- a/apps/desktop/src/renderer/state/appStore.ts +++ b/apps/desktop/src/renderer/state/appStore.ts @@ -2,7 +2,7 @@ import React, { createContext, useContext, type ReactNode } from "react"; import { useStore } from "zustand"; import { createStore, type StoreApi } from "zustand/vanilla"; import type { StateCreator } from "zustand"; -import type { CtoAttentionState, KeybindingsSnapshot, LaneDeleteProgress, LaneListSnapshot, LaneSummary, OpenProjectBinding, ProjectInfo, ProjectPathInspection, ProviderMode, RecentProjectSummary, TerminalSessionSummary } from "../../shared/types"; +import type { CtoAttentionState, KeybindingsSnapshot, LaneDeleteProgress, LaneListSnapshot, LaneSummary, OpenProjectBinding, PrSummary, ProjectInfo, ProjectPathInspection, ProviderMode, RecentProjectSummary, TerminalSessionSummary } from "../../shared/types"; import { recentProjectStateKey } from "../../shared/projectIdentity"; import { THIS_MACHINE_ID } from "../../shared/machineIdentity"; import { MODEL_REGISTRY, type ModelDescriptor } from "../../shared/modelRegistry"; @@ -1045,6 +1045,13 @@ export type CrossMachineMachineLanes = { online: boolean; lanes: LaneSummary[]; sessions: TerminalSessionSummary[]; + /** + * The machine's own mapped PR records. A PR row lives in the `.ade` database + * of the machine that owns the lane, so — like lanes and sessions — it can + * only be read from that machine. Carried here so a card or chat header can + * show its lane's PR without the project tab being bound to that machine. + */ + prs: PrSummary[]; lastSyncedAtMs: number | null; /** Last read failure, kept alongside (not instead of) the retained lanes. */ error: string | null; @@ -1213,6 +1220,7 @@ export type AppState = { online?: boolean; lanes?: LaneSummary[]; sessions?: TerminalSessionSummary[]; + prs?: PrSummary[]; error?: string | null; }) => void; /** @@ -1783,6 +1791,7 @@ const createAppState: StateCreator = (set, get) => { const previous = prev.crossMachineLanesByMachineId[machineId] ?? null; const lanes = reuseStructurallyEqualArray(entry.lanes, previous?.lanes); const sessions = reuseStructurallyEqualArray(entry.sessions, previous?.sessions); + const prs = reuseStructurallyEqualArray(entry.prs, previous?.prs); const incomingBinding = entry.binding !== undefined ? entry.binding : previous?.binding ?? null; const binding = reuseStructurallyEqualValue(incomingBinding, previous?.binding ?? null); @@ -1797,8 +1806,11 @@ const createAppState: StateCreator = (set, get) => { // returned nothing must not erase what the machine last reported. lanes: lanes ?? previous?.lanes ?? [], sessions: sessions ?? previous?.sessions ?? [], + prs: prs ?? previous?.prs ?? [], lastSyncedAtMs: - entry.lanes || entry.sessions ? Date.now() : previous?.lastSyncedAtMs ?? null, + entry.lanes || entry.sessions || entry.prs + ? Date.now() + : previous?.lastSyncedAtMs ?? null, error: entry.error !== undefined ? entry.error : previous?.error ?? null, }; const sliceUnchanged = ( @@ -1810,6 +1822,7 @@ const createAppState: StateCreator = (set, get) => { && previous.online === next.online && previous.lanes === next.lanes && previous.sessions === next.sessions + && previous.prs === next.prs && previous.lastSyncedAtMs === next.lastSyncedAtMs && previous.error === next.error ); diff --git a/apps/desktop/src/renderer/state/appStore.worktreeGate.test.ts b/apps/desktop/src/renderer/state/appStoreWorktreeGate.test.ts similarity index 100% rename from apps/desktop/src/renderer/state/appStore.worktreeGate.test.ts rename to apps/desktop/src/renderer/state/appStoreWorktreeGate.test.ts diff --git a/apps/desktop/src/renderer/state/crossMachineLanes.test.ts b/apps/desktop/src/renderer/state/crossMachineLanes.test.ts index 4f10961fd4..85279ccc17 100644 --- a/apps/desktop/src/renderer/state/crossMachineLanes.test.ts +++ b/apps/desktop/src/renderer/state/crossMachineLanes.test.ts @@ -9,6 +9,7 @@ import { buildCrossMachineLaneRows, cancelCrossMachineOptimisticChatSession, decodeForeignLanes, + decodeForeignPrs, decodeForeignSessions, reconcileCrossMachineOptimisticSessions, orderCrossMachineRows, @@ -149,6 +150,7 @@ describe("offline machines stay in the sidebar, dimmed", () => { // Newer activity than the reachable machine, and still ranked below it. lanes: [makeLane({ id: "lane-offline", createdAt: "2026-07-28T10:00:00.000Z" })], sessions: [], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, @@ -160,6 +162,7 @@ describe("offline machines stay in the sidebar, dimmed", () => { online: true, lanes: [makeLane({ id: "lane-online", createdAt: "2026-07-20T10:00:00.000Z" })], sessions: [], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, @@ -312,6 +315,7 @@ describe("machine marker", () => { online: true, lanes: [makeLane({ id: "lane-foreign", branchRef: "feature/foreign" })], sessions: [], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, @@ -358,6 +362,7 @@ describe("machine marker", () => { online: false, lanes: [activeLane], sessions: [makeSession({ id: "session-duplicate", laneId: activeLane.id })], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, @@ -375,6 +380,7 @@ describe("machine marker", () => { online: true, lanes: [thisMacLane], sessions: [makeSession({ id: "session-local", laneId: thisMacLane.id })], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, @@ -412,6 +418,7 @@ describe("machine marker", () => { online: false, lanes: [makeLane({ id: "lane-offline", branchRef: "feature/shared" })], sessions: [], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, @@ -423,6 +430,7 @@ describe("machine marker", () => { online: true, lanes: [makeLane({ id: "lane-online", branchRef: "feature/shared" })], sessions: [], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, @@ -458,6 +466,7 @@ describe("machine marker", () => { online: true, lanes: [makeLane({ id: "lane-a", branchRef: "feature/a" })], sessions: [], + prs: [], lastSyncedAtMs: 1, error: null, }, @@ -469,6 +478,7 @@ describe("machine marker", () => { online: true, lanes: [makeLane({ id: "lane-b", branchRef: "feature/b" })], sessions: [], + prs: [], lastSyncedAtMs: 1, error: null, }, @@ -494,6 +504,7 @@ describe("machine marker", () => { online: true, lanes: [makeLane({ id: "lane-foreign", branchRef: "feature/shared" })], sessions: [], + prs: [], lastSyncedAtMs: 1, error: null, }, @@ -602,6 +613,7 @@ describe("selectOtherMachineBranchStates", () => { }), ], sessions: [], + prs: [], lastSyncedAtMs: 1, error: null, }, @@ -650,6 +662,7 @@ describe("selectOtherMachineBranchStates", () => { online: true, lanes: [makeLane({ id: "lane-foreign", branchRef: "feature/other" })], sessions: [], + prs: [], lastSyncedAtMs: 1, error: null, }, @@ -678,6 +691,7 @@ describe("selectOtherMachineBranchStates", () => { }), ], sessions: [], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, @@ -698,6 +712,7 @@ describe("selectOtherMachineBranchStates", () => { online: false, lanes: [makeLane({ id: "lane-foreign", branchRef: "feature/shared" })], sessions: [], + prs: [], lastSyncedAtMs: null, error: "not yet reachable", }, @@ -731,6 +746,7 @@ describe("selectOtherMachineBranchStates", () => { online: true, lanes: [activeLane], sessions: [], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, @@ -748,6 +764,7 @@ describe("selectOtherMachineBranchStates", () => { }), ], sessions: [], + prs: [], lastSyncedAtMs: Date.now(), error: null, }, @@ -779,6 +796,7 @@ describe("selectOtherMachineBranchStates", () => { online: false, lanes: [makeLane({ id: "lane-foreign", branchRef: "feature/shared" })], sessions: [], + prs: [], lastSyncedAtMs: syncedAtMs, error: "offline", }, @@ -798,6 +816,32 @@ describe("foreign payload decoding", () => { expect(decodeForeignSessions([{ id: "s", laneId: "l" }, { id: "s2" }])).toHaveLength(1); expect(decodeForeignSessions("nope")).toEqual([]); }); + + // A half-decoded PR renders "PR #undefined", or a badge whose click is a + // silent no-op because the foreign click-through has nowhere to go. Dropping + // the row shows no badge, which is honest. + it("drops PR rows missing any field the foreign badge renders", () => { + const complete = { + id: "pr-1", + laneId: "lane-1", + headBranch: "feature/x", + githubPrNumber: 91, + githubUrl: "https://github.com/arul28/ADE/pull/91", + state: "open", + }; + + expect(decodeForeignPrs([complete])).toHaveLength(1); + expect(decodeForeignPrs({ prs: [complete] })).toHaveLength(1); + expect(decodeForeignPrs([ + { ...complete, githubPrNumber: undefined }, + { ...complete, githubUrl: "" }, + { ...complete, laneId: "" }, + { ...complete, state: undefined }, + { ...complete, id: "" }, + null, + ])).toEqual([]); + expect(decodeForeignPrs("nope")).toEqual([]); + }); }); describe("cross-machine refresh scheduling", () => { @@ -899,25 +943,31 @@ describe("cross-machine refresh scheduling", () => { }); await Promise.resolve(); await vi.advanceTimersByTimeAsync(400); - expect(callAction).toHaveBeenCalledTimes(2); + // Three reads go out TOGETHER on a lane-cadence tick: lanes, chats and PRs. + // PRs ride the lane cadence (a PR is only rendered by joining it to a lane) + // and must be issued in parallel — reading them after the lane read would + // hold this machine's lanes and chats behind a second 8s timeout and stall + // every other machine's cadence with it. + expect(callAction).toHaveBeenCalledTimes(3); // The old setInterval path started another generation on its own cadence, // invalidating this still-live read. A settled-chain poll must leave it alone // for as long as the read's own timeout allows it to run. await vi.advanceTimersByTimeAsync(7_500); - expect(callAction).toHaveBeenCalledTimes(2); + expect(callAction).toHaveBeenCalledTimes(3); pending.splice(0).forEach((resolve, index) => resolve({ - result: index === 0 ? { lanes: [] } : { sessions: [] }, + result: index === 0 ? { lanes: [] } : index === 1 ? { sessions: [] } : { prs: [] }, })); await Promise.resolve(); await Promise.resolve(); await vi.advanceTimersByTimeAsync(9_000); - expect(callAction).toHaveBeenCalledTimes(2); - // The next tick reads chats only: the lane list has its own 30s cadence, and - // no chat referenced a lane this machine has not already reported. - await vi.advanceTimersByTimeAsync(2_000); expect(callAction).toHaveBeenCalledTimes(3); + // The next tick reads chats only: the lane list (and with it the PR list) + // has its own 30s cadence, and no chat referenced a lane this machine has + // not already reported. + await vi.advanceTimersByTimeAsync(2_000); + expect(callAction).toHaveBeenCalledTimes(4); expect(callAction).toHaveBeenLastCalledWith( "target-studio", "project-a", @@ -993,6 +1043,86 @@ describe("cross-machine refresh scheduling", () => { stop(); }); + // The bug this whole change exists to fix: a foreign machine's PR rows live in + // ITS database, so they were never fetched and its cards/headers rendered no + // PR badge until the project tab was rebound to that machine. + it("stores a foreign machine's PRs on its slice, and fetches them for a catch-up lane", async () => { + vi.useFakeTimers(); + const requests: Array<{ domain: string; action: string }> = []; + let sessionLaneId = "lane-known"; + const callAction = vi.fn(async ( + _targetId: string, + _projectId: string, + request: { domain: string; action: string }, + ) => { + requests.push({ domain: request.domain, action: request.action }); + if (request.domain === "lane") { + return { result: { lanes: [ + { id: "lane-known", name: "Known", branchRef: "feature/known" }, + { id: "lane-brand-new", name: "New", branchRef: "feature/new" }, + ] } }; + } + if (request.domain === "pr") { + return { result: { prs: [{ + id: "pr-foreign", + laneId: "lane-known", + headBranch: "feature/known", + githubPrNumber: 91, + githubUrl: "https://github.com/acme/repo-a/pull/91", + state: "open", + }] } }; + } + return { result: { sessions: [{ id: "session-1", laneId: sessionLaneId }] } }; + }); + window.ade = { + remoteRuntime: { + callAction, + getConnectionSnapshot: vi.fn(async () => ({ + connections: [{ + state: "connected", + target: { id: "target-studio", name: "Mac Studio (12)", hostname: "studio" }, + projects: [{ + projectId: "project-a", + rootPath: "/repo-a", + displayName: "Repo A", + gitOriginUrl: "git@github.com:acme/repo-a.git", + }], + }], + connectedCount: 1, + })), + onConnectionSnapshotChanged: vi.fn(() => () => {}), + }, + } as unknown as typeof window.ade; + + const stop = startCrossMachineLaneSync({ + scopeKey: "local:/repo-a", + repoDisplayName: "Repo A", + repoOriginUrl: "git@github.com:acme/repo-a.git", + boundTargetId: null, + boundProjectId: null, + }); + await Promise.resolve(); + await vi.advanceTimersByTimeAsync(400); + + const slice = () => useAppStore.getState().crossMachineLanesByMachineId["target-studio"]; + expect(requests.filter((entry) => entry.domain === "pr")).toHaveLength(1); + expect(slice()?.prs).toEqual([expect.objectContaining({ id: "pr-foreign", laneId: "lane-known" })]); + + // Chat-only ticks inside the lane window must not re-read PRs: they ride the + // lane cadence, not the 10s chat cadence. + await vi.advanceTimersByTimeAsync(21_000); + expect(requests.filter((entry) => entry.domain === "pr")).toHaveLength(1); + expect(slice()?.prs).toHaveLength(1); + + // A chat on a lane we have never seen forces an off-cadence lane read. That + // lane must arrive WITH its PR, or it renders blank for a full 30s cadence. + sessionLaneId = "lane-brand-new"; + await vi.advanceTimersByTimeAsync(10_500); + expect(requests.filter((entry) => entry.domain === "pr")).toHaveLength(2); + + stop(); + }); + it("does not let a late catch-up read suppress a new scope's first lane read", async () => { vi.useFakeTimers(); const requests: Array<{ domain: string; action: string }> = []; diff --git a/apps/desktop/src/renderer/state/crossMachineLanes.ts b/apps/desktop/src/renderer/state/crossMachineLanes.ts index 05fb4f65fe..80a9863b10 100644 --- a/apps/desktop/src/renderer/state/crossMachineLanes.ts +++ b/apps/desktop/src/renderer/state/crossMachineLanes.ts @@ -31,6 +31,7 @@ import type { AgentChatSession, LaneSummary, OpenProjectBinding, + PrSummary, RecentProjectSummary, RemoteRuntimeConnectionSnapshot, RemoteRuntimeConnectionState, @@ -762,6 +763,35 @@ export function decodeForeignLanes(result: unknown): LaneSummary[] { return lanes; } +/** + * Same contract as `decodeForeignLanes`, for `pr.listAll`. + * + * Validates every field the badge path actually reads, not just the joining + * ones: a peer on an older build that omits `githubPrNumber` would render + * "PR #undefined", and one that omits `githubUrl` would render a badge whose + * click is a silent no-op (the foreign click-through has nowhere else to go). + * Dropping the row shows no badge, which is honest; a half-decoded one is not. + */ +export function decodeForeignPrs(result: unknown): PrSummary[] { + const list = Array.isArray(result) + ? result + : isRecord(result) && Array.isArray(result.prs) + ? result.prs + : []; + const prs: PrSummary[] = []; + for (const candidate of list) { + if (!isRecord(candidate)) continue; + if (typeof candidate.id !== "string" || !candidate.id.trim()) continue; + if (typeof candidate.laneId !== "string" || !candidate.laneId.trim()) continue; + if (typeof candidate.headBranch !== "string") continue; + if (typeof candidate.githubPrNumber !== "number") continue; + if (typeof candidate.githubUrl !== "string" || !candidate.githubUrl.trim()) continue; + if (typeof candidate.state !== "string") continue; + prs.push(candidate as unknown as PrSummary); + } + return prs; +} + /** Same contract as `decodeForeignLanes`, for `session.list`. */ export function decodeForeignSessions(result: unknown): TerminalSessionSummary[] { const list = Array.isArray(result) @@ -897,9 +927,31 @@ async function readMachine( MACHINE_READ_TIMEOUT_MS, `lane.list on ${machineName}`, ); + // PRs ride the lane cadence, not the chat cadence. A PR is only ever rendered + // by joining it to a lane, it changes on the same slow scale a lane does, and + // the read is a foreign round trip — paying for it every ten seconds would + // buy a fresher check dot at the cost this cadence exists to avoid. + // Best-effort: a machine that answers `lane.list` but fails `pr.listAll` (an + // older build, a transient error) must still contribute its lanes and chats. + // No bound-machine skip needed: `isEligibleMachineOption` already excludes the + // tab's machine from every target this function is called with, so the rows + // read here are always a machine `useLanePrsByLaneId` cannot answer itself. + const readPrs = async (): Promise => { + try { + const response = await withTimeout( + callAction(targetId, projectId, { domain: "pr", action: "listAll", args: {} }), + MACHINE_READ_TIMEOUT_MS, + `pr.listAll on ${machineName}`, + ); + return decodeForeignPrs(response.result); + } catch { + return null; + } + }; try { - const [laneResult, sessionResult] = await Promise.all([ - shouldReadLanes(machineId) ? readLanes() : null, + const lanesDue = shouldReadLanes(machineId); + const [laneResult, sessionResult, duePrs] = await Promise.all([ + lanesDue ? readLanes() : null, withTimeout( callAction(targetId, projectId, { domain: "session", @@ -909,6 +961,7 @@ async function readMachine( MACHINE_READ_TIMEOUT_MS, `session.list on ${machineName}`, ), + lanesDue ? readPrs() : null, ]); // Cancellation: a scope change or a newer snapshot bumped the generation // while this read was in flight, so its answer is about a different world. @@ -925,6 +978,15 @@ async function readMachine( async () => decodeForeignLanes((await readLanes()).result), ); if (generation !== runtime.generation) return; + // On a cadence tick the PR read already went out alongside the lane read, so + // it costs no extra latency. Only the off-cadence catch-up (a chat on a lane + // we had never seen forces a lane read mid-tick) has to fetch here — without + // it that lane renders with no PR badge until the next 30s tick, the exact + // blank this change exists to remove. Deliberately NOT a blanket sequential + // read: that would hold this machine's lanes and chats out of the store + // behind an 8s PR timeout, and stall every other machine's cadence with it. + const prResult = duePrs ?? (lanes && !lanesDue ? await readPrs() : null); + if (generation !== runtime.generation) return; store.mergeCrossMachineLanes({ machineId, machineName, @@ -938,6 +1000,9 @@ async function readMachine( // happens to fire. Omitting the flag retains that verdict. ...(isMachineEligibleNow(machineId) ? { online: true } : {}), ...(lanes ? { lanes } : {}), + // Same retention contract as lanes: a PR read that failed or was not due + // this tick omits the key, so the machine keeps what it last reported. + ...(prResult ? { prs: prResult } : {}), sessions: reconcileCrossMachineOptimisticSessions(binding, sessions), error: null, }); @@ -969,9 +1034,27 @@ async function readThisMachine( MACHINE_READ_TIMEOUT_MS, "lane.list on This Mac", ); + // See `readMachine`: PRs ride the lane cadence for the same reasons, and the + // read is best-effort — a machine that fails only this read must still + // contribute its lanes and chats rather than falling into the error path and + // blanking the whole slice. No active-binding skip here: this function only + // runs when the tab is bound to a REMOTE machine (see its one caller), so + // This Mac is never the machine `useLanePrsByLaneId` already answers. + const readPrs = async (): Promise => { + try { + return await withTimeout( + window.ade.prs.listAll(binding), + MACHINE_READ_TIMEOUT_MS, + "pr.listAll on This Mac", + ); + } catch { + return null; + } + }; try { - const [laneResult, sessions] = await Promise.all([ - shouldReadLanes(THIS_MACHINE_ID) ? readLanes() : null, + const lanesDue = shouldReadLanes(THIS_MACHINE_ID); + const [laneResult, sessions, duePrs] = await Promise.all([ + lanesDue ? readLanes() : null, withTimeout( window.ade.sessions.list( { limit: FOREIGN_SESSION_LIMIT }, @@ -980,6 +1063,7 @@ async function readThisMachine( MACHINE_READ_TIMEOUT_MS, "session.list on This Mac", ), + lanesDue ? readPrs() : null, ]); if (generation !== runtime.generation) return; const lanes = await resolveLaneCadence( @@ -990,6 +1074,10 @@ async function readThisMachine( readLanes, ); if (generation !== runtime.generation) return; + // See `readMachine`: parallel on a cadence tick, sequential only for the + // off-cadence catch-up, so a slow PR read never holds back lanes and chats. + const prs = duePrs ?? (lanes && !lanesDue ? await readPrs() : null); + if (generation !== runtime.generation) return; store.mergeCrossMachineLanes({ machineId: THIS_MACHINE_ID, machineName: THIS_MACHINE_NAME, @@ -998,6 +1086,7 @@ async function readThisMachine( binding, online: true, ...(lanes ? { lanes } : {}), + ...(prs ? { prs } : {}), sessions: reconcileCrossMachineOptimisticSessions(binding, sessions), error: null, }); diff --git a/apps/desktop/src/renderer/webclient/adapter/prs.ts b/apps/desktop/src/renderer/webclient/adapter/prs.ts index 4cde989c9f..6a547d519b 100644 --- a/apps/desktop/src/renderer/webclient/adapter/prs.ts +++ b/apps/desktop/src/renderer/webclient/adapter/prs.ts @@ -4,6 +4,7 @@ import type { PrSummary, } from "../../../shared/types"; import type { AdapterInfra, AdeNamespace } from "./types"; +import { assertWebRuntimePinUnsupported } from "./runtimePinGuard"; export function createPrsNamespace(infra: AdapterInfra): AdeNamespace<"prs"> { const { commands, events } = infra; @@ -68,14 +69,17 @@ export function createPrsNamespace(infra: AdapterInfra): AdeNamespace<"prs"> { linkToLane: (args: unknown) => call("prs.linkToLane", args, null, false), preflightCreateLaneFromPrBranch: (args: unknown) => call("prs.preflightCreateLaneFromPrBranch", args, null), createLaneFromPrBranch: (args: unknown) => call("prs.createLaneFromPrBranch", args, null, false), - getForLane: async (laneId: string) => - (await mobileSnapshot()).prs.find((pr) => pr.laneId === laneId) ?? null, + getForLane: async (laneId: string, pin?: unknown) => { + assertWebRuntimePinUnsupported("prs.getForLane", pin); + return (await mobileSnapshot()).prs.find((pr) => pr.laneId === laneId) ?? null; + }, // Manual ⟳ PR-sync (ChatGitToolbar) — force a fresh reconcile for one lane. // Mirrors the preload runtime action `pr.syncLanePr` (single positional // laneId); the sync command layer marshals a named record, exactly like the // `prs.getForLane` host handler that reads `{ laneId }`. It mutates, so the // read cache is dropped afterward like `refresh` does. - syncLanePr: async (laneId: string) => { + syncLanePr: async (laneId: string, pin?: unknown) => { + assertWebRuntimePinUnsupported("prs.syncLanePr", pin); const result = await call("prs.syncLanePr", { laneId }, null, false); commands.invalidateCache(["prs."]); return result; @@ -86,17 +90,33 @@ export function createPrsNamespace(infra: AdapterInfra): AdeNamespace<"prs"> { await call("prs.reconcileOnFocus", { force: true }, undefined, false); commands.invalidateCache(["prs."]); }, - listAll: async () => (await mobileSnapshot()).prs, + listAll: async (pin?: unknown) => { + assertWebRuntimePinUnsupported("prs.listAll", pin); + return (await mobileSnapshot()).prs; + }, listOpenForRepo: () => read("prs.listOpenForRepo", {}, []), - refresh: async (args?: unknown) => { + refresh: async (args?: unknown, pin?: unknown) => { + assertWebRuntimePinUnsupported("prs.refresh", pin); const result = await call("prs.refresh", args, [], false); commands.invalidateCache(["prs."]); return arrayField(result, "prs"); }, - getStatus: (prId: string) => read("prs.getStatus", { prId }, null), - getChecks: (prId: string) => read("prs.getChecks", { prId }, []), - getComments: (prId: string) => read("prs.getComments", { prId }, []), - getReviews: (prId: string) => read("prs.getReviews", { prId }, []), + getStatus: (prId: string, pin?: unknown) => { + assertWebRuntimePinUnsupported("prs.getStatus", pin); + return read("prs.getStatus", { prId }, null); + }, + getChecks: (prId: string, pin?: unknown) => { + assertWebRuntimePinUnsupported("prs.getChecks", pin); + return read("prs.getChecks", { prId }, []); + }, + getComments: (prId: string, pin?: unknown) => { + assertWebRuntimePinUnsupported("prs.getComments", pin); + return read("prs.getComments", { prId }, []); + }, + getReviews: (prId: string, pin?: unknown) => { + assertWebRuntimePinUnsupported("prs.getReviews", pin); + return read("prs.getReviews", { prId }, []); + }, getReviewThreads: (prId: string) => read("prs.getReviewThreads", { prId }, []), updateDescription: async (args: unknown) => { await call("prs.updateDescription", args, undefined, false); @@ -173,7 +193,10 @@ export function createPrsNamespace(infra: AdapterInfra): AdeNamespace<"prs"> { return result; }, listIntegrationWorkflows: (args?: unknown) => call("prs.listIntegrationWorkflows", args, []), - onEvent: (listener: (event: unknown) => void) => events.on("prsEvent", listener as never), + onEvent: (listener: (event: unknown) => void, pin?: unknown) => { + assertWebRuntimePinUnsupported("prs.onEvent", pin); + return events.on("prsEvent", listener as never); + }, getDetail: (prId: string) => read("prs.getDetail", { prId }, null), getFiles: (prId: string) => read("prs.getFiles", { prId }, []), getCommits: (prId: string) => read("prs.getCommits", { prId }, []), diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0c69c8be25..f127747de8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -623,6 +623,7 @@ Related feature docs: [Chat](./features/chat/README.md), [Agents](./features/age - Runtime-backed event subscriptions can merge local Electron IPC and the runtime event stream behind one renderer API. For example, `window.ade.lanes.onLifecycleEvent` listens to `ade.lanes.lifecycle.event` for desktop-local fallback paths and to runtime `lane_lifecycle_event` payloads for local-brain or SSH-bound windows. Runtime stream results (`RemoteRuntimeStreamEventsResult` in `apps/desktop/src/shared/types/remoteRuntime.ts`) carry `eventEpoch`, `gap`, and `oldestCursor`; preload resets cursors/dedupe on epoch changes and notifies project-binding refresh paths when a gap means replay history was evicted. - **Pinned event pumps.** A window runs several independent event pumps at once: the active binding's pump (in `preload.ts`) plus, for every binding the window has open but is *not* bound to, the pumps in `apps/desktop/src/preload/pinnedRuntimeEvents.ts`. That module owns one shared PTY pump per pinned binding (fed by both polling and `ade.runtime.event` push notifications, with its own cursor, epoch generation, dedupe ring, and failure backoff) and a per-listener generic pump used by pinned chat/project subscriptions; the active pump shares its epoch-normalization, stale-event, and dedupe helpers. Each pinned binding's PTY pump cannot share the active pump's single mutable cursor — switching the window binding would reset it, and polling two machines through it would cross-contaminate epoch and dedup state. Main-side subscriptions are reference-counted per `(binding, category)` so several pumps on the same binding share one, and preload sends `ade.runtime.events.release` when the last of them goes away (see [§5.4](#54-event-subscriptions-push-not-poll)). A pushed event announcing a *new* epoch is not dispatched: it rewinds the cursor so a local pin replays the restarted buffer and a remote pin re-anchors to the live head with `{ cursor: 0, replay: false }`. - Pinned APIs are opt-in trailing arguments, never a mode switch: `pty.create` / `resumeSession` / `sendToSession` / `write` / `resize` / `dispose` / `setDataSubscriptions` / `onData` / `onExit`, `terminal.preview`, `sessions.get` / `list` / `readTranscriptTail`, and the chat/session APIs all accept an optional `OpenProjectBinding`. Omitting it takes the byte-for-byte unchanged bound path plus its local IPC fallback (`callPinnedOrBoundRuntimeActionOr`). The hosted web adapter cannot route a pin to a second host, so its shims reject one loudly (`assertWebRuntimePinUnsupported`) instead of silently answering from the single web host. +- The `pr` read surface is pinned the same way, because a lane's PR row lives in the `.ade` database of the machine that owns the lane. `callPrReadRuntimeActionOr(pin, action, request, local)` delegates to `callPinnedOrBoundRuntimeActionOr`, and `prs.getForLane` / `syncLanePr` / `listAll` / `refresh` / `getStatus` / `getChecks` / `getComments` / `getReviews` plus the `prs.onEvent` subscription all take the trailing pin. Here `pin: null` is a stated choice rather than an absence — it means "the machine the project tab is bound to", which is exactly what the bound-machine-scoped PRs tab wants — so every tab-scoped call site passes it explicitly rather than relying on the default. A pinned subscription must follow its reads: the bound runtime's `prs-updated` feed describes a different database and would leave a pinned PR pill permanently stale. See [features/pull-requests/README.md](./features/pull-requests/README.md#which-machine-answers-a-pr-read). - `contextIsolation: true`, `nodeIntegration: false`, `sandbox: false` (required for preload functionality). - Global window type: `apps/desktop/src/preload/global.d.ts`. - `window.ade.sessions.settle(sessionId, { outcome?, @@ -902,7 +903,7 @@ Electron renderer runtime does **not** wrap the app in `React.StrictMode`. Brows - Narrow selectors on components to minimize re-renders. - `refreshLanes` accepts independent lane-status and lane-snapshot flags. Callers can refresh cheap runtime snapshot decorations without recomputing git status, or update git status without rebuilding conflict/rebase/auto-rebase overlays; statusless refreshes preserve the previous `LaneStatus`/`parentStatus` in store so the UI does not flicker to unknown git state. - Per-project work-view state keyed by project identity (`WorkProjectViewState`): local bindings use their root path, while remote bindings use `OpenProjectBinding.key` (`remote::`). Alongside Work filters, collapsed section ids, and right-sidebar state, version 3 includes the Lanes tab's filter, pinned lane ids, and expanded lane id. Version 4 adds the Work-sidebar-only `workPinnedLaneIds`, lane sort mode, sparse manual lane order, and structured session chips; normalization is additive, so a v3 blob retains the former Created order with no Work pins or chips. It keeps Settled reachable as a quiet collapsed tail: Status/Time mode use `status:settled`, Lane mode uses explicit `settled-open:` markers, and a fully quiet lane uses the inverted `lane-open:` marker. There is no independent Tiers/Show settled filter. The one-time status-collapse migration is gated against its own version-2 threshold, so later additive schema bumps do not re-collapse a section the user expanded. Persistence under `ade.workViewState.v1` is a scoped delta read-modify-write: every mounted project store upserts or deletes only the project/lane keys it owns instead of flushing its stale copy of the whole map. `refreshLanes` prunes lane scopes only from a non-empty lane inventory and only inside that store's project scope. `registerProjectSurfaceStore` / `workViewStoreForProject` route project-specific writes made by `AppShell` and `TopBar`, which render above `AppStoreProvider`, to the owning surface store. Lane/status deeplinks are transient view overrides and user filter, grouping, chip, pin, or ordering changes return to and then update the saved base. The right-edge fields are `workSidebarOpen`, `workSidebarTab` (`"git" | "files" | "ios" | "app-control" | "browser"`), and `workSidebarWidthPct` (clamped 26–55). The sidebar consolidates lane-scoped tools that were previously split across separate floating panes; per-chat iOS / App Control drawers still exist on `AgentChatPane` but are suppressed when the chat is mounted as a Work tile so the sidebar owns those surfaces at lane scope. Remote-bound Work sidebars expose only the runtime-backed Git and Files tabs; local-only iOS Simulator, App Control, and Browser panes stay hidden. The `browser` tab is not lane-scoped on local bindings: each ADE window/project keeps its own tab collection and inspect state, while browser authentication storage is global to the installation/channel through `persist:ade-browser`. -- Cross-machine Work union. `crossMachineLanesByMachineId` + `crossMachineLaneScopeKey` (fed by `renderer/state/crossMachineLanes.ts`) hold every *other* connected machine's lanes and sessions for the repository the active tab is showing, so the Work sidebar can list work in flight anywhere — chats and CLI/shell sessions alike — without changing the tab's binding. Keyed by machine because a lane owns its machine (`lanes.worktree_path` is an absolute path on exactly one machine) and sessions inherit theirs through `laneId` — there is no per-session machine field. `mergeCrossMachineLanes` retains omitted `lanes`/`sessions` so a failed read leaves the machine's rows on screen, and `setCrossMachineMachinesOnline` flags rather than deletes an entry that goes offline — the sidebar renders it dimmed, collapsed, and inert, and the retained slice also backs the push-divergence guard. Presence is decided in one place, `applyReachability`, from connection state alone: a drop is believed only after a reconnect attempt has completed and failed (`connecting` observed, then a non-connected state) plus a 45 s floor, with a 120 s ceiling for a dial that never finishes. Two states have no attempt left to wait for and dim on the floor alone: `idle`, which will not redial on its own, and `connected` but unable to re-prove this repository, which answers yet is never read for it. The floor and the ceiling are one deadline, not two rules. The `connecting`/`error` states a redial or sleep/wake publishes therefore do not reflow the sidebar, and reconnecting is applied instantly. The verdict belongs to the store rather than the tick: a machine that is already dimmed stays dimmed until it is eligible again, so a Work-tab remount — which tears down the shared runtime and its drop records while the store slice survives — cannot re-brighten it for another floor; the retention deadline is re-anchored to that machine's last successful read instead. Only `dropCrossMachineLanes` deletes, and only for a target gone from the snapshot, a connected machine that positively reports the repository missing *and* has a resolvable origin to prove it by (`repoMatchFor` will say "missing" off a folder-name mismatch alone, and the scope's origin is transiently null while the bound machine blips), or 24 hours unreachable. The scope key is per repository, so a project-tab switch invalidates the union wholesale. A detached chat launch whose runtime pin differs from the active binding seeds an optimistic summary directly into the owning machine slice; the binding/session-keyed pending record survives stale in-flight list responses and is replaced by the authoritative row with the same stable session id (or pruned on delete/expiry). This keeps the active binding free of foreign UUID-lane placeholders while avoiding a blank interval before the next remote list arrives. Foreign lanes use the same `sessionFilingBucket` active/snoozed/settled partition, fully quiet collapsed header, and quiet-tail renderer as local lanes, with composite machine/lane persistence keys. Their **Manage lane** dialog and mutations carry the same owning binding. Sessions whose referenced lane is absent render as explicit warning-tinted **Orphaned sessions** groups with a refresh-only recovery action; ADE preserves the sessions and never interprets unknown ownership as permission to mutate the active machine. Refreshes ride the connection-snapshot subscription plus existing lane-lifecycle/session-changed events (coalesced, bounded, timed out, capped in parallelism) and a fallback loop for machines with no renderer change feed; that loop is paused entirely while the window is hidden, re-reads chats every 10 s, and re-reads lanes on a 30 s cadence, because `lane.list` with `includeStatus` costs a git status per lane on the other machine. A chat naming a lane that machine has never reported forces the lane read immediately, but only once: lane ids a completed read did not explain are remembered until the next one, because `session.list` does not filter on lane status while `lane.list` excludes archived lanes, so a chat on an archived lane is permanently unresolvable and would otherwise demand the expensive read on every tick forever. Foreign reads never gate the local list. `selectOtherMachineBranchStates` is the memoized selector the push-divergence guard reads at click time. +- Cross-machine Work union. `crossMachineLanesByMachineId` + `crossMachineLaneScopeKey` (fed by `renderer/state/crossMachineLanes.ts`) hold every *other* connected machine's lanes, sessions, and mapped PR rows for the repository the active tab is showing, so the Work sidebar can list work in flight anywhere — chats and CLI/shell sessions alike, each with its own PR badge — without changing the tab's binding. Keyed by machine because a lane owns its machine (`lanes.worktree_path` is an absolute path on exactly one machine) and both sessions and PR rows inherit theirs through `laneId` — there is no per-session or per-PR machine field. PRs are read on the lane cadence rather than the chat cadence and ride along with the lane read (see [features/pull-requests/README.md](./features/pull-requests/README.md#which-machine-answers-a-pr-read)). `mergeCrossMachineLanes` retains omitted `lanes`/`sessions`/`prs` so a failed read leaves the machine's rows on screen, and `setCrossMachineMachinesOnline` flags rather than deletes an entry that goes offline — the sidebar renders it dimmed, collapsed, and inert, and the retained slice also backs the push-divergence guard. Presence is decided in one place, `applyReachability`, from connection state alone: a drop is believed only after a reconnect attempt has completed and failed (`connecting` observed, then a non-connected state) plus a 45 s floor, with a 120 s ceiling for a dial that never finishes. Two states have no attempt left to wait for and dim on the floor alone: `idle`, which will not redial on its own, and `connected` but unable to re-prove this repository, which answers yet is never read for it. The floor and the ceiling are one deadline, not two rules. The `connecting`/`error` states a redial or sleep/wake publishes therefore do not reflow the sidebar, and reconnecting is applied instantly. The verdict belongs to the store rather than the tick: a machine that is already dimmed stays dimmed until it is eligible again, so a Work-tab remount — which tears down the shared runtime and its drop records while the store slice survives — cannot re-brighten it for another floor; the retention deadline is re-anchored to that machine's last successful read instead. Only `dropCrossMachineLanes` deletes, and only for a target gone from the snapshot, a connected machine that positively reports the repository missing *and* has a resolvable origin to prove it by (`repoMatchFor` will say "missing" off a folder-name mismatch alone, and the scope's origin is transiently null while the bound machine blips), or 24 hours unreachable. The scope key is per repository, so a project-tab switch invalidates the union wholesale. A detached chat launch whose runtime pin differs from the active binding seeds an optimistic summary directly into the owning machine slice; the binding/session-keyed pending record survives stale in-flight list responses and is replaced by the authoritative row with the same stable session id (or pruned on delete/expiry). This keeps the active binding free of foreign UUID-lane placeholders while avoiding a blank interval before the next remote list arrives. Foreign lanes use the same `sessionFilingBucket` active/snoozed/settled partition, fully quiet collapsed header, and quiet-tail renderer as local lanes, with composite machine/lane persistence keys. Their **Manage lane** dialog and mutations carry the same owning binding. Sessions whose referenced lane is absent render as explicit warning-tinted **Orphaned sessions** groups with a refresh-only recovery action; ADE preserves the sessions and never interprets unknown ownership as permission to mutate the active machine. Refreshes ride the connection-snapshot subscription plus existing lane-lifecycle/session-changed events (coalesced, bounded, timed out, capped in parallelism) and a fallback loop for machines with no renderer change feed; that loop is paused entirely while the window is hidden, re-reads chats every 10 s, and re-reads lanes on a 30 s cadence, because `lane.list` with `includeStatus` costs a git status per lane on the other machine. A chat naming a lane that machine has never reported forces the lane read immediately, but only once: lane ids a completed read did not explain are remembered until the next one, because `session.list` does not filter on lane status while `lane.list` excludes archived lanes, so a chat on an archived lane is permanently unresolvable and would otherwise demand the expensive read on every tick forever. Foreign reads never gate the local list. `selectOtherMachineBranchStates` is the memoized selector the push-divergence guard reads at click time. - Project tab bookkeeping. `openProjectTabRoots: string[]` tracks local roots open in the window (mirrored to the main process via `ade.app.setWindowProjectTabs` so background services keep those projects warm); `openRemoteProjectTabs` tracks full remote bindings so inactive remote tabs remain first-class retained surfaces. `ProjectTabHost` applies one shared eight-surface LRU across local and remote tabs: inactive mounted surfaces are hidden, inert, and animation-paused, while an open surface that falls outside the bound snapshots its scoped state back into the root caches before unmounting. `projectInfoByRoot: Record` caches local `ProjectInfo` payloads for tab favicons and offline tab rendering. - Stale-while-revalidate switch caches. `laneSelectionByProject` remembers the `{ laneId, sessionId }` selection per project identity so switching tabs lands on the lane/chat the user last had open instead of "first lane". `laneCacheByProject` mirrors the last good `{ lanes, laneSnapshots }`; local and remote switches apply it immediately (no spinner, no chat-pane unmount) and refresh silently in the background. `sessionsCacheByProject` does the same for `useWorkSessions` so chat tabs and terminal grids do not blank during a tab swap. `projectRouteStorage.ts` persists the last route under the binding key, independent of whether the surface is currently mounted. Cache pruning retains every open local root and remote binding key; tab close and target disconnect deliberately evict only their affected remote state. The two eviction paths are distinct: `evictProjectState(key)` + `removeStoredProjectRoute(key)` is the full "forget this surface" wipe used by an explicit tab close and by removing a machine, while `evictProjectDataCaches(key)` is the disconnect-only sibling that drops just the snapshots that can go stale while a remote is unreachable (`laneCacheByProject`, `laneSelectionByProject`, `sessionsCacheByProject`, and the persisted lane cache) and preserves `workViewByProject` / `laneWorkViewByScope` plus the stored route so reconnecting restores the chat or tile that was open. `closeProject({ preserveRemoteViewState: true })` applies the same narrower rule when a disconnect closes the last remaining tab. - `projectRevision` is a monotonically incrementing counter bumped inside `setProject` whenever the active project root actually changes. Long-lived renderer-side caches (most notably the module-level xterm runtime cache in `TerminalView.tsx`, whose key also carries the session's runtime pin) combine it with the project identity key, so identical paths on different remote targets cannot share PTY runtimes. All project-transition paths (`refreshProject`, `openRepo`, `switchProjectToPath`, `closeProject`) go through `setProject` to keep the counter honest. diff --git a/docs/features/chat/README.md b/docs/features/chat/README.md index e73297357a..4fa59db1f7 100644 --- a/docs/features/chat/README.md +++ b/docs/features/chat/README.md @@ -96,8 +96,8 @@ for its separate RPC, sync, storage, and UI contracts. | `apps/desktop/src/renderer/components/chat/SubagentActivityCards.tsx` | Inline subagent transcript cards mounted by `AgentChatMessageList` from the render events `chatTranscriptRows.ts` derives. `SubagentSpawnCard` anchors where the agent started (identicon/colour from `chatSubagentIdentity`, task title, agent-type/background chips, a single live `running · · tools · ` line that ticks each second, and a `jump to result` link once the agent ends); `SubagentResultCard` renders at the settle position (status + duration, ~2-line report preview, View transcript, `jump to start`, warm amber tones for stopped/failed instead of red error blocks); `BackgroundFinishChip` is the one-line finish chip for backgrounded shell commands; `SubagentStoppedGroupCard` collapses a run of interrupt-stopped subagents into one amber "N agents stopped when you interrupted" line that expands to a per-agent list with `jump to start` links. All inherit `--chat-accent`. | | `apps/desktop/src/renderer/components/chat/spawnNavigation.ts` | One canonical `navigateToSpawnedChat(sessionId, laneId?)` helper that dispatches the `ade:work:select-session` window event (behind a try/catch, no-op on a falsy id). Every spawn surface routes through it: the inline `SubagentSpawnCard`, `spawn_wake_divider` and `spawn_completed` completion rows, spawned-chat rows in `ChatSubagentsPanel`, the `AgentChatPane` parent-thread breadcrumb, and the `SessionCard` lineage glyph. `TerminalsPage` resolves an omitted lane from the loaded session list before focusing the target, so cross-lane jumps land on the correct lane. | | `apps/desktop/src/renderer/components/chat/ChatActionsDrawerPanel.tsx`, `ChatSourcesPanel.tsx`, `chatSources.ts` | Codex Chat Actions source inventory. Sources is the first available tab and derives a deduplicated list of attachments/files, web searches/results, MCP apps/tools, and external resource URLs from the current transcript. HTTP(S) rows open in ADE's built-in browser; internal `node_repl` plumbing and unsafe protocols are excluded. | -| `apps/desktop/src/renderer/components/chat/ChatGitToolbar.tsx` | Git / PR quick-action toolbar above the composer. If the lane already has a linked PR, the PR button opens or toggles that PR; otherwise it routes to the PR workspace with a create-PR handoff (`create=1&sourceLaneId=&target=primary`). When the chat PR pane or compact PR menu opens, it asks `prReadCache.refreshLinkedPrCoalesced` for a targeted `prs.refresh({ prIds })` so the badge picks up merged/closed/check transitions without broad GitHub polling. An unmapped lane PR (a `github_pr_projections`-derived summary with `pr.unmapped === true` and a synthetic `gh:` id) has no DB row to refresh or fetch checks for, so both the live refresh and `getChecks` are skipped for it. The toolbar is a **status strip only** — the manual PR-sync (↻) control lives in the PR pane's title bar, so surfaces that render the toolbar without a PR pane heal through reconcile-on-focus and `prs-updated` instead. | -| `apps/desktop/src/renderer/components/chat/ChatPrPane.tsx`, `ChatPrInlineCreator.tsx` | Left floating PR pane for Work chat, with its own title bar (`Pull request` + ↻ refresh + ✕ close) — the manual PR-sync control lives here, not in `ChatGitToolbar`. Renders cached lane PR details immediately, then performs the same cooldown-bound targeted PR refresh as the toolbar before settling the state. Terminal PRs hide stale running-check labels so merged/closed PRs do not keep showing in-progress CI from an old cache row. An unmapped (`pr.unmapped`) lane PR skips the live refresh and the checks/reviews/status enrichment, since it has no DB row behind the synthetic `gh:` id. With no PR it embeds `ChatPrInlineCreator`, whose title defaults to the chat session title. The pane's open/closed state is per chat and persisted across restarts through `chatCompanionUiState` — see [Composer and chat UI](composer-and-ui.md#source-file-map). | +| `apps/desktop/src/renderer/components/chat/ChatGitToolbar.tsx` | Git / PR quick-action toolbar above the composer. If the lane already has a linked PR, the PR button opens or toggles that PR; otherwise it routes to the PR workspace with a create-PR handoff (`create=1&sourceLaneId=&target=primary`). When the chat PR pane or compact PR menu opens, it asks `prReadCache.refreshLinkedPrCoalesced` for a targeted `prs.refresh({ prIds })` so the badge picks up merged/closed/check transitions without broad GitHub polling. An unmapped lane PR (a `github_pr_projections`-derived summary with `pr.unmapped === true` and a synthetic `gh:` id) has no DB row to refresh or fetch checks for, so both the live refresh and `getChecks` are skipped for it. The toolbar is a **status strip only** — the manual PR-sync (↻) control lives in the PR pane's title bar, so surfaces that render the toolbar without a PR pane heal through reconcile-on-focus and `prs-updated` instead. It takes an optional `runtimePin`: a lane's PR row lives in its own machine's database, so a chat on another machine reads and subscribes through that machine's runtime rather than showing the bare create-PR button for a session that already has one. Effects key on the pin's `key`, not the object, which is rebuilt on every cross-machine merge. Under a pin the unpinned `diff.getChanges` status read is skipped and PR *creation* is withheld — see [Pull requests](../pull-requests/README.md#which-machine-answers-a-pr-read). | +| `apps/desktop/src/renderer/components/chat/ChatPrPane.tsx`, `ChatPrInlineCreator.tsx` | Left floating PR pane for Work chat, with its own title bar (`Pull request` + ↻ refresh + ✕ close) — the manual PR-sync control lives here, not in `ChatGitToolbar`. Renders cached lane PR details immediately, then performs the same cooldown-bound targeted PR refresh as the toolbar before settling the state. Terminal PRs hide stale running-check labels so merged/closed PRs do not keep showing in-progress CI from an old cache row. An unmapped (`pr.unmapped`) lane PR skips the live refresh and the checks/reviews/status enrichment, since it has no DB row behind the synthetic `gh:` id. With no PR it embeds `ChatPrInlineCreator`, whose title defaults to the chat session title — unless the pane carries a `runtimePin`, in which case it says `Switch to to open one`, because the creator derives its branch, base, and Linear links from the bound machine's lanes and `createFromLane` is unpinned. Reads and the event subscription do follow the pin. The pane's open/closed state is per chat and persisted across restarts through `chatCompanionUiState` — see [Composer and chat UI](composer-and-ui.md#source-file-map). | | `apps/desktop/src/renderer/lib/visualContextFormatting.ts` | Serializes iOS, App Control, built-in browser, and attachment context into prompt text. | | `apps/desktop/src/renderer/components/chat/RewindFilesConfirmDialog.tsx`, `rewindFilesPreview.ts` | Chat file-rewind confirmation surface. Claude uses the SDK `rewindFiles` control call; Codex uses ADE's git-backed file restore plan plus a version-gated app-server call — `thread/fork` before the target turn on servers >= 0.145.0, and the deprecated `thread/rollback` fallback on <= 0.144 or when the target turn has no usable id (see [agent-routing.md](./agent-routing.md#codex-rewind-and-0145-readiness)). `rewindFilesPreview.ts` maps the selected user message to turn diff summaries and per-file SHA ranges; the dialog lists every restored file, expands rows into `AdeDiffViewer`, and confirms the provider rewind without using browser-native confirm UI. | | `apps/desktop/src/renderer/components/chat/ChatSubagentsPanel.tsx`, `chatExecutionSummary.ts`, `chatSubagentIdentity.tsx`, `codex/CodexGoalCard.tsx` | Chat Info drawer content: Codex goal card, capped/collapsible plan and task sections, and capped Subagents/Background/Schedule rosters. Running subagent and background durations derive from the wall clock and tick once per second; terminal rows retain their final compact duration. Terminal work moves into one **Completed** disclosure without reordering survivors; failed and pinned rows stay active; Clear hides only terminal Completed rows and Restore reverses it. Schedule pause/play remains in the Schedule header, recurring rows show last-run plus next-fire timing, and each active ADE-managed durable row exposes Cancel; provider-only/non-durable transcript rows stay visible without a false cancellation control. Spawned-chat snapshots carry a derived `childSessionId`; the derivation preserves the `chat:` task id and `spawnKind` when the canonical dotted lifecycle twin merges into the underscore event. Their roster rows show the child's live session title, put the runtime in the small kind chip, and navigate directly to the child instead of opening a provider transcript drawer. `chatSubagentIdentity.tsx` centralizes deterministic identity and exposes status-optional, size-configurable glyphs for both roster state and compact lineage cues. | diff --git a/docs/features/chat/composer-and-ui.md b/docs/features/chat/composer-and-ui.md index 415ce24cd1..e09990c71b 100644 --- a/docs/features/chat/composer-and-ui.md +++ b/docs/features/chat/composer-and-ui.md @@ -46,13 +46,13 @@ subagents, computer use). The pane derives all visible state from the | `ChatBuiltInBrowserPanel.tsx` | In-app browser panel mounted under the Work right-edge sidebar's `browser` tab. Renders the address bar, navigation/tab strip, inspect toolbar, screenshot capture, and an empty/error state derived from `BuiltInBrowserStatus`; the actual page content is painted by a main-process `WebContentsView` whose bounds the panel reports back to the broker via `ade.builtInBrowser.setBounds`. Inspect-mode hit-tests emit `BuiltInBrowserContextItem` payloads through `onAddContext`; the sidebar then dispatches `ade:agent-chat:add-builtin-browser-context` to the active chat. The panel does not run inside `AgentChatPane` directly — instead, anywhere in the renderer that wants to open a URL calls `openUrlInAdeBrowser()` (in `apps/desktop/src/renderer/lib/openExternal.ts`), which fires `ADE_OPEN_BUILT_IN_BROWSER_EVENT` and asks the broker to open a new tab. | | `ChatTerminalDrawer.tsx` | Collapsible terminal drawer at the bottom of the chat. | | `ChatGitToolbar.tsx` | Git status and quick-action toolbar above the composer. The PR action opens or toggles a linked PR when one exists, otherwise opens the PR creation handoff for the current lane targeting the primary branch. Opening the chat PR pane or compact PR menu performs a targeted, cooldown-bound refresh for that single linked PR. The toolbar is a **status strip only** — the manual PR-sync (↻) button moved into the PR pane's title bar, so surfaces that render this toolbar without a PR pane have no manual sync affordance and heal through reconcile-on-focus plus `prs-updated` instead. | -| `ChatPrPane.tsx` | Left floating PR pane for Work chat. Owns a title bar (`Pull request` + ↻ refresh + ✕ close): ↻ calls `prs.syncLanePr` and then re-reads the pane's PR, and spins for either a manual sync or a backend reconcile-on-focus (`pr-reconcile`, debounced 300 ms on the hide so a fast reconcile does not flicker). ✕ is wired to the parent's `onClose` (the header PR pill still toggles it). Shows cached lane PR details immediately, then refreshes the linked PR row with the same targeted refresh path so pane toggles surface current merged/closed/check state without a broad PR sync. An unmapped lane PR (projection-derived, `pr.unmapped`) skips the refresh and checks/reviews enrichment — there is no DB row behind its synthetic `gh:` id. With no PR it embeds `ChatPrInlineCreator` and forwards the chat's `sessionTitle`. | +| `ChatPrPane.tsx` | Left floating PR pane for Work chat. Owns a title bar (`Pull request` + ↻ refresh + ✕ close): ↻ calls `prs.syncLanePr` and then re-reads the pane's PR, and spins for either a manual sync or a backend reconcile-on-focus (`pr-reconcile`, debounced 300 ms on the hide so a fast reconcile does not flicker). ✕ is wired to the parent's `onClose` (the header PR pill still toggles it). Shows cached lane PR details immediately, then refreshes the linked PR row with the same targeted refresh path so pane toggles surface current merged/closed/check state without a broad PR sync. An unmapped lane PR (projection-derived, `pr.unmapped`) skips the refresh and checks/reviews enrichment — there is no DB row behind its synthetic `gh:` id. With no PR it embeds `ChatPrInlineCreator` and forwards the chat's `sessionTitle`; under a `runtimePin` it points at the owning machine instead, since creation is not pinned. Reads, the ↻ sync, and the event subscription all take the pin so a chat on another machine sees its lane's real PR. | | `ChatPrInlineCreator.tsx` | Inline create-PR form inside the PR pane. Laid out as a **flow** with no uppercase section captions: a flat, boxless source row (lane name + branch + lock glyph, immutable), a `↓` connector carrying `N ahead · N behind · clean`/`dirty` from `lane.status` (muted `comparing…` when the lane has no status yet), then the canonical `LaneCombobox` target dropdown (no free text), title, description, and Create. The title defaults to the chat session title whenever it is a real title (the placeholder `New chat` never wins), otherwise to the ` -> ` derivation. Linear magic words and the deeplink footer are added server-side by `prService`. On success it hands the created `PrSummary` up through `onCreated` so the pane swaps to details without waiting for `prs-updated`. | | `ChatUserMinimap.tsx`, `chatUserMinimap.logic.ts` | Tick rail down the transcript's **left** gutter, one hairline per user message, gated on the `chatUserMinimapEnabled` appearance setting and mouse pointers only (`[@media(pointer:fine)]`). Ticks are positioned by percentage of rail height, so they compress instead of overflowing and there is no marker cap or subsampling — the entry index stays 1:1 with the tick index, which is what pointer→index mapping depends on. The whole rail is a single `