diff --git a/app/src/components/terminal/TerminalView.test.tsx b/app/src/components/terminal/TerminalView.test.tsx index 78392ea..8de6e72 100644 --- a/app/src/components/terminal/TerminalView.test.tsx +++ b/app/src/components/terminal/TerminalView.test.tsx @@ -1625,11 +1625,14 @@ describe("createOsc8LinkHandler — clicking a link Claude Code printed", () => h.hover?.(new MouseEvent("mousemove"), "javascript:alert(1)", range); expect(onOpenFile).not.toHaveBeenCalled(); expect(openUrlExternal).not.toHaveBeenCalled(); - // The card, if any, is the refusal — never the target, never an offer to - // open it. (The brief asked for no card here; the existing test "says so - // on hover when the target would be refused" requires the refusal card.) - expect(hoverCard()?.textContent ?? "").not.toContain("javascript:"); - expect(hoverCard()?.textContent ?? "").not.toContain("Open in viewer"); + // The refusal card (ruling (a)): present, no origin, never the target, + // never an offer to open it. + const card = hoverCard(); + expect(card).not.toBeNull(); + expect(card!.querySelector('[data-testid="osc8-hover-origin"]')).toBeNull(); + expect(card!.textContent).toContain("will not be opened"); + expect(card!.textContent).not.toContain("javascript:"); + expect(card!.textContent).not.toContain("Open in viewer"); }); it("refuses a file: target whose escapes do not decode", () => { @@ -1638,7 +1641,11 @@ describe("createOsc8LinkHandler — clicking a link Claude Code printed", () => h.activate(click(), "file:///workspace/%E0%A4%A", range); h.hover?.(new MouseEvent("mousemove"), "file:///workspace/%E0%A4%A", range); expect(onOpenFile).not.toHaveBeenCalled(); - expect(hoverCard()?.textContent ?? "").not.toContain("Open in viewer"); + const card = hoverCard(); + expect(card).not.toBeNull(); + expect(card!.querySelector('[data-testid="osc8-hover-origin"]')).toBeNull(); + expect(card!.textContent).toContain("will not be opened"); + expect(card!.textContent).not.toContain("Open in viewer"); }); it("the file: hover card names the viewer and the path", () => { @@ -1657,6 +1664,19 @@ describe("createOsc8LinkHandler — clicking a link Claude Code printed", () => expect(hoverCard()).toBeNull(); }); + it("holds a file-path click to the modifier its card promised", () => { + const h = createOsc8LinkHandler(() => host, () => state, vi.fn()); + state.mouseTracking = true; + h.showFileCard("src/foo.ts"); + expect(hoverCard()?.textContent).toContain("Shift+click"); + state.mouseTracking = false; + expect(h.opensFileLink(click())).toBe(false); + expect(h.opensFileLink(click({ shiftKey: true }))).toBe(true); + // A card drawn with nothing tracking promises nothing. + h.showFileCard("src/foo.ts"); + expect(h.opensFileLink(click())).toBe(true); + }); + it("declares allowNonHttpProtocols so file: targets reach it", () => { expect(createOsc8LinkHandler(() => host, () => state).allowNonHttpProtocols).toBe(true); }); @@ -1711,9 +1731,6 @@ describe("the link handler is wired into the terminal, and reads its live mode", expect(typeof handler.hover).toBe("function"); }); - // The gate has to ask the terminal, not a boolean captured at construction: - // the mode changes whenever the container prints a DECSET, which is several - // times a second in Claude Code. it("opens a file: target in the viewer against the session's project", async () => { vi.mocked(openFileViewer).mockClear(); mountSession("bash"); @@ -1727,7 +1744,9 @@ describe("the link handler is wired into the terminal, and reads its live mode", await Promise.resolve(); }); - expect(openFileViewer).toHaveBeenCalledWith("p1", "/workspace/api/src/a b.ts"); + expect(openFileViewer).toHaveBeenCalledWith( + "p1", "/workspace/api/src/a b.ts", undefined, undefined, undefined, + ); expect(openUrlExternal).not.toHaveBeenCalled(); }); @@ -1774,6 +1793,65 @@ describe("the link handler is wired into the terminal, and reads its live mode", expect(toasts.at(-1)?.dedupeKey).toBe("file-viewer-open"); }); + it("holds a plain-text file link to the modifier its hover card promised", async () => { + vi.mocked(openFileViewer).mockClear(); + const platform = navigator.platform; + Object.defineProperty(navigator, "platform", { value: "Linux x86_64", configurable: true }); + const register = vi.spyOn(Terminal.prototype, "registerLinkProvider"); + try { + mountSession("claude"); + const provider = register.mock.calls.at(-1)?.[0]; + if (!provider) throw new Error("no link provider was registered"); + await write("Edited src/foo.ts:42 today"); + const links = vi.fn(); + provider.provideLinks(1, links); + const [link] = links.mock.calls[0][0]; + + await write("\x1b[?1002h"); + link.hover?.(new MouseEvent("mousemove"), link.text); + expect(document.body.textContent).toContain("Shift+click to open"); + await write("\x1b[?1002l"); + + await act(async () => { + link.activate(new MouseEvent("click", { button: 0, detail: 1 }), link.text); + await Promise.resolve(); + }); + expect(openFileViewer).not.toHaveBeenCalled(); + + await act(async () => { + link.activate( + new MouseEvent("click", { button: 0, detail: 1, shiftKey: true }), + link.text, + ); + await Promise.resolve(); + }); + expect(openFileViewer).toHaveBeenCalledWith("p1", "src/foo.ts", 42, undefined, undefined); + } finally { + register.mockRestore(); + Object.defineProperty(navigator, "platform", { value: platform, configurable: true }); + } + }); + + it("says the project is not ready rather than doing nothing", async () => { + vi.mocked(openFileViewer).mockClear(); + useAppState.setState({ sessions: [] }); + render(); + + wiredHandler().activate( + new MouseEvent("click", { button: 0, detail: 1 }), + "file:///workspace/api/README.md", + range, + ); + + expect(openFileViewer).not.toHaveBeenCalled(); + const toasts = useAppState.getState().toasts; + expect(toasts.at(-1)?.detail).toContain("not ready yet"); + expect(toasts.at(-1)?.dedupeKey).toBe("file-viewer-open"); + }); + + // The gate has to ask the terminal, not a boolean captured at construction: + // the mode changes whenever the container prints a DECSET, which is several + // times a second in Claude Code. it("refuses a plain click once the container turns mouse tracking on", async () => { mountSession("claude"); await write("\x1b[?1002h"); diff --git a/app/src/components/terminal/TerminalView.tsx b/app/src/components/terminal/TerminalView.tsx index 80ca1b7..180d60e 100644 --- a/app/src/components/terminal/TerminalView.tsx +++ b/app/src/components/terminal/TerminalView.tsx @@ -140,6 +140,30 @@ function reportOpenFailure(e: unknown) { }); } +/** + * Open a path in the file viewer for this terminal's project, or say why not. + * The session record (and so the project) can arrive after the first render; + * a click in that window gets a toast rather than silently doing nothing. + */ +function openInViewer( + projectId: string | undefined, + path: string, + line?: number, + col?: number, + endLine?: number, +): void { + if (!projectId) { + useAppState.getState().pushToast({ + kind: "error", + message: "Could not open the file", + detail: "This terminal's project is not ready yet", + dedupeKey: "file-viewer-open", + }); + return; + } + openFileViewer(projectId, path, line, col, endLine).catch(reportViewerFailure); +} + /** Report a file-viewer open that the backend refused (not found, no container, cap). */ function reportViewerFailure(e: unknown): void { useAppState.getState().pushToast({ @@ -164,10 +188,17 @@ export type Osc8LinkHandler = ILinkHandler & { /** * Draw the "Open in viewer" card for a path matched in plain text by the * file-path link provider. Takes the raw path (a relative path stays - * relative — `file://src/x` would parse `src` as a host), and leaves - * `modifierPromised` alone: the provider applies its own gate. + * relative — `file://src/x` would parse `src` as a host). Records the + * card's modifier promise exactly as `hover` does, so + * {@link opensFileLink} can hold the click to it. */ showFileCard(path: string): void; + /** + * The file-path provider's click gate: {@link opensOnClick} with whatever + * the card on screen promised. There is only ever one card, and `clear()` + * resets the promise with it, so sharing the flag with OSC 8 links is exact. + */ + opensFileLink(event: MouseEvent): boolean; }; /** @@ -652,11 +683,19 @@ export function createOsc8LinkHandler( clear(); const host = getHost(); if (!host) return; + const ctx = readState(); + // Same promise as `hover`: the card names a modifier, so the click is + // held to it even if the container drops tracking before it lands. + modifierPromised = ctx.mouseTracking; card = makeCard(); - fillFileCard(card, path, readState()); + fillFileCard(card, path, ctx); host.appendChild(card); }, + opensFileLink(event) { + return opensOnClick(event, readState(), modifierPromised); + }, + allowNonHttpProtocols: true, }; } @@ -998,11 +1037,7 @@ export default function TerminalView({ sessionId, active }: Props) { linkHandler: (osc8LinkHandlerRef.current = createOsc8LinkHandler( () => term.element ?? null, () => readClickContext(term), - (path) => { - if (projectIdRef.current) { - openFileViewer(projectIdRef.current, path).catch(reportViewerFailure); - } - }, + (path) => openInViewer(projectIdRef.current, path), )), fontFamily: "'JetBrains Mono', 'Fira Code', 'Cascadia Code', Menlo, Monaco, monospace", theme: { @@ -1073,17 +1108,15 @@ export default function TerminalView({ sessionId, active }: Props) { // File paths in plain text open in the file viewer. Registered after the // addon so URLs are claimed first; the matcher also refuses anything - // inside a `scheme://` span. Same gate as the URL branch above. The hover - // card is the OSC 8 handler's, fed the raw path (see `showFileCard`). + // inside a `scheme://` span. The hover card is the OSC 8 handler's, fed the + // raw path, and the click gate honours the modifier that card promised. const filePathLinks = term.registerLinkProvider( createFilePathLinkProvider( term, - (m: FilePathMatch) => { - const pid = projectIdRef.current; - if (!pid) return; - openFileViewer(pid, m.path, m.line, m.col, m.endLine).catch(reportViewerFailure); - }, - (event) => opensOnClick(event, readClickContext(term)), + (m: FilePathMatch) => + openInViewer(projectIdRef.current, m.path, m.line, m.col, m.endLine), + // Held to what the card promised, like OSC 8 links (see `showFileCard`). + (event) => osc8LinkHandlerRef.current?.opensFileLink(event) ?? false, { show: (path) => osc8LinkHandlerRef.current?.showFileCard(path), hide: () => osc8LinkHandlerRef.current?.dismiss(),