Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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: [
Expand Down Expand Up @@ -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[] = [
{
Expand Down Expand Up @@ -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",
);
Expand All @@ -256,15 +253,15 @@ 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",
);
});
});

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",
Expand Down Expand Up @@ -292,15 +289,15 @@ 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",
);
});
});

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) =>
Expand Down Expand Up @@ -329,15 +326,15 @@ 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",
);
});
});

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) =>
Expand Down Expand Up @@ -366,15 +363,15 @@ 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",
);
});
});

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[] = [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down
11 changes: 9 additions & 2 deletions ui/goose2/src/features/chat/hooks/useArtifactLinkHandler.ts
Original file line number Diff line number Diff line change
@@ -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();
Expand All @@ -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);
Expand Down
1 change: 0 additions & 1 deletion ui/goose2/src/features/chat/lib/artifactPathPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ export {
evaluatePathScope,
extractToolCallCandidates,
inferHomeDirFromRoots,
isExternalHref,
isWriteOrientedTool,
normalizePath,
resolveMarkdownLocalHref,
Expand Down
12 changes: 1 addition & 11 deletions ui/goose2/src/features/chat/lib/artifactPathPolicyCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
collectCommandArgPathCandidates,
extractToolNamePathCandidates,
} from "@/features/chat/lib/artifactPathCommandExtraction";
import { isExternalHref } from "@/shared/lib/isExternalHref";

export type ArtifactCandidateSource =
| "arg_key"
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => ({
Expand Down
22 changes: 11 additions & 11 deletions ui/goose2/src/features/chat/ui/__tests__/FilesList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}));
Expand All @@ -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([]);
});
Expand Down Expand Up @@ -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();
});

Expand All @@ -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 () => {
Expand Down
10 changes: 3 additions & 7 deletions ui/goose2/src/features/chat/ui/__tests__/MessageBubble.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 ───────────────────────────────────────────────────────────

Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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();
Expand Down
7 changes: 7 additions & 0 deletions ui/goose2/src/shared/i18n/locales/en/common.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@
"title": "Environment Variables",
"toggleVisibility": "Toggle value visibility"
},
"linkSafety": {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Localize new link-safety keys in Spanish locale

LinkSafetyModal now reads common:components.linkSafety.*, but this commit only adds those keys to locales/en/common.json; locales/es/common.json is still missing them. In Spanish sessions the modal will fall back to English for every new label, creating a visible localization regression on a surface that was previously fully translated.

Useful? React with 👍 / 👎.

"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}}",
Expand Down
7 changes: 7 additions & 0 deletions ui/goose2/src/shared/i18n/locales/es/common.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
10 changes: 10 additions & 0 deletions ui/goose2/src/shared/lib/isExternalHref.ts
Original file line number Diff line number Diff line change
@@ -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:")
);
}
Loading
Loading