diff --git a/ui/goose2/src/features/chat/hooks/__tests__/ArtifactPolicyContext.test.tsx b/ui/goose2/src/features/chat/hooks/__tests__/ArtifactPolicyContext.test.tsx index 93ed16b1c61c..0ea636e86ab9 100644 --- a/ui/goose2/src/features/chat/hooks/__tests__/ArtifactPolicyContext.test.tsx +++ b/ui/goose2/src/features/chat/hooks/__tests__/ArtifactPolicyContext.test.tsx @@ -6,17 +6,14 @@ import { useArtifactPolicyContext, } from "../ArtifactPolicyContext"; +import { openPath } from "@tauri-apps/plugin-opener"; + const mockPathExists = vi.fn<(path: string) => Promise>(); -const mockOpenPath = vi.fn<(path: string) => Promise>(); vi.mock("@/shared/api/system", () => ({ pathExists: (path: string) => mockPathExists(path), })); -vi.mock("@tauri-apps/plugin-opener", () => ({ - openPath: (path: string) => mockOpenPath(path), -})); - function Probe({ readArgs, writeArgs, @@ -117,7 +114,7 @@ function FallbackProbe({ describe("ArtifactPolicyContext", () => { it("computes one primary host per message and resolves tool cards by args identity", () => { mockPathExists.mockReset(); - mockOpenPath.mockReset(); + vi.mocked(openPath).mockReset(); const readArgs = { path: "/Users/test/project-a/notes.md" }; const writeArgs = { paths: [ @@ -189,7 +186,7 @@ describe("ArtifactPolicyContext", () => { it("does not treat read-only tool paths as session artifacts", () => { mockPathExists.mockReset(); - mockOpenPath.mockReset(); + vi.mocked(openPath).mockReset(); const readArgs = { path: "/Users/test/project-a/notes.md" }; const messages: Message[] = [ { @@ -230,7 +227,7 @@ describe("ArtifactPolicyContext", () => { it("falls back to the home artifacts root when a project artifacts path is missing", async () => { mockPathExists.mockReset(); - mockOpenPath.mockReset(); + vi.mocked(openPath).mockReset(); mockPathExists.mockImplementation( async (path: string) => path === "/Users/test/.goose/artifacts/report.md", ); @@ -256,7 +253,7 @@ describe("ArtifactPolicyContext", () => { screen.getByRole("button", { name: "Open path" }).click(); await waitFor(() => { - expect(mockOpenPath).toHaveBeenCalledWith( + expect(vi.mocked(openPath)).toHaveBeenCalledWith( "/Users/test/.goose/artifacts/report.md", ); }); @@ -264,7 +261,7 @@ describe("ArtifactPolicyContext", () => { it("falls back from a working-dir artifacts path to the project root when the file lives there", async () => { mockPathExists.mockReset(); - mockOpenPath.mockReset(); + vi.mocked(openPath).mockReset(); mockPathExists.mockImplementation( async (path: string) => path === "/Users/test/project-a/README_ENHANCED.md", @@ -292,7 +289,7 @@ describe("ArtifactPolicyContext", () => { screen.getByRole("button", { name: "Open path" }).click(); await waitFor(() => { - expect(mockOpenPath).toHaveBeenCalledWith( + expect(vi.mocked(openPath)).toHaveBeenCalledWith( "/Users/test/project-a/README_ENHANCED.md", ); }); @@ -300,7 +297,7 @@ describe("ArtifactPolicyContext", () => { it("does not strip /artifacts/ from a parent directory in the path", async () => { mockPathExists.mockReset(); - mockOpenPath.mockReset(); + vi.mocked(openPath).mockReset(); // The file lives at the nested artifacts path — the parent `/artifacts/` should NOT be stripped mockPathExists.mockImplementation( async (path: string) => @@ -329,7 +326,7 @@ describe("ArtifactPolicyContext", () => { screen.getByRole("button", { name: "Open path" }).click(); await waitFor(() => { - expect(mockOpenPath).toHaveBeenCalledWith( + expect(vi.mocked(openPath)).toHaveBeenCalledWith( "/Users/test/artifacts/project/artifacts/README_ENHANCED.md", ); }); @@ -337,7 +334,7 @@ describe("ArtifactPolicyContext", () => { it("falls back correctly when /artifacts/ appears in a parent dir", async () => { mockPathExists.mockReset(); - mockOpenPath.mockReset(); + vi.mocked(openPath).mockReset(); // File is NOT at the artifacts path, but IS at the root-stripped path mockPathExists.mockImplementation( async (path: string) => @@ -366,7 +363,7 @@ describe("ArtifactPolicyContext", () => { screen.getByRole("button", { name: "Open path" }).click(); await waitFor(() => { - expect(mockOpenPath).toHaveBeenCalledWith( + expect(vi.mocked(openPath)).toHaveBeenCalledWith( "/Users/test/artifacts/project/README.md", ); }); @@ -374,7 +371,7 @@ describe("ArtifactPolicyContext", () => { it("uses assistant text after a tool call to populate file actions and the Files tab", () => { mockPathExists.mockReset(); - mockOpenPath.mockReset(); + vi.mocked(openPath).mockReset(); const writeArgs = {}; const messages: Message[] = [ { diff --git a/ui/goose2/src/features/chat/hooks/__tests__/useArtifactLinkHandler.test.tsx b/ui/goose2/src/features/chat/hooks/__tests__/useArtifactLinkHandler.test.tsx index 1d958cc5a827..610c2cd0b3d9 100644 --- a/ui/goose2/src/features/chat/hooks/__tests__/useArtifactLinkHandler.test.tsx +++ b/ui/goose2/src/features/chat/hooks/__tests__/useArtifactLinkHandler.test.tsx @@ -110,14 +110,12 @@ describe("useArtifactLinkHandler", () => { ); }); - it("ignores external URLs (does not call resolveMarkdownHref)", async () => { + it("does not intercept external URLs (defers to MarkdownLink's LinkSafetyModal)", async () => { const user = userEvent.setup(); - mockResolveMarkdownHref.mockReturnValue(null); render(); await user.click(screen.getByText("External")); - // isExternalHref returns true, so resolveMarkdownHref is never called expect(mockResolveMarkdownHref).not.toHaveBeenCalled(); expect(mockOpenResolvedPath).not.toHaveBeenCalled(); }); diff --git a/ui/goose2/src/features/chat/hooks/useArtifactLinkHandler.ts b/ui/goose2/src/features/chat/hooks/useArtifactLinkHandler.ts index c6600ab6f81a..52aaa54e71fe 100644 --- a/ui/goose2/src/features/chat/hooks/useArtifactLinkHandler.ts +++ b/ui/goose2/src/features/chat/hooks/useArtifactLinkHandler.ts @@ -1,10 +1,15 @@ import { useState, useCallback } from "react"; -import { isExternalHref } from "@/features/chat/lib/artifactPathPolicy"; +import { isExternalHref } from "@/shared/lib/isExternalHref"; import { useArtifactPolicyContext } from "@/features/chat/hooks/ArtifactPolicyContext"; /** * Delegated click handler that intercepts local link clicks within a * container and routes them through the artifact policy layer. + * + * External links are intentionally not handled here — MarkdownLink + * renders them as elements with preventDefault that open a + * LinkSafetyModal for confirmation. The isExternalHref early return + * below ensures there is no conflict. */ export function useArtifactLinkHandler() { const { resolveMarkdownHref, openResolvedPath } = useArtifactPolicyContext(); @@ -15,7 +20,9 @@ export function useArtifactLinkHandler() { const anchor = (event.target as HTMLElement).closest("a"); if (!anchor) return; const href = anchor.getAttribute("href"); - if (!href || isExternalHref(href)) return; + if (!href) return; + + if (isExternalHref(href)) return; event.preventDefault(); const resolved = resolveMarkdownHref(href); diff --git a/ui/goose2/src/features/chat/lib/artifactPathPolicy.ts b/ui/goose2/src/features/chat/lib/artifactPathPolicy.ts index 2c3c1252f61e..5cd22db2e5a1 100644 --- a/ui/goose2/src/features/chat/lib/artifactPathPolicy.ts +++ b/ui/goose2/src/features/chat/lib/artifactPathPolicy.ts @@ -17,7 +17,6 @@ export { evaluatePathScope, extractToolCallCandidates, inferHomeDirFromRoots, - isExternalHref, isWriteOrientedTool, normalizePath, resolveMarkdownLocalHref, diff --git a/ui/goose2/src/features/chat/lib/artifactPathPolicyCore.ts b/ui/goose2/src/features/chat/lib/artifactPathPolicyCore.ts index e499842d921a..e227e0e20b05 100644 --- a/ui/goose2/src/features/chat/lib/artifactPathPolicyCore.ts +++ b/ui/goose2/src/features/chat/lib/artifactPathPolicyCore.ts @@ -2,6 +2,7 @@ import { collectCommandArgPathCandidates, extractToolNamePathCandidates, } from "@/features/chat/lib/artifactPathCommandExtraction"; +import { isExternalHref } from "@/shared/lib/isExternalHref"; export type ArtifactCandidateSource = | "arg_key" @@ -145,17 +146,6 @@ function hasKnownScheme(value: string): boolean { return /^[a-zA-Z][a-zA-Z\d+.-]*:/.test(value); } -export function isExternalHref(href?: string): boolean { - if (!href) return false; - const lower = href.trim().toLowerCase(); - return ( - lower.startsWith("http://") || - lower.startsWith("https://") || - lower.startsWith("mailto:") || - lower.startsWith("tel:") - ); -} - function isLikelyAbsoluteFilesystemPath(candidate: string): boolean { if (/^[a-zA-Z]:[\\/]/.test(candidate)) return true; if (!candidate.startsWith("/")) return false; diff --git a/ui/goose2/src/features/chat/ui/__tests__/ContextPanel.test.tsx b/ui/goose2/src/features/chat/ui/__tests__/ContextPanel.test.tsx index 64c587318b3f..89747e53be68 100644 --- a/ui/goose2/src/features/chat/ui/__tests__/ContextPanel.test.tsx +++ b/ui/goose2/src/features/chat/ui/__tests__/ContextPanel.test.tsx @@ -28,8 +28,8 @@ vi.mock("@/shared/hooks/useChangedFiles", () => ({ }), })); -vi.mock("@tauri-apps/plugin-opener", () => ({ - openPath: vi.fn(), +vi.mock("@tauri-apps/api/event", () => ({ + listen: () => Promise.resolve(() => {}), })); vi.mock("@/shared/api/system", () => ({ diff --git a/ui/goose2/src/features/chat/ui/__tests__/FilesList.test.tsx b/ui/goose2/src/features/chat/ui/__tests__/FilesList.test.tsx index b5040028a427..a0c6ba77e044 100644 --- a/ui/goose2/src/features/chat/ui/__tests__/FilesList.test.tsx +++ b/ui/goose2/src/features/chat/ui/__tests__/FilesList.test.tsx @@ -3,21 +3,19 @@ import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { FilesList } from "../FilesList"; -const { mockListDirectoryEntries, mockOpenPath, mockRevealInFileManager } = - vi.hoisted(() => ({ +import { openPath } from "@tauri-apps/plugin-opener"; + +const { mockListDirectoryEntries, mockRevealInFileManager } = vi.hoisted( + () => ({ mockListDirectoryEntries: vi.fn(), - mockOpenPath: vi.fn(), mockRevealInFileManager: vi.fn(), - })); + }), +); vi.mock("@/shared/api/system", () => ({ listDirectoryEntries: mockListDirectoryEntries, })); -vi.mock("@tauri-apps/plugin-opener", () => ({ - openPath: mockOpenPath, -})); - vi.mock("@/shared/lib/fileManager", () => ({ revealInFileManager: mockRevealInFileManager, })); @@ -32,7 +30,7 @@ const makeEntry = (overrides: Record = {}) => ({ describe("FilesList", () => { beforeEach(() => { vi.clearAllMocks(); - mockOpenPath.mockResolvedValue(undefined); + vi.mocked(openPath).mockResolvedValue(undefined); mockRevealInFileManager.mockResolvedValue(undefined); mockListDirectoryEntries.mockResolvedValue([]); }); @@ -102,7 +100,7 @@ describe("FilesList", () => { "/Users/test/project/src", ); }); - expect(mockOpenPath).not.toHaveBeenCalled(); + expect(vi.mocked(openPath)).not.toHaveBeenCalled(); expect(screen.getByText("App.tsx")).toBeInTheDocument(); }); @@ -119,7 +117,9 @@ describe("FilesList", () => { await user.click(await screen.findByText("README.md")); - expect(mockOpenPath).toHaveBeenCalledWith("/Users/test/project/README.md"); + expect(vi.mocked(openPath)).toHaveBeenCalledWith( + "/Users/test/project/README.md", + ); }); it("supports context menu actions for folders and files", async () => { diff --git a/ui/goose2/src/features/chat/ui/__tests__/MessageBubble.test.tsx b/ui/goose2/src/features/chat/ui/__tests__/MessageBubble.test.tsx index 55531e8ed5f8..51bcf5d511f3 100644 --- a/ui/goose2/src/features/chat/ui/__tests__/MessageBubble.test.tsx +++ b/ui/goose2/src/features/chat/ui/__tests__/MessageBubble.test.tsx @@ -4,11 +4,7 @@ import userEvent from "@testing-library/user-event"; import { MessageBubble } from "../MessageBubble"; import { useAgentStore } from "@/features/agents/stores/agentStore"; import type { Message } from "@/shared/types/messages"; - -const mockOpenPath = vi.fn(); -vi.mock("@tauri-apps/plugin-opener", () => ({ - openPath: (path: string) => mockOpenPath(path), -})); +import { openPath } from "@tauri-apps/plugin-opener"; // ── helpers ─────────────────────────────────────────────────────────── @@ -40,7 +36,7 @@ function assistantMessage( describe("MessageBubble", () => { beforeEach(() => { useAgentStore.setState({ personas: [] }); - mockOpenPath.mockClear(); + vi.mocked(openPath).mockClear(); }); it("renders user message with correct alignment", () => { @@ -133,7 +129,7 @@ describe("MessageBubble", () => { await user.click( screen.getByRole("button", { name: /open attachment report\.pdf/i }), ); - expect(mockOpenPath).toHaveBeenCalledWith("/Users/test/report.pdf"); + expect(vi.mocked(openPath)).toHaveBeenCalledWith("/Users/test/report.pdf"); expect( screen.getByRole("button", { name: /open attachment screenshots/i }), ).toBeInTheDocument(); diff --git a/ui/goose2/src/shared/i18n/locales/en/common.json b/ui/goose2/src/shared/i18n/locales/en/common.json index 96aff012683b..3791fb77ac4f 100644 --- a/ui/goose2/src/shared/i18n/locales/en/common.json +++ b/ui/goose2/src/shared/i18n/locales/en/common.json @@ -45,6 +45,13 @@ "title": "Environment Variables", "toggleVisibility": "Toggle value visibility" }, + "linkSafety": { + "copied": "Copied!", + "copyLink": "Copy link", + "description": "You're about to visit an external website.", + "openLink": "Open link", + "title": "Open external link?" + }, "messageBranch": { "next": "Next branch", "page": "{{current}} of {{total}}", diff --git a/ui/goose2/src/shared/i18n/locales/es/common.json b/ui/goose2/src/shared/i18n/locales/es/common.json index 37c2aea48d60..074e770b93dd 100644 --- a/ui/goose2/src/shared/i18n/locales/es/common.json +++ b/ui/goose2/src/shared/i18n/locales/es/common.json @@ -40,6 +40,13 @@ "codeBlock": { "copyLabel": "Copiar código" }, + "linkSafety": { + "copied": "¡Copiado!", + "copyLink": "Copiar enlace", + "description": "Estás a punto de visitar un sitio web externo.", + "openLink": "Abrir enlace", + "title": "¿Abrir enlace externo?" + }, "environmentVariables": { "copyLabel": "Copiar variable de entorno", "title": "Variables de entorno", diff --git a/ui/goose2/src/shared/lib/isExternalHref.ts b/ui/goose2/src/shared/lib/isExternalHref.ts new file mode 100644 index 000000000000..6d7c4b4d4653 --- /dev/null +++ b/ui/goose2/src/shared/lib/isExternalHref.ts @@ -0,0 +1,10 @@ +export function isExternalHref(href?: string): boolean { + if (!href) return false; + const lower = href.trim().toLowerCase(); + return ( + lower.startsWith("http://") || + lower.startsWith("https://") || + lower.startsWith("mailto:") || + lower.startsWith("tel:") + ); +} diff --git a/ui/goose2/src/shared/ui/ai-elements/link-safety-modal.tsx b/ui/goose2/src/shared/ui/ai-elements/link-safety-modal.tsx new file mode 100644 index 000000000000..0f943ee84f07 --- /dev/null +++ b/ui/goose2/src/shared/ui/ai-elements/link-safety-modal.tsx @@ -0,0 +1,99 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { openUrl } from "@tauri-apps/plugin-opener"; +import { Button } from "@/shared/ui/button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/shared/ui/dialog"; + +interface LinkSafetyModalProps { + isOpen: boolean; + onClose: () => void; + url: string; +} + +export function LinkSafetyModal({ + isOpen, + onClose, + url, +}: LinkSafetyModalProps) { + const { t } = useTranslation("common"); + const [isCopied, setIsCopied] = useState(false); + const timeoutRef = useRef(0); + + useEffect(() => { + if (isOpen) setIsCopied(false); + }, [isOpen]); + + useEffect( + () => () => { + window.clearTimeout(timeoutRef.current); + }, + [], + ); + + const handleOpen = useCallback(async () => { + try { + await openUrl(url); + } catch (e: unknown) { + console.error("[linkSafety] openUrl failed:", e); + } + onClose(); + }, [url, onClose]); + + const handleCopy = useCallback(() => { + if (isCopied) return; + navigator.clipboard + .writeText(url) + .then(() => { + setIsCopied(true); + timeoutRef.current = window.setTimeout(() => setIsCopied(false), 2000); + }) + .catch((e: unknown) => + console.error("[linkSafety] clipboard write failed:", e), + ); + }, [url, isCopied]); + + const handleOpenChange = useCallback( + (open: boolean) => { + if (!open) onClose(); + }, + [onClose], + ); + + return ( + + + + {t("components.linkSafety.title")} + + {t("components.linkSafety.description")} + + +
+ {url} +
+ + + + + + + ); +} diff --git a/ui/goose2/src/shared/ui/ai-elements/message.tsx b/ui/goose2/src/shared/ui/ai-elements/message.tsx index e75b98067bc5..429a732d1e34 100644 --- a/ui/goose2/src/shared/ui/ai-elements/message.tsx +++ b/ui/goose2/src/shared/ui/ai-elements/message.tsx @@ -6,6 +6,8 @@ import { TooltipProvider, TooltipTrigger, } from "@/shared/ui/tooltip"; +import { isExternalHref } from "@/shared/lib/isExternalHref"; +import { LinkSafetyModal } from "@/shared/ui/ai-elements/link-safety-modal"; import { cn } from "@/shared/lib/cn"; import { cjk } from "@streamdown/cjk"; import { code } from "@streamdown/code"; @@ -325,17 +327,104 @@ export type MessageResponseProps = ComponentProps; const streamdownPlugins = { cjk, code, math, mermaid }; +type OpenLinkSafetyModal = (url: string) => void; + +const LinkSafetyContext = createContext(null); + +/** + * Custom link component that splits behavior by link type: + * - External links → with preventDefault that opens a LinkSafetyModal via context + * - Internal links → plain so useArtifactLinkHandler can intercept via closest("a") + * + * Both render as elements. useArtifactLinkHandler has an early return for external + * hrefs, so there is no conflict with its delegated click handler. + * + * This replaces Streamdown's built-in linkSafety which renders