PR Pane Auto Open -> main

apps/desktop/src/renderer/components/chat/AgentChatPane.test.tsx

@@ -1886,6 +1886,25 @@ describe("AgentChatPane pane reserve", () => {
     expect(await screen.findByAltText("ADE")).toBeTruthy();
     expect(readLeftReserve(container)).toBe("0px");
   });
+
+  it("never persists a phantom-open PR pane from the draft surface's PR pill", async () => {
+    installAdeMocks({ sessions: [] });
+    seedDrawerStore();
+    // Regression: the draft surface renders no PR pane (no selected session),
+    // so its header pill must fall back to the toolbar's inline menu instead
+    // of toggling persisted open state for a pane that cannot appear here.
+    render(
+      <MemoryRouter>
+        <AgentChatPane laneId="lane-1" hideSessionTabs onSessionCreated={vi.fn()} />
+      </MemoryRouter>,
+    );
+
+    expect(await screen.findByAltText("ADE")).toBeTruthy();
+    fireEvent.click(await screen.findByRole("button", { name: "PR" }));
+    await act(async () => {});
+
+    expect(readChatCompanionUiState("draft:lane-1").prPaneOpen).toBe(false);
+  });
 });
 
 describe("AgentChatPane companion drawers", () => {

apps/desktop/src/renderer/components/chat/AgentChatPane.tsx

@@ -205,7 +205,7 @@ import { ChatActionsDrawerPanel, type ChatActionsTab } from "./ChatActionsDrawer
 import { ChatSourcesPanel } from "./ChatSourcesPanel";
 import { CrossMachineHandoffModal } from "./CrossMachineHandoffModal";
 import { ChatPrPane } from "./ChatPrPane";
-import { useChatPrAutoPop } from "./useChatPrAutoPop";
+import { useChatPrPaneOpen } from "./useChatPrPaneOpen";
 import {
   patchChatCompanionUiState,
   readChatCompanionUiState,
@@ -3994,16 +3994,11 @@ export function AgentChatPane({
   // a chat on another machine reports paths on that machine's disk, so opening
   // one has to ask that machine — not whichever machine this tab is bound to.
   const chatWorkspacePaths = useWorkspacePathOpener({ laneId, navigate, runtimePin: 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,
-  });
+  // Left PR floating pane (ADE chats only). It never auto-opens — only an
+  // explicit toggle moves it; shared with the CLI session surface via
+  // useChatPrPaneOpen. `persistKey` makes open/closed per chat and durable
+  // across restarts: open it once for a chat and it stays open until closed.
+  const { prPaneOpen, setPrPaneOpen } = useChatPrPaneOpen(companionStateKey);
   const renderedSession = useMemo(
     () => (
       renderedSessionId
@@ -4108,7 +4103,7 @@ export function AgentChatPane({
       companionHydrationKeyRef.current = null;
       return;
     }
-    // `prPaneOpen` is owned by useChatPrAutoPop's own persist effect; the patch
+    // `prPaneOpen` is owned by useChatPrPaneOpen's own persist effect; the patch
     // helper does the read-merge-write, so a drawer toggle can't clobber it.
     patchChatCompanionUiState(companionStateKey, {
       chatActionsOpen,
@@ -12598,7 +12593,13 @@ export function AgentChatPane({
         lifecycleSessionId={selectedSessionId ?? null}
         showGitToolbar={showWorkspaceChrome}
         prSessionId={renderedSessionId}
-        onTogglePrPane={showWorkspaceChrome && laneId ? () => setPrPaneOpen((v) => !v) : undefined}
+        // Only wire the pane toggle where the pane actually renders (a selected
+        // session). On the draft/new-chat surface an unwired pill falls back to
+        // the toolbar's inline PR menu instead of toggling persisted state for
+        // a pane that cannot appear there.
+        onTogglePrPane={showWorkspaceChrome && laneId && Boolean(selectedSessionId)
+          ? () => setPrPaneOpen((v) => !v)
+          : undefined}
         prPaneOpen={prPaneOpen}
         runtimePin={chatRuntimePin}
         trailingActions={chatHeaderTrailingActions}
@@ -13467,10 +13468,11 @@ export function AgentChatPane({
   //
   // Gate the reserve on the surface that actually renders those panes. Both the
   // PR pane and the chat-actions pane live in the `selectedSessionId` branch
-  // below; the empty/draft surface renders neither. `prPaneOpen` is persisted
-  // per lane by `useChatPrAutoPop`, so without this gate a lane that once had
-  // the PR pane open pays a 276px left gutter on the new-chat screen — shoving
-  // the hero composer sideways to clear a pane that is not on screen.
+  // below; the empty/draft surface renders neither. `chatActionsOpen` (like
+  // `prPaneOpen`) is persisted per chat, so without this gate a lane that once
+  // had the chat-actions pane open pays a 276px right gutter on the new-chat
+  // screen — shoving the hero composer sideways to clear a pane that is not on
+  // screen.
   const sessionSurfaceMounted = Boolean(selectedSessionId);
   const paneReserve = sessionSurfaceMounted
     ? computePaneReserve(chatAreaWidth, chatActionsFloating)
@@ -13976,7 +13978,6 @@ export function AgentChatPane({
                             branchName={laneGitBranch}
                             sessionTitle={selectedSession?.title ?? null}
                             sessionId={renderedSessionId}
-                            delta={prPaneDelta}
                             onClose={() => setPrPaneOpen(false)}
                             runtimePin={chatRuntimePin}
                           />,

apps/desktop/src/renderer/components/chat/ChatPrPane.test.tsx

@@ -29,12 +29,7 @@ vi.mock("framer-motion", () => {
   };
 });
 
-import {
-  ChatPrPane,
-  chatPrSignature,
-  detectChatPrDelta,
-  type ChatPrSignature,
-} from "./ChatPrPane";
+import { ChatPrPane } from "./ChatPrPane";
 import type { PrCheck, PrEventPayload, PrReview, PrStatus, PrSummary } from "../../../shared/types";
 import { clearPrReadInFlightForTest } from "../../lib/prReadCache";
 import { useAppStore } from "../../state/appStore";
@@ -68,13 +63,6 @@ function makePr(over: Partial<PrSummary> = {}): PrSummary {
   };
 }
 
-const sig = (over: Partial<ChatPrSignature> = {}): ChatPrSignature => ({
-  exists: true,
-  state: "open",
-  headSha: "aaa111",
-  ...over,
-});
-
 const originalAde = (globalThis.window as { ade?: unknown }).ade;
 
 function installAde(over?: {
@@ -177,45 +165,6 @@ afterEach(() => {
   vi.clearAllMocks();
 });
 
-describe("detectChatPrDelta", () => {
-  it("pops for a newly created / first-linked PR", () => {
-    expect(detectChatPrDelta(sig({ exists: false, state: null, headSha: null }), makePr())?.kind).toBe("created");
-  });
-
-  it("pops for lifecycle transitions", () => {
-    expect(detectChatPrDelta(sig(), makePr({ state: "merged" }))?.kind).toBe("merged");
-    expect(detectChatPrDelta(sig(), makePr({ state: "closed" }))?.kind).toBe("closed");
-    expect(detectChatPrDelta(sig({ state: "draft" }), makePr({ state: "open" }))?.kind).toBe("ready");
-    expect(detectChatPrDelta(sig({ state: "closed" }), makePr({ state: "open" }))?.kind).toBe("reopened");
-    expect(detectChatPrDelta(sig(), makePr({ state: "draft" }))?.kind).toBe("draft");
-  });
-
-  it("pops for a new commit (head sha change)", () => {
-    expect(detectChatPrDelta(sig({ headSha: "aaa111" }), makePr({ headSha: "bbb222" }))?.kind).toBe("commit");
-  });
-
-  it("does NOT pop for checks/review-only changes", () => {
-    // Same state + same head sha, only checks/review changed.
-    const next = makePr({ checksStatus: "failing", reviewStatus: "changes_requested" });
-    expect(detectChatPrDelta(sig(), next)).toBeNull();
-  });
-
-  it("does NOT pop when the PR was unlinked / removed", () => {
-    expect(detectChatPrDelta(sig(), null)).toBeNull();
-  });
-});
-
-describe("chatPrSignature", () => {
-  it("captures existence, state, and head sha", () => {
-    expect(chatPrSignature(makePr({ state: "merged", headSha: "zzz" }))).toEqual({
-      exists: true,
-      state: "merged",
-      headSha: "zzz",
-    });
-    expect(chatPrSignature(null)).toEqual({ exists: false, state: null, headSha: null });
-  });
-});
-
 describe("ChatPrPane", () => {
   it("refreshes a linked PR on mount and hides stale running checks once merged", async () => {
     const stalePr = makePr({
@@ -390,14 +339,6 @@ describe("ChatPrPane", () => {
     expect(screen.getByText("−38")).toBeTruthy();
   });
 
-  it("shows the delta line describing what just changed", async () => {
-    installAde();
-    renderPane({ delta: { kind: "merged", label: "Merged", tone: "good", nonce: 1 } });
-    // Title still says the PR is open; "Merged" here is the delta line.
-    expect(await screen.findByText("Merged")).toBeTruthy();
-    expect(screen.getByText("just now")).toBeTruthy();
-  });
-
   it("renders enriched check counts from getChecks", async () => {
     installAde({
       getChecks: [