-
-
-
- {tone.label}
- {formatPrBadgeLabel(pr)}
-
-
- {live.label}
-
-
-
-
-
- {delta && deltaVisible ? (
-
-
- {delta.label}
- just now
-
-
- ) : null}
-
+
+
+ {tone.label}
+ {formatPrBadgeLabel(pr)}
+
+
+ {live.label}
+
+
{pr.title}
@@ -362,22 +231,20 @@ function PrDetails({
@@ -419,7 +286,6 @@ export const ChatPrPane = React.memo(function ChatPrPane({
branchName,
sessionTitle = null,
sessionId = null,
- delta = null,
onClose,
runtimePin = null,
}: {
@@ -433,8 +299,6 @@ export const ChatPrPane = React.memo(function ChatPrPane({
sessionTitle?: string | null;
/** The chat whose explicit PR links should be shown first. */
sessionId?: string | null;
- /** Describes the PR change that triggered this pane's auto-pop (owned by the parent). */
- 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. */
@@ -470,7 +334,6 @@ export const ChatPrPane = React.memo(function ChatPrPane({
const [reviews, setReviews] = useState
(null);
const [status, setStatus] = useState(null);
const [relay, setRelay] = useState(null);
- const [deltaVisible, setDeltaVisible] = useState(false);
// Manual title-bar ↻ sync in flight.
const [syncing, setSyncing] = useState(false);
// Backend reconcile-on-focus running (project-scoped); drives the subtle
@@ -668,14 +531,6 @@ export const ChatPrPane = React.memo(function ChatPrPane({
return () => { cancelled = true; };
}, [prRepoOwner, prRepoName]);
- // Show the delta line for a few seconds after each new delta, then fade it.
- useEffect(() => {
- if (!delta) { setDeltaVisible(false); return; }
- setDeltaVisible(true);
- const id = window.setTimeout(() => setDeltaVisible(false), DELTA_VISIBLE_MS);
- return () => window.clearTimeout(id);
- }, [delta?.nonce, delta]);
-
// 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.
@@ -749,8 +604,6 @@ export const ChatPrPane = React.memo(function ChatPrPane({
reviews={reviews}
status={status}
relay={relay}
- delta={delta}
- deltaVisible={deltaVisible}
copied={copied}
onOpenAde={openInAde}
onOpenGitHub={() => void openInGitHub()}
diff --git a/apps/desktop/src/renderer/components/chat/chatCompanionUiState.ts b/apps/desktop/src/renderer/components/chat/chatCompanionUiState.ts
index 07361b626d..fe04ef5417 100644
--- a/apps/desktop/src/renderer/components/chat/chatCompanionUiState.ts
+++ b/apps/desktop/src/renderer/components/chat/chatCompanionUiState.ts
@@ -20,7 +20,7 @@ export type ChatCompanionUiState = {
iosSimulatorOpen: boolean;
appControlOpen: boolean;
terminalDrawerOpen: boolean;
- /** Floating PR pane (left side). Persisted per chat, incl. webhook auto-pop. */
+ /** Floating PR pane (left side). Persisted per chat; explicit open/close only. */
prPaneOpen: boolean;
};
@@ -122,7 +122,7 @@ export function writeChatCompanionUiState(key: string, state: ChatCompanionUiSta
* Merge `patch` into the stored record for `key`.
*
* The namespace has two independent owners — the chat shell's drawer state and
- * `useChatPrAutoPop`'s `prPaneOpen` — so a whole-record write from either one
+ * `useChatPrPaneOpen`'s `prPaneOpen` — so a whole-record write from either one
* clobbers the other unless it reads forward first. Doing the read-merge-write
* here makes that structural instead of a convention each caller has to honour.
*/
diff --git a/apps/desktop/src/renderer/components/chat/useChatPrAutoPop.test.tsx b/apps/desktop/src/renderer/components/chat/useChatPrAutoPop.test.tsx
deleted file mode 100644
index 7ed0a36c9a..0000000000
--- a/apps/desktop/src/renderer/components/chat/useChatPrAutoPop.test.tsx
+++ /dev/null
@@ -1,241 +0,0 @@
-/* @vitest-environment jsdom */
-
-import { act, renderHook, waitFor } from "@testing-library/react";
-import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
-
-import { useChatPrAutoPop } from "./useChatPrAutoPop";
-import {
- chatCompanionUiStorageKey,
- readChatCompanionUiState,
- resetChatCompanionUiStateCacheForTests,
- writeChatCompanionUiState,
- DEFAULT_CHAT_COMPANION_UI_STATE,
-} from "./chatCompanionUiState";
-import type { PrEventPayload, PrSummary } from "../../../shared/types";
-
-function makePr(over: Partial = {}): PrSummary {
- return {
- id: "pr1",
- laneId: "lane1",
- projectId: "proj",
- repoOwner: "o",
- repoName: "r",
- githubPrNumber: 42,
- githubUrl: "https://github.com/o/r/pull/42",
- githubNodeId: null,
- title: "PR",
- state: "open",
- baseBranch: "main",
- headBranch: "feat",
- checksStatus: "none",
- reviewStatus: "none",
- additions: 1,
- deletions: 0,
- mergeConflicts: null,
- behindBaseBy: null,
- headSha: "aaa111",
- lastSyncedAt: new Date().toISOString(),
- createdAt: "2026-06-30T00:00:00Z",
- updatedAt: "rev-1",
- creationStrategy: null,
- ...over,
- };
-}
-
-const originalAde = (globalThis.window as { ade?: unknown }).ade;
-let emit: ((event: PrEventPayload) => void) | null = null;
-
-function installAde(initial: PrSummary | null) {
- emit = null;
- (globalThis.window as { ade?: unknown }).ade = {
- prs: {
- getForLane: vi.fn().mockResolvedValue(initial),
- onEvent: vi.fn().mockImplementation((cb: (event: PrEventPayload) => void) => {
- emit = cb;
- return () => { emit = null; };
- }),
- },
- };
-}
-
-function emitUpdated(prs: PrSummary[]) {
- act(() => emit?.({ type: "prs-updated", polledAt: "now", prs }));
-}
-
-beforeEach(() => {
- window.localStorage.clear();
- resetChatCompanionUiStateCacheForTests();
-});
-
-afterEach(() => {
- (globalThis.window as { ade?: unknown }).ade = originalAde;
- window.localStorage.clear();
- resetChatCompanionUiStateCacheForTests();
- vi.clearAllMocks();
-});
-
-describe("useChatPrAutoPop", () => {
- it("starts closed and does not pop for a PR that already existed on open", async () => {
- installAde(makePr({ state: "open" }));
- const { result } = renderHook(() => useChatPrAutoPop("lane1"));
- // Let the silent baseline seed resolve.
- await waitFor(() => expect(window.ade.prs.getForLane).toHaveBeenCalled());
- await act(async () => {});
- // Re-emitting the same open PR (e.g. a checks refresh) must not pop.
- emitUpdated([makePr({ state: "open", checksStatus: "passing" })]);
- expect(result.current.prPaneOpen).toBe(false);
- expect(result.current.prPaneDelta).toBeNull();
- });
-
- it("pops with a lifecycle delta when the PR merges", async () => {
- installAde(makePr({ state: "open" }));
- const { result } = renderHook(() => useChatPrAutoPop("lane1"));
- await act(async () => {});
- emitUpdated([makePr({ state: "merged" })]);
- expect(result.current.prPaneOpen).toBe(true);
- expect(result.current.prPaneDelta?.kind).toBe("merged");
- });
-
- it("pops for a new commit push (head sha change)", async () => {
- installAde(makePr({ state: "open", headSha: "aaa111" }));
- const { result } = renderHook(() => useChatPrAutoPop("lane1"));
- await act(async () => {});
- emitUpdated([makePr({ state: "open", headSha: "bbb222" })]);
- expect(result.current.prPaneOpen).toBe(true);
- expect(result.current.prPaneDelta?.kind).toBe("commit");
- });
-
- it("re-pops on each qualifying event with a fresh nonce", async () => {
- installAde(makePr({ state: "open", headSha: "aaa111" }));
- const { result } = renderHook(() => useChatPrAutoPop("lane1"));
- await act(async () => {});
- emitUpdated([makePr({ state: "open", headSha: "bbb222" })]);
- const first = result.current.prPaneDelta?.nonce;
- emitUpdated([makePr({ state: "merged", headSha: "bbb222" })]);
- expect(result.current.prPaneDelta?.kind).toBe("merged");
- expect(result.current.prPaneDelta?.nonce).not.toBe(first);
- });
-
- it("degrades to no-op when the prs bridge is unavailable", () => {
- (globalThis.window as { ade?: unknown }).ade = {};
- const { result } = renderHook(() => useChatPrAutoPop("lane1"));
- expect(result.current.prPaneOpen).toBe(false);
- expect(result.current.prPaneDelta).toBeNull();
- });
-});
-
-describe("useChatPrAutoPop persistence", () => {
- it("seeds the pane open from stored companion state", () => {
- installAde(makePr({ state: "open" }));
- writeChatCompanionUiState("chat-1", { ...DEFAULT_CHAT_COMPANION_UI_STATE, prPaneOpen: true });
- resetChatCompanionUiStateCacheForTests();
-
- const { result } = renderHook(() => useChatPrAutoPop("lane1", { persistKey: "chat-1" }));
- // Seeded on the FIRST render — not after an effect — so the pane never
- // flashes closed on chat open.
- expect(result.current.prPaneOpen).toBe(true);
- });
-
- it("persists a manual toggle to localStorage", async () => {
- installAde(null);
- const { result } = renderHook(() => useChatPrAutoPop("lane1", { persistKey: "chat-1" }));
- await act(async () => {});
-
- act(() => result.current.setPrPaneOpen(true));
- expect(readChatCompanionUiState("chat-1").prPaneOpen).toBe(true);
- expect(JSON.parse(window.localStorage.getItem(chatCompanionUiStorageKey("chat-1"))!).prPaneOpen).toBe(true);
-
- act(() => result.current.setPrPaneOpen(false));
- expect(readChatCompanionUiState("chat-1").prPaneOpen).toBe(false);
- });
-
- it("persists the webhook auto-pop too", async () => {
- installAde(makePr({ state: "open" }));
- const { result } = renderHook(() => useChatPrAutoPop("lane1", { persistKey: "chat-1" }));
- await act(async () => {});
-
- emitUpdated([makePr({ state: "merged" })]);
-
- expect(result.current.prPaneOpen).toBe(true);
- expect(readChatCompanionUiState("chat-1").prPaneOpen).toBe(true);
- });
-
- it("an already-open PR still does not pop on chat open with a persist key", async () => {
- installAde(makePr({ state: "open" }));
- const { result } = renderHook(() => useChatPrAutoPop("lane1", { persistKey: "chat-1" }));
- await waitFor(() => expect(window.ade.prs.getForLane).toHaveBeenCalled());
- await act(async () => {});
-
- emitUpdated([makePr({ state: "open", checksStatus: "passing" })]);
-
- expect(result.current.prPaneOpen).toBe(false);
- expect(result.current.prPaneDelta).toBeNull();
- // The silent baseline seed must not have written an "open" pane either.
- expect(readChatCompanionUiState("chat-1").prPaneOpen).toBe(false);
- });
-
- it("re-seeds per chat when the persist key changes", async () => {
- installAde(null);
- writeChatCompanionUiState("chat-2", { ...DEFAULT_CHAT_COMPANION_UI_STATE, prPaneOpen: true });
- const { result, rerender } = renderHook(
- ({ key }: { key: string }) => useChatPrAutoPop("lane1", { persistKey: key }),
- { initialProps: { key: "chat-1" } },
- );
- await act(async () => {});
- expect(result.current.prPaneOpen).toBe(false);
-
- rerender({ key: "chat-2" });
- await act(async () => {});
- expect(result.current.prPaneOpen).toBe(true);
- // Switching chats must not write chat-1's value into chat-2 (or vice versa).
- expect(readChatCompanionUiState("chat-1").prPaneOpen).toBe(false);
- expect(readChatCompanionUiState("chat-2").prPaneOpen).toBe(true);
- });
-
- it("never writes the outgoing chat's value into the incoming chat's record", async () => {
- // Regression: the hydrate and persist effects share a fiber and flush in
- // declaration order, so on the commit where the key changes the persist
- // effect still closes over the OUTGOING chat's `prPaneOpen`. Asserting the
- // settled value cannot catch that — a corrective render repairs it — so
- // watch the writes themselves.
- installAde(null);
- writeChatCompanionUiState("chat-2", { ...DEFAULT_CHAT_COMPANION_UI_STATE, prPaneOpen: true });
- resetChatCompanionUiStateCacheForTests();
-
- const chat2Key = chatCompanionUiStorageKey("chat-2");
- const writes: string[] = [];
- const originalSetItem = Storage.prototype.setItem;
- const setItem = vi
- .spyOn(Storage.prototype, "setItem")
- .mockImplementation(function (this: Storage, key: string, value: string) {
- if (key === chat2Key) writes.push(value);
- originalSetItem.call(this, key, value);
- });
-
- try {
- const { rerender } = renderHook(
- ({ key }: { key: string }) => useChatPrAutoPop("lane1", { persistKey: key }),
- { initialProps: { key: "chat-1" } },
- );
- await act(async () => {});
- rerender({ key: "chat-2" });
- await act(async () => {});
-
- // chat-1 was closed; chat-2 is open. No write to chat-2 may carry `false`.
- const clobbered = writes.filter((value) => value.includes("\"prPaneOpen\":false"));
- expect(clobbered).toEqual([]);
- expect(readChatCompanionUiState("chat-2").prPaneOpen).toBe(true);
- } finally {
- setItem.mockRestore();
- }
- });
-
- it("keeps working with no persist key", async () => {
- installAde(makePr({ state: "open" }));
- const { result } = renderHook(() => useChatPrAutoPop("lane1"));
- await act(async () => {});
- emitUpdated([makePr({ state: "merged" })]);
- expect(result.current.prPaneOpen).toBe(true);
- expect(window.localStorage.length).toBe(0);
- });
-});
diff --git a/apps/desktop/src/renderer/components/chat/useChatPrAutoPop.ts b/apps/desktop/src/renderer/components/chat/useChatPrAutoPop.ts
deleted file mode 100644
index 7005f1e31f..0000000000
--- a/apps/desktop/src/renderer/components/chat/useChatPrAutoPop.ts
+++ /dev/null
@@ -1,122 +0,0 @@
-import { useEffect, useRef, useState, type Dispatch, type SetStateAction } from "react";
-
-import {
- chatPrSignature,
- detectChatPrDelta,
- type ChatPrDelta,
- type ChatPrSignature,
-} from "./ChatPrPane";
-import { patchChatCompanionUiState, readChatCompanionUiState } from "./chatCompanionUiState";
-import type { OpenProjectBinding } from "../../../shared/types";
-
-export type UseChatPrAutoPop = {
- prPaneOpen: boolean;
- setPrPaneOpen: Dispatch>;
- /** The PR change that triggered the most recent auto-pop; handed to ChatPrPane. */
- prPaneDelta: ChatPrDelta | null;
-};
-
-/**
- * Drives the floating PR pane's auto-pop for a lane's linked PR, shared by the
- * ADE chat surface (AgentChatPane) and the CLI session surface (WorkViewArea).
- *
- * On each webhook-driven `prs-updated` it re-reads this lane's PR summary and
- * pops the pane for a pop-worthy change — a newly created/linked PR, a lifecycle
- * change (merged/closed/reopened/ready/draft), or a new commit push. Checks and
- * review changes update the pane but do not pop it (see detectChatPrDelta). The
- * baseline is seeded silently so an already-open PR never pops on chat open.
- * Re-pops on each qualifying event; rapid bursts collapse into one visible pop
- * because the pane only ever shows the latest delta.
- *
- * `persistKey` (the surface's per-chat companion-state key) makes the pane's
- * open/closed state per chat AND durable across restarts: without it the pane
- * is bare component state, so every chat switch and every app launch reopens
- * from "closed" regardless of what the user left open.
- */
-export function useChatPrAutoPop(
- laneId: string | null | undefined,
- opts?: { persistKey?: string | null; runtimePin?: OpenProjectBinding | null },
-): UseChatPrAutoPop {
- const persistKey = opts?.persistKey ?? null;
- // The lane's PR lives on the lane's machine, so both the seed read and the
- // event feed have to come from there — see `ChatGitToolbar.runtimePin`. Keyed
- // on the pin KEY, read through a ref: a local pin is a fresh object on every
- // cross-machine merge, and re-subscribing re-anchors the pinned event pump.
- const runtimePin = opts?.runtimePin ?? null;
- const runtimePinRef = useRef(runtimePin);
- runtimePinRef.current = runtimePin;
- const runtimePinKey = runtimePin?.key ?? null;
- const [prPaneOpen, setPrPaneOpen] = useState(
- () => (persistKey ? readChatCompanionUiState(persistKey).prPaneOpen : false),
- );
- const [prPaneDelta, setPrPaneDelta] = useState(null);
- const prevPrSigRef = useRef(null); // null = not yet seeded
- const nonceRef = useRef(0);
- // Which key the current `prPaneOpen` was hydrated from.
- const hydratedPersistKeyRef = useRef(persistKey);
- // Set by the hydrate effect and consumed by the persist effect below. Both
- // effects belong to the same fiber and run in declaration order within one
- // passive-effect flush, so on the commit where `persistKey` changes the
- // persist effect still closes over the OUTGOING chat's `prPaneOpen`. Marking
- // the hydration here — rather than relying on the key ref, which the hydrate
- // effect has already advanced by then — makes the persist effect skip exactly
- // that one stale flush instead of writing chat A's value into chat B's record.
- const pendingHydrationKeyRef = useRef(null);
-
- useEffect(() => {
- if (hydratedPersistKeyRef.current === persistKey) return;
- hydratedPersistKeyRef.current = persistKey;
- pendingHydrationKeyRef.current = persistKey;
- setPrPaneOpen(persistKey ? readChatCompanionUiState(persistKey).prPaneOpen : false);
- }, [persistKey]);
-
- // Every transition persists — the toolbar toggle, the ✕, and the webhook
- // auto-pop below all land here because they all move the same state. The
- // patch merges inside the store, so the chat shell's drawer fields on the
- // same record survive.
- useEffect(() => {
- if (!persistKey || hydratedPersistKeyRef.current !== persistKey) return;
- if (pendingHydrationKeyRef.current === persistKey) {
- // Stale flush from the key change. If hydration changed the value, the
- // corrective render re-runs this effect with the real one; if it didn't,
- // storage already agrees and there is nothing to write either way.
- pendingHydrationKeyRef.current = null;
- return;
- }
- if (readChatCompanionUiState(persistKey).prPaneOpen === prPaneOpen) return;
- patchChatCompanionUiState(persistKey, { prPaneOpen });
- }, [persistKey, prPaneOpen]);
-
- useEffect(() => {
- setPrPaneDelta(null);
- prevPrSigRef.current = null;
- // The prs bridge can be absent in some surfaces / test harnesses / early
- // boot; degrade to "no auto-pop" instead of crashing the host surface.
- const prs = window.ade?.prs;
- if (!laneId || !prs?.getForLane || !prs?.onEvent) return;
- let cancelled = false;
- prs
- .getForLane(laneId, runtimePinRef.current)
- .then((pr) => {
- if (!cancelled && prevPrSigRef.current === null) prevPrSigRef.current = chatPrSignature(pr);
- })
- .catch(() => {});
- const unsubscribe = prs.onEvent((event) => {
- if (event.type !== "prs-updated") return;
- const next = event.prs.find((pr) => pr.laneId === laneId) ?? null;
- const prev = prevPrSigRef.current;
- prevPrSigRef.current = chatPrSignature(next);
- if (prev === null) return; // just seeded from this event — no pop
- const change = detectChatPrDelta(prev, next);
- if (!change) return;
- setPrPaneDelta({ ...change, nonce: ++nonceRef.current });
- setPrPaneOpen(true);
- }, runtimePinRef.current);
- return () => {
- cancelled = true;
- unsubscribe();
- };
- }, [laneId, runtimePinKey]);
-
- return { prPaneOpen, setPrPaneOpen, prPaneDelta };
-}
diff --git a/apps/desktop/src/renderer/components/chat/useChatPrPaneOpen.test.tsx b/apps/desktop/src/renderer/components/chat/useChatPrPaneOpen.test.tsx
new file mode 100644
index 0000000000..d29bc6aeb1
--- /dev/null
+++ b/apps/desktop/src/renderer/components/chat/useChatPrPaneOpen.test.tsx
@@ -0,0 +1,114 @@
+/* @vitest-environment jsdom */
+
+import { act, renderHook } from "@testing-library/react";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import { useChatPrPaneOpen } from "./useChatPrPaneOpen";
+import {
+ chatCompanionUiStorageKey,
+ readChatCompanionUiState,
+ resetChatCompanionUiStateCacheForTests,
+ writeChatCompanionUiState,
+ DEFAULT_CHAT_COMPANION_UI_STATE,
+} from "./chatCompanionUiState";
+
+beforeEach(() => {
+ window.localStorage.clear();
+ resetChatCompanionUiStateCacheForTests();
+});
+
+afterEach(() => {
+ window.localStorage.clear();
+ resetChatCompanionUiStateCacheForTests();
+});
+
+describe("useChatPrPaneOpen", () => {
+ it("starts closed", () => {
+ const { result } = renderHook(() => useChatPrPaneOpen("chat-1"));
+ expect(result.current.prPaneOpen).toBe(false);
+ });
+
+ it("seeds the pane open from stored companion state", () => {
+ writeChatCompanionUiState("chat-1", { ...DEFAULT_CHAT_COMPANION_UI_STATE, prPaneOpen: true });
+ resetChatCompanionUiStateCacheForTests();
+
+ const { result } = renderHook(() => useChatPrPaneOpen("chat-1"));
+ // Seeded on the FIRST render — not after an effect — so the pane never
+ // flashes closed on chat open.
+ expect(result.current.prPaneOpen).toBe(true);
+ });
+
+ it("persists a manual toggle to localStorage", () => {
+ const { result } = renderHook(() => useChatPrPaneOpen("chat-1"));
+
+ act(() => result.current.setPrPaneOpen(true));
+ expect(readChatCompanionUiState("chat-1").prPaneOpen).toBe(true);
+ expect(JSON.parse(window.localStorage.getItem(chatCompanionUiStorageKey("chat-1"))!).prPaneOpen).toBe(true);
+
+ act(() => result.current.setPrPaneOpen(false));
+ expect(readChatCompanionUiState("chat-1").prPaneOpen).toBe(false);
+ });
+
+ it("re-seeds per chat when the persist key changes", async () => {
+ writeChatCompanionUiState("chat-2", { ...DEFAULT_CHAT_COMPANION_UI_STATE, prPaneOpen: true });
+ const { result, rerender } = renderHook(
+ ({ key }: { key: string }) => useChatPrPaneOpen(key),
+ { initialProps: { key: "chat-1" } },
+ );
+ await act(async () => {});
+ expect(result.current.prPaneOpen).toBe(false);
+
+ rerender({ key: "chat-2" });
+ await act(async () => {});
+ expect(result.current.prPaneOpen).toBe(true);
+ // Switching chats must not write chat-1's value into chat-2 (or vice versa).
+ expect(readChatCompanionUiState("chat-1").prPaneOpen).toBe(false);
+ expect(readChatCompanionUiState("chat-2").prPaneOpen).toBe(true);
+ });
+
+ it("never writes the outgoing chat's value into the incoming chat's record", async () => {
+ // Regression: the hydrate and persist effects share a fiber and flush in
+ // declaration order, so on the commit where the key changes the persist
+ // effect still closes over the OUTGOING chat's `prPaneOpen`. Asserting the
+ // settled value cannot catch that — a corrective render repairs it — so
+ // watch the writes themselves.
+ writeChatCompanionUiState("chat-2", { ...DEFAULT_CHAT_COMPANION_UI_STATE, prPaneOpen: true });
+ resetChatCompanionUiStateCacheForTests();
+
+ const chat2Key = chatCompanionUiStorageKey("chat-2");
+ const writes: string[] = [];
+ const originalSetItem = Storage.prototype.setItem;
+ const setItem = vi
+ .spyOn(Storage.prototype, "setItem")
+ .mockImplementation(function (this: Storage, key: string, value: string) {
+ if (key === chat2Key) writes.push(value);
+ originalSetItem.call(this, key, value);
+ });
+
+ try {
+ const { rerender } = renderHook(
+ ({ key }: { key: string }) => useChatPrPaneOpen(key),
+ { initialProps: { key: "chat-1" } },
+ );
+ await act(async () => {});
+ rerender({ key: "chat-2" });
+ await act(async () => {});
+
+ // chat-1 was closed; chat-2 is open. No write to chat-2 may carry `false`.
+ const clobbered = writes.filter((value) => value.includes("\"prPaneOpen\":false"));
+ expect(clobbered).toEqual([]);
+ expect(readChatCompanionUiState("chat-2").prPaneOpen).toBe(true);
+ } finally {
+ setItem.mockRestore();
+ }
+ });
+
+ it("keeps working with no persist key and writes nothing to storage", async () => {
+ const { result } = renderHook(() => useChatPrPaneOpen(null));
+ await act(async () => {});
+
+ act(() => result.current.setPrPaneOpen(true));
+ expect(result.current.prPaneOpen).toBe(true);
+ expect(window.localStorage.length).toBe(0);
+ });
+});
diff --git a/apps/desktop/src/renderer/components/chat/useChatPrPaneOpen.ts b/apps/desktop/src/renderer/components/chat/useChatPrPaneOpen.ts
new file mode 100644
index 0000000000..c24f9c3f06
--- /dev/null
+++ b/apps/desktop/src/renderer/components/chat/useChatPrPaneOpen.ts
@@ -0,0 +1,61 @@
+import { useEffect, useRef, useState, type Dispatch, type SetStateAction } from "react";
+
+import { patchChatCompanionUiState, readChatCompanionUiState } from "./chatCompanionUiState";
+
+export type UseChatPrPaneOpen = {
+ prPaneOpen: boolean;
+ setPrPaneOpen: Dispatch>;
+};
+
+/**
+ * Owns the floating PR pane's open/closed state, shared by the ADE chat
+ * surface (AgentChatPane) and the CLI session surface (WorkViewArea).
+ *
+ * The pane never auto-opens — only an explicit user toggle moves it. Once
+ * opened it stays open until the user closes it.
+ *
+ * `persistKey` (the surface's per-chat companion-state key) makes that
+ * open/closed state per chat AND durable across restarts: without it the pane
+ * is bare component state, so every chat switch and every app launch reopens
+ * from "closed" regardless of what the user left open.
+ */
+export function useChatPrPaneOpen(persistKey: string | null): UseChatPrPaneOpen {
+ const [prPaneOpen, setPrPaneOpen] = useState(
+ () => (persistKey ? readChatCompanionUiState(persistKey).prPaneOpen : false),
+ );
+ // Which key the current `prPaneOpen` was hydrated from.
+ const hydratedPersistKeyRef = useRef(persistKey);
+ // Set by the hydrate effect and consumed by the persist effect below. Both
+ // effects belong to the same fiber and run in declaration order within one
+ // passive-effect flush, so on the commit where `persistKey` changes the
+ // persist effect still closes over the OUTGOING chat's `prPaneOpen`. Marking
+ // the hydration here — rather than relying on the key ref, which the hydrate
+ // effect has already advanced by then — makes the persist effect skip exactly
+ // that one stale flush instead of writing chat A's value into chat B's record.
+ const pendingHydrationKeyRef = useRef(null);
+
+ useEffect(() => {
+ if (hydratedPersistKeyRef.current === persistKey) return;
+ hydratedPersistKeyRef.current = persistKey;
+ pendingHydrationKeyRef.current = persistKey;
+ setPrPaneOpen(persistKey ? readChatCompanionUiState(persistKey).prPaneOpen : false);
+ }, [persistKey]);
+
+ // Every transition persists — the toolbar toggle and the ✕ both land here.
+ // The patch merges inside the store, so the chat shell's drawer fields on
+ // the same record survive.
+ useEffect(() => {
+ if (!persistKey || hydratedPersistKeyRef.current !== persistKey) return;
+ if (pendingHydrationKeyRef.current === persistKey) {
+ // Stale flush from the key change. If hydration changed the value, the
+ // corrective render re-runs this effect with the real one; if it didn't,
+ // storage already agrees and there is nothing to write either way.
+ pendingHydrationKeyRef.current = null;
+ return;
+ }
+ if (readChatCompanionUiState(persistKey).prPaneOpen === prPaneOpen) return;
+ patchChatCompanionUiState(persistKey, { prPaneOpen });
+ }, [persistKey, prPaneOpen]);
+
+ return { prPaneOpen, setPrPaneOpen };
+}
diff --git a/apps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsx b/apps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsx
index 7c1a899ab8..448b76ce48 100644
--- a/apps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsx
+++ b/apps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsx
@@ -177,8 +177,12 @@ vi.mock("../chat/ChatPrPane", async (importOriginal) => {
const actual = await importOriginal();
return {
...actual,
- ChatPrPane: ({ laneId }: { laneId: string }) => (
-
+ ChatPrPane: ({ laneId, runtimePin }: { laneId: string; runtimePin?: { key: string } | null }) => (
+
),
};
});
@@ -562,7 +566,7 @@ describe("WorkViewArea", () => {
expect(terminals.map((terminal) => terminal.getAttribute("data-session-id"))).toContain("session-1");
});
- it("pins the PR pane and auto-pop reads to the owning machine for a foreign running CLI", async () => {
+ it("pins the PR pane 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",
@@ -593,18 +597,16 @@ describe("WorkViewArea", () => {
const local = within(view.container);
// 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(() => {
- expect(prsMocks.getForLane).toHaveBeenCalledWith("lane-1", runtimePin);
- });
- expect(prsMocks.onEvent).toHaveBeenCalledWith(expect.any(Function), runtimePin);
+ // same PR affordance as a local one — the pane is handed the resolved pin
+ // so its reads carry it. Before this, the pane was suppressed outright,
+ // which made a remote session's PR invisible until the tab was rebound.
fireEvent.click(local.getByRole("button", { name: "Toggle PR pane" }));
- expect((await local.findByTestId("chat-pr-pane")).getAttribute("data-lane-id")).toBe("lane-1");
+ const pane = await local.findByTestId("chat-pr-pane");
+ expect(pane.getAttribute("data-lane-id")).toBe("lane-1");
+ expect(pane.getAttribute("data-runtime-pin-key")).toBe("remote:target-b:project-b");
});
- it("keeps PR auto-pop and pane controls enabled for a local running CLI", async () => {
+ it("keeps PR pane controls enabled for a local running CLI", async () => {
const session = { ...makeRunningSession("session-local", "pty-local"), toolType: "codex" as const };
const view = render(
{
);
const local = within(view.container);
- await waitFor(() => {
- expect(prsMocks.getForLane).toHaveBeenCalledWith("lane-1", null);
- expect(prsMocks.onEvent).toHaveBeenCalledTimes(1);
- });
fireEvent.click(local.getByRole("button", { name: "Toggle PR pane" }));
expect((await local.findByTestId("chat-pr-pane")).getAttribute("data-lane-id")).toBe("lane-1");
});
diff --git a/apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx b/apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx
index 7ffc796314..3758bbee10 100644
--- a/apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx
+++ b/apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx
@@ -42,7 +42,7 @@ import { resolveModelDescriptorWithRuntimeCatalog, createUnknownModelPlaceholder
import { WorkStartSurface } from "./WorkStartSurface";
import { CliSessionWorkSurfaceHeader } from "./CliSessionWorkSurfaceHeader";
import { ChatPrPane } from "../chat/ChatPrPane";
-import { useChatPrAutoPop } from "../chat/useChatPrAutoPop";
+import { useChatPrPaneOpen } from "../chat/useChatPrPaneOpen";
import { isChatToolType, primarySessionLabel, stripTerminalLabelControls, formatToolTypeLabel } from "../../lib/sessions";
import { SmartTooltip } from "../ui/SmartTooltip";
import { cn } from "../ui/cn";
@@ -758,8 +758,9 @@ const CLI_FLOATING_PANE_CARD_CLASS =
* overlaid on top of the terminal. The overlay is absolutely positioned inside a
* wrapper that sizes the terminal — it never changes the terminal host's box, so
* the PTY's ResizeObserver never fires and the running CLI process is not
- * re-flowed (no SIGWINCH). The pill in the header toggles it, and it auto-pops on
- * webhook-driven PR changes via the same useChatPrAutoPop hook the ADE chat uses.
+ * re-flowed (no SIGWINCH). The pill in the header toggles it; it never
+ * auto-opens and shares the persisted open state with the ADE chat pane via the
+ * same useChatPrPaneOpen hook.
*/
function CliSessionSurface({
session,
@@ -800,11 +801,8 @@ 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 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,
- });
+ // same pill and pane as a local one — the pin just routes them.
+ const { prPaneOpen, setPrPaneOpen } = useChatPrPaneOpen(session.id);
const supportsSplit = layoutVariant !== "grid-tile";
const prFloating = prPaneOpen && Boolean(session.laneId) && supportsSplit;
return (
@@ -853,7 +851,6 @@ function CliSessionSurface({
setPrPaneOpen(false)}
runtimePin={runtimePin}
/>
diff --git a/docs/features/chat/composer-and-ui.md b/docs/features/chat/composer-and-ui.md
index 606128b897..1bc547bcba 100644
--- a/docs/features/chat/composer-and-ui.md
+++ b/docs/features/chat/composer-and-ui.md
@@ -52,8 +52,8 @@ subagents, computer use). The pane derives all visible state from the
| `ChatUserMinimap.tsx`, `chatUserMinimap.logic.ts` | Tick rail down the transcript's **left** gutter, one clean tick per user message with no guide hairline, gated on the `chatUserMinimapEnabled` appearance setting and mouse pointers only (`[@media(pointer:fine)]`). Ticks are positioned by percentage of the full message-list 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 `