fix: links in chat could not be opened (#8544)

Signed-off-by: Matt Toohey <contact@matttoohey.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

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<boolean>>();
-const mockOpenPath = vi.fn<(path: string) => Promise<void>>();
 
 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[] = [
       {

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(<Harness href="https://example.com" label="External" />);
     await user.click(screen.getByText("External"));
 
-    // isExternalHref returns true, so resolveMarkdownHref is never called
     expect(mockResolveMarkdownHref).not.toHaveBeenCalled();
     expect(mockOpenResolvedPath).not.toHaveBeenCalled();
   });

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 <a> 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);

ui/goose2/src/features/chat/lib/artifactPathPolicy.ts

@@ -17,7 +17,6 @@ export {
   evaluatePathScope,
   extractToolCallCandidates,
   inferHomeDirFromRoots,
-  isExternalHref,
   isWriteOrientedTool,
   normalizePath,
   resolveMarkdownLocalHref,

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;

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", () => ({

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<string, unknown> = {}) => ({
 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 () => {

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();

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}}",

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",

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:")
+  );
+}

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<number>(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 (
+    <Dialog open={isOpen} onOpenChange={handleOpenChange}>
+      <DialogContent>
+        <DialogHeader>
+          <DialogTitle>{t("components.linkSafety.title")}</DialogTitle>
+          <DialogDescription>
+            {t("components.linkSafety.description")}
+          </DialogDescription>
+        </DialogHeader>
+        <div className="break-all rounded-md bg-muted p-3 font-mono text-sm">
+          {url}
+        </div>
+        <DialogFooter className="flex-row">
+          <Button
+            className="flex-1"
+            onClick={handleCopy}
+            type="button"
+            variant="outline"
+          >
+            {isCopied
+              ? t("components.linkSafety.copied")
+              : t("components.linkSafety.copyLink")}
+          </Button>
+          <Button className="flex-1" onClick={handleOpen} type="button">
+            {t("components.linkSafety.openLink")}
+          </Button>
+        </DialogFooter>
+      </DialogContent>
+    </Dialog>
+  );
+}

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<typeof Streamdown>;
 
 const streamdownPlugins = { cjk, code, math, mermaid };
 
+type OpenLinkSafetyModal = (url: string) => void;
+
+const LinkSafetyContext = createContext<OpenLinkSafetyModal | null>(null);
+
+/**
+ * Custom link component that splits behavior by link type:
+ * - External links → <a> with preventDefault that opens a LinkSafetyModal via context
+ * - Internal links → plain <a> so useArtifactLinkHandler can intercept via closest("a")
+ *
+ * Both render as <a> 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 <button> for ALL
+ * links, breaking artifact navigation since useArtifactLinkHandler matches on <a>.
+ */
+const MarkdownLink = memo(
+  ({
+    children,
+    href,
+    node: _node,
+    ...rest
+  }: ComponentProps<"a"> & { node?: unknown }) => {
+    const openModal = useContext(LinkSafetyContext);
+
+    if (isExternalHref(href)) {
+      return (
+        <a
+          className="wrap-anywhere font-medium text-primary underline"
+          data-streamdown="link"
+          href={href}
+          rel="noreferrer"
+          onClick={(e) => {
+            e.preventDefault();
+            openModal?.(href ?? "");
+          }}
+          {...rest}
+        >
+          {children}
+        </a>
+      );
+    }
+
+    return (
+      <a
+        className="wrap-anywhere font-medium text-primary underline"
+        data-streamdown="link"
+        href={href}
+        rel="noreferrer"
+        {...rest}
+      >
+        {children}
+      </a>
+    );
+  },
+);
+MarkdownLink.displayName = "MarkdownLink";
+
+const streamdownComponents = { a: MarkdownLink };
+
+const linkSafetyConfig: ComponentProps<typeof Streamdown>["linkSafety"] = {
+  enabled: false,
+};
+
 export const MessageResponse = memo(
-  ({ className, ...props }: MessageResponseProps) => (
-    <Streamdown
-      className={cn(
-        "size-full [&>*:first-child]:mt-0 [&>*:last-child]:mb-0",
-        className,
-      )}
-      plugins={streamdownPlugins}
-      {...props}
-    />
-  ),
+  ({ className, ...props }: MessageResponseProps) => {
+    const [modalUrl, setModalUrl] = useState<string | null>(null);
+
+    const openModal = useCallback((url: string) => {
+      setModalUrl(url);
+    }, []);
+
+    const closeModal = useCallback(() => {
+      setModalUrl(null);
+    }, []);
+
+    return (
+      <LinkSafetyContext.Provider value={openModal}>
+        <Streamdown
+          className={cn(
+            "size-full [&>*:first-child]:mt-0 [&>*:last-child]:mb-0",
+            className,
+          )}
+          components={streamdownComponents}
+          linkSafety={linkSafetyConfig}
+          plugins={streamdownPlugins}
+          {...props}
+        />
+        <LinkSafetyModal
+          isOpen={modalUrl !== null}
+          onClose={closeModal}
+          url={modalUrl ?? ""}
+        />
+      </LinkSafetyContext.Provider>
+    );
+  },
+  // Internal state (modalUrl) is intentionally outside this comparator —
+  // React always re-renders when local state changes regardless of memo.
+  // If modalUrl is ever lifted to a prop, this comparator must be updated.
   (prevProps, nextProps) =>
     prevProps.children === nextProps.children &&
     nextProps.isAnimating === prevProps.isAnimating,

ui/goose2/src/test/setup.ts

@@ -4,6 +4,11 @@ import { beforeEach, vi } from "vitest";
 
 import { DEFAULT_LOCALE, TRANSLATION_NAMESPACES, i18n } from "@/shared/i18n";
 
+vi.mock("@tauri-apps/plugin-opener", () => ({
+  openPath: vi.fn(),
+  openUrl: vi.fn(),
+}));
+
 vi.mock("react-syntax-highlighter", () => ({
   Prism: ({ children }: { children: React.ReactNode }) =>
     React.createElement(