From c16f0d5b7023083ab3988910388af8c6fbf2bf31 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Wed, 2 Sep 2026 13:28:31 -0700 Subject: [PATCH] Put the cursor in the terminal after sending a note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sending already switched to the target terminal's tab, which looks like it should be enough: `TerminalView` focuses xterm whenever a terminal becomes active. But that effect keys off `active`, so it only fires on a *change* — and the dock's ordinary case is sending to the terminal already on screen. `setActiveTabKey` writes the key that is already set, nothing changes, no effect re-runs, and focus stays on the Send button. The note is sitting in the prompt and the user still has to click the terminal before pressing Enter. So the send now asks for focus explicitly, through a one-shot request in the store that `TerminalView` consumes and clears — the shape `pendingHomeTab` already uses. Clearing is not tidiness: hold the id and the second send to the same terminal writes a value that is already there, which is precisely the no-op this exists to fix. Focus is requested only on success. A failed send toasts and leaves the user where they are, because there is nothing in the prompt to press Enter on. The three `TerminalView` tests give focus away after mounting before making any assertion, so what they observe is the request landing and never the focus that `active` already grants on mount — which would pass with the feature absent. 752 tests pass, 62 files. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011YPqHpjV4EL6RNEwrRKqQm --- .../notes/SendToAgentButton.test.tsx | 43 +++++++++++++- .../components/notes/SendToAgentButton.tsx | 23 +++++--- .../components/terminal/TerminalView.test.tsx | 56 +++++++++++++++++++ app/src/components/terminal/TerminalView.tsx | 15 +++++ app/src/store/appState.test.ts | 24 ++++++++ app/src/store/appState.ts | 18 ++++++ 6 files changed, 169 insertions(+), 10 deletions(-) diff --git a/app/src/components/notes/SendToAgentButton.test.tsx b/app/src/components/notes/SendToAgentButton.test.tsx index 7197aab..9373727 100644 --- a/app/src/components/notes/SendToAgentButton.test.tsx +++ b/app/src/components/notes/SendToAgentButton.test.tsx @@ -11,14 +11,22 @@ vi.mock("../../hooks/useTerminal", () => ({ })); const setActiveTabKey = vi.fn(); +const requestTerminalFocus = vi.fn(); const pushToast = vi.fn(); let projects: Project[] = []; vi.mock("../../store/appState", () => ({ useAppState: Object.assign( (selector: (s: unknown) => unknown) => - selector({ projects, setActiveTabKey, pushToast }), - { getState: () => ({ projects, setActiveTabKey, pushToast }) }, + selector({ projects, setActiveTabKey, requestTerminalFocus, pushToast }), + { + getState: () => ({ + projects, + setActiveTabKey, + requestTerminalFocus, + pushToast, + }), + }, ), terminalTabKey: (id: string) => `term:${id}`, })); @@ -149,4 +157,35 @@ describe("SendToAgentButton", () => { // overflow, so a downward menu at the bottom edge is invisible. await waitFor(() => expect(screen.getByRole("menu")).toHaveClass("bottom-full")); }); + + // Switching to the tab is not enough. When the dock is open beside the + // terminal it sends to, that terminal is already the active tab, so + // `setActiveTabKey` changes nothing and no effect re-runs — leaving focus on + // this button, one click short of the Enter the user came to press. + it("hands focus to the terminal so the next keystroke is Enter", async () => { + sessions = [session()]; + render(); + fireEvent.click(screen.getByRole("button", { name: /send to agent/i })); + + await waitFor(() => expect(requestTerminalFocus).toHaveBeenCalledWith("s1")); + }); + + it("leaves focus alone when the send failed", async () => { + sessions = [session()]; + sendInput.mockRejectedValueOnce(new Error("pty gone")); + render(); + fireEvent.click(screen.getByRole("button", { name: /send to agent/i })); + + await waitFor(() => expect(pushToast).toHaveBeenCalled()); + expect(requestTerminalFocus).not.toHaveBeenCalled(); + }); + + it("focuses the session picked from the menu, not the first one", async () => { + sessions = [session(), session({ id: "s2", sessionName: "review" })]; + render(); + fireEvent.click(screen.getByRole("button", { name: /send to agent/i })); + fireEvent.click(await screen.findByRole("menuitem", { name: "review" })); + + await waitFor(() => expect(requestTerminalFocus).toHaveBeenCalledWith("s2")); + }); }); \ No newline at end of file diff --git a/app/src/components/notes/SendToAgentButton.tsx b/app/src/components/notes/SendToAgentButton.tsx index 38e808e..9e1de82 100644 --- a/app/src/components/notes/SendToAgentButton.tsx +++ b/app/src/components/notes/SendToAgentButton.tsx @@ -36,13 +36,15 @@ export default function SendToAgentButton({ fullWidth = false, }: Props) { const { sessions, sendInput } = useTerminal(); - const { projects, setActiveTabKey, pushToast } = useAppState( - useShallow((s) => ({ - projects: s.projects, - setActiveTabKey: s.setActiveTabKey, - pushToast: s.pushToast, - })), - ); + const { projects, setActiveTabKey, requestTerminalFocus, pushToast } = + useAppState( + useShallow((s) => ({ + projects: s.projects, + setActiveTabKey: s.setActiveTabKey, + requestTerminalFocus: s.requestTerminalFocus, + pushToast: s.pushToast, + })), + ); const [menuOpen, setMenuOpen] = useState(false); const rootRef = useRef(null); @@ -86,6 +88,11 @@ export default function SendToAgentButton({ // A courtesy, not part of the send: if the tab cannot be focused the // text still went. setActiveTabKey(terminalTabKey(sessionId)); + // Switching tabs is not the same as taking focus, and when the dock is + // open beside the terminal it just sent to, that tab is already the + // active one — so nothing above moves the caret off this button. The + // note is sitting in the prompt waiting for Enter; put the user there. + requestTerminalFocus(sessionId); } catch (e) { pushToast({ kind: "error", @@ -94,7 +101,7 @@ export default function SendToAgentButton({ }); } }, - [body, sendInput, setActiveTabKey, pushToast], + [body, sendInput, setActiveTabKey, requestTerminalFocus, pushToast], ); const onClick = useCallback(() => { diff --git a/app/src/components/terminal/TerminalView.test.tsx b/app/src/components/terminal/TerminalView.test.tsx index 42284d0..b6a18bf 100644 --- a/app/src/components/terminal/TerminalView.test.tsx +++ b/app/src/components/terminal/TerminalView.test.tsx @@ -538,3 +538,59 @@ describe("TerminalView — reaching the URL prompt without a mouse", () => { expect(document.activeElement).toBe(before); }); }); + +describe("TerminalView — focus on request", () => { + /** Mount, then deliberately give focus away, so what the assertions below + * observe is the *request* taking effect and never the focus `active` + * already grants on mount. That distinction is the whole point: the notes + * dock sends to a terminal whose tab is already active, where nothing + * changes and no `active` effect re-runs. */ + async function mountAndBlur() { + const view = mountSession("claude"); + await act(async () => {}); + const elsewhere = document.createElement("button"); + document.body.appendChild(elsewhere); + elsewhere.focus(); + expect(document.activeElement).toBe(elsewhere); + return view; + } + + it("focuses the terminal named by the request", async () => { + const view = await mountAndBlur(); + + await act(async () => { + useAppState.getState().requestTerminalFocus("s1"); + }); + + expect(document.activeElement).toBe(helperTextarea(view.container)); + }); + + it("ignores a request meant for another session", async () => { + const view = await mountAndBlur(); + const before = document.activeElement; + + await act(async () => { + useAppState.getState().requestTerminalFocus("s2"); + }); + + expect(document.activeElement).toBe(before); + expect(document.activeElement).not.toBe(helperTextarea(view.container)); + }); + + it("clears the request, so a second send focuses again", async () => { + const view = await mountAndBlur(); + + await act(async () => { + useAppState.getState().requestTerminalFocus("s1"); + }); + expect(useAppState.getState().pendingTerminalFocus).toBeNull(); + + const elsewhere = document.querySelector("button"); + (elsewhere as HTMLButtonElement).focus(); + + await act(async () => { + useAppState.getState().requestTerminalFocus("s1"); + }); + expect(document.activeElement).toBe(helperTextarea(view.container)); + }); +}); \ No newline at end of file diff --git a/app/src/components/terminal/TerminalView.tsx b/app/src/components/terminal/TerminalView.tsx index 1111ec6..d2b2740 100644 --- a/app/src/components/terminal/TerminalView.tsx +++ b/app/src/components/terminal/TerminalView.tsx @@ -731,6 +731,21 @@ export default function TerminalView({ sessionId, active }: Props) { } }, [active, gpuRenderingSetting]); + // Focus on demand, for the caller that cannot rely on the effect above. + // That one keys off `active`, so it covers switching *to* a terminal and + // nothing else — and the notes dock sends to the terminal already on screen, + // where `active` never changes. Consumed once and cleared, so asking twice + // for the same terminal works. + const pendingTerminalFocus = useAppState((s) => s.pendingTerminalFocus); + const clearPendingTerminalFocus = useAppState( + (s) => s.clearPendingTerminalFocus, + ); + useEffect(() => { + if (pendingTerminalFocus !== sessionId) return; + termRef.current?.focus(); + clearPendingTerminalFocus(); + }, [pendingTerminalFocus, sessionId, clearPendingTerminalFocus]); + // Auto-dismiss toast after 30 seconds — unless the user is standing in it. // A keyboard user who has just jumped into the toast is mid-decision, and // pulling it out from under them costs them the only route to finishing a diff --git a/app/src/store/appState.test.ts b/app/src/store/appState.test.ts index ddd9027..606680b 100644 --- a/app/src/store/appState.test.ts +++ b/app/src/store/appState.test.ts @@ -112,3 +112,27 @@ describe("toasts", () => { expect(toasts()).toHaveLength(2); }); }); + +describe("terminal focus requests", () => { + beforeEach(() => useAppState.setState({ pendingTerminalFocus: null })); + + const pending = () => useAppState.getState().pendingTerminalFocus; + + it("names the session that should take focus", () => { + useAppState.getState().requestTerminalFocus("s1"); + expect(pending()).toBe("s1"); + }); + + // Consumed once, exactly like `pendingHomeTab`. Without the clear, the + // second send to a terminal already holding the request would set the same + // value, no state would change, and no effect would re-run — which is the + // failure this whole mechanism exists to fix. + it("is cleared once consumed, so the same terminal can be asked again", () => { + useAppState.getState().requestTerminalFocus("s1"); + useAppState.getState().clearPendingTerminalFocus(); + expect(pending()).toBeNull(); + + useAppState.getState().requestTerminalFocus("s1"); + expect(pending()).toBe("s1"); + }); +}); diff --git a/app/src/store/appState.ts b/app/src/store/appState.ts index d3771da..d196b07 100644 --- a/app/src/store/appState.ts +++ b/app/src/store/appState.ts @@ -144,6 +144,21 @@ interface AppState { /** Consumed once by `ProjectHome`, then cleared. */ pendingHomeTab: { projectId: string; tab: string } | null; clearPendingHomeTab: () => void; + /** + * Ask a terminal to take keyboard focus. + * + * `TerminalView` already focuses when its tab *becomes* active, which covers + * switching to a terminal. It cannot cover being asked to focus the terminal + * that is already on screen — nothing changes, so no effect re-runs — and + * that is the ordinary case for the notes dock, which sits beside the + * terminal it sends to. + * + * Consumed once and cleared, like `pendingHomeTab`: holding the id would + * make a second request for the same terminal a no-op state write. + */ + pendingTerminalFocus: string | null; + requestTerminalFocus: (sessionId: string) => void; + clearPendingTerminalFocus: () => void; closeHomeTab: (projectId: string) => void; setActiveTabKey: (key: string) => void; cycleTab: (delta: number) => void; @@ -352,6 +367,9 @@ export const useAppState = create((set) => ({ }), pendingHomeTab: null, clearPendingHomeTab: () => set({ pendingHomeTab: null }), + pendingTerminalFocus: null, + requestTerminalFocus: (sessionId) => set({ pendingTerminalFocus: sessionId }), + clearPendingTerminalFocus: () => set({ pendingTerminalFocus: null }), closeHomeTab: (projectId) => set((state) => { const key = homeTabKey(projectId);