From 59ccc1f171ddb9074efd756c1e45d10ab6bb51e2 Mon Sep 17 00:00:00 2001 From: rsolmano Date: Sun, 30 Aug 2026 16:57:22 +0200 Subject: [PATCH 1/6] todo: Draft and approve the evidence-link handling design ThinkRail-Todo: 01a052bd-94dc-7921-b0ce-d561c74f094a/t_80a9b99c517a --- apps/web/src/chat/SPEC.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/web/src/chat/SPEC.md b/apps/web/src/chat/SPEC.md index d7521ee8..c0aac582 100644 --- a/apps/web/src/chat/SPEC.md +++ b/apps/web/src/chat/SPEC.md @@ -79,7 +79,7 @@ blocks in order into rows; `ChatTurnView` dispatches on row kind: summarized disappear immediately rather than only after reload. Its `summary` opens on click (`data-testid="chat-compaction"`). Hydration/reopen starts directly from that same durable form. Both forms share the **"Context compacted"** title; only facts unavailable after reload disappear. -- `markdown` — a non-empty assistant text block (react-markdown + remark-gfm + shiki). A fenced +- `markdown` — a non-empty assistant text block (react-markdown + remark-gfm + shiki). Safe worktree-relative links in assistant prose open the target in ThinkRail through `ChatTurnView`'s existing workspace-file callback; absolute paths are accepted only when they normalize inside the active worktree, while URL schemes, protocol-relative URLs, fragments, and unsafe/outside paths retain ordinary safe new-tab anchor behavior. The generic `Markdown` primitive remains props-driven and receives this behavior as an `a` component override only at the assistant-turn integration edge. A fenced ```mermaid block renders as a themed diagram via `tools/visualize`'s `MermaidView` (fullscreen pan-zoom, error → source fallback) — uniform across every `Markdown` surface (chat, file/specs preview); until mounted it renders as highlighted source, so static contexts (`RenderedDiff`'s From 378177b964ed32d01b1ed6290531613c3ec289c3 Mon Sep 17 00:00:00 2001 From: rsolmano Date: Sun, 30 Aug 2026 17:11:11 +0200 Subject: [PATCH 2/6] todo: Implement and test safe workspace-file navigation ThinkRail-Todo: 01a052bd-94dc-7921-b0ce-d561c74f094a/t_94aa4f1bf4cf --- apps/web/src/chat/SPEC.md | 2 +- apps/web/src/chat/assistantLinks.test.tsx | 47 +++++++++++++++ apps/web/src/chat/assistantLinks.tsx | 72 +++++++++++++++++++++++ apps/web/src/chat/turns.tsx | 7 ++- e2e/tool-file-links.spec.ts | 45 ++++++++++++++ 5 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 apps/web/src/chat/assistantLinks.test.tsx create mode 100644 apps/web/src/chat/assistantLinks.tsx diff --git a/apps/web/src/chat/SPEC.md b/apps/web/src/chat/SPEC.md index c0aac582..686c3369 100644 --- a/apps/web/src/chat/SPEC.md +++ b/apps/web/src/chat/SPEC.md @@ -79,7 +79,7 @@ blocks in order into rows; `ChatTurnView` dispatches on row kind: summarized disappear immediately rather than only after reload. Its `summary` opens on click (`data-testid="chat-compaction"`). Hydration/reopen starts directly from that same durable form. Both forms share the **"Context compacted"** title; only facts unavailable after reload disappear. -- `markdown` — a non-empty assistant text block (react-markdown + remark-gfm + shiki). Safe worktree-relative links in assistant prose open the target in ThinkRail through `ChatTurnView`'s existing workspace-file callback; absolute paths are accepted only when they normalize inside the active worktree, while URL schemes, protocol-relative URLs, fragments, and unsafe/outside paths retain ordinary safe new-tab anchor behavior. The generic `Markdown` primitive remains props-driven and receives this behavior as an `a` component override only at the assistant-turn integration edge. A fenced +- `markdown` — a non-empty assistant text block (react-markdown + remark-gfm + shiki). Safe worktree-relative links in assistant prose open the target in ThinkRail through `ChatTurnView`'s existing workspace-file callback; absolute paths are accepted only when they normalize inside the active worktree, while URL schemes, protocol-relative URLs, fragments, and unsafe/outside paths retain ordinary safe new-tab anchor behavior. The generic `Markdown` primitive remains props-driven and receives this behavior as an `a` component override only at the assistant-turn integration edge. That override keeps a stable component identity while its workspace inputs are unchanged: workbench focus can rerender a chat row between pointer-down and click, and replacing the anchor in that interval cancels activation. A fenced ```mermaid block renders as a themed diagram via `tools/visualize`'s `MermaidView` (fullscreen pan-zoom, error → source fallback) — uniform across every `Markdown` surface (chat, file/specs preview); until mounted it renders as highlighted source, so static contexts (`RenderedDiff`'s diff --git a/apps/web/src/chat/assistantLinks.test.tsx b/apps/web/src/chat/assistantLinks.test.tsx new file mode 100644 index 00000000..7de2a674 --- /dev/null +++ b/apps/web/src/chat/assistantLinks.test.tsx @@ -0,0 +1,47 @@ +import { describe, expect, test } from "bun:test"; +import { renderToStaticMarkup } from "react-dom/server"; +import { AssistantMarkdown, assistantFileTarget } from "./assistantLinks"; + +describe("assistantFileTarget", () => { + test("accepts normalized relative paths and absolute paths inside the workspace", () => { + expect(assistantFileTarget(".thinkrail/context/report.md", "/repo")).toBe( + ".thinkrail/context/report.md", + ); + expect(assistantFileTarget("docs/../README.md#usage", "/repo")).toBe("README.md"); + expect(assistantFileTarget("/repo/docs/report.md?raw=1", "/repo")).toBe("docs/report.md"); + }); + + test("rejects external, fragment, outside-workspace, and context-free targets", () => { + for (const href of [ + "https://example.com/report", + "mailto:owner@example.com", + "//example.com/report", + "#report", + "../report.md", + "/tmp/report.md", + ]) { + expect(assistantFileTarget(href, "/repo")).toBeNull(); + } + expect(assistantFileTarget("README.md", undefined)).toBeNull(); + }); +}); + +test("assistant Markdown distinguishes workspace files from ordinary anchors", () => { + const html = renderToStaticMarkup( + {}} + />, + ); + + expect(html).toContain('data-testid="chat-file-link"'); + expect(html).toContain('data-path=".thinkrail/context/report.md"'); + expect(html.match(/target="_blank"/g)).toHaveLength(2); + expect(html).toContain('href="https://example.com/report"'); + expect(html).toContain('href="../report.md"'); +}); diff --git a/apps/web/src/chat/assistantLinks.tsx b/apps/web/src/chat/assistantLinks.tsx new file mode 100644 index 00000000..c69dc47a --- /dev/null +++ b/apps/web/src/chat/assistantLinks.tsx @@ -0,0 +1,72 @@ +import { type ReactNode, useMemo } from "react"; +import type { Components } from "react-markdown"; +import { Markdown } from "./Markdown"; +import { toolFileTarget } from "./tools/ToolFileLink"; + +export function assistantFileTarget( + href: string | undefined, + workspaceRoot: string | undefined, +): string | null { + const candidate = href?.trim(); + if (!candidate || !workspaceRoot || candidate.startsWith("#") || candidate.startsWith("//")) { + return null; + } + const path = candidate.split(/[?#]/, 1)[0]; + return path ? toolFileTarget(path, workspaceRoot) : null; +} + +function AssistantLink({ + href, + children, + workspaceRoot, + onOpenFile, +}: { + href?: string | undefined; + children?: ReactNode; + workspaceRoot?: string | undefined; + onOpenFile?: ((path: string) => void) | undefined; +}) { + const target = assistantFileTarget(href, workspaceRoot); + if (!target || !onOpenFile) { + return ( + + {children} + + ); + } + return ( + { + event.preventDefault(); + onOpenFile(target); + }} + > + {children} + + ); +} + +export function AssistantMarkdown({ + text, + workspaceRoot, + onOpenFile, +}: { + text: string; + workspaceRoot?: string | undefined; + onOpenFile?: ((path: string) => void) | undefined; +}) { + const components = useMemo( + () => ({ + a: ({ href, children }) => ( + + {children} + + ), + }), + [workspaceRoot, onOpenFile], + ); + return ; +} diff --git a/apps/web/src/chat/turns.tsx b/apps/web/src/chat/turns.tsx index 3d79e7ad..e74a4cdb 100644 --- a/apps/web/src/chat/turns.tsx +++ b/apps/web/src/chat/turns.tsx @@ -22,6 +22,7 @@ import { userText, } from "@/lib"; import { ActivityGroup } from "./ActivityGroup"; +import { AssistantMarkdown } from "./assistantLinks"; import { FileChip } from "./FileChip"; import { useFold, useSelection } from "./foldState"; import { Markdown } from "./Markdown"; @@ -91,7 +92,11 @@ export function ChatTurnView({ data-role="assistant" className="tr-text-reading text-text-default" > - + ); case "subagentCompletion": diff --git a/e2e/tool-file-links.spec.ts b/e2e/tool-file-links.spec.ts index 52f41512..60b6389a 100644 --- a/e2e/tool-file-links.spec.ts +++ b/e2e/tool-file-links.spec.ts @@ -175,6 +175,51 @@ async function returnToChat(page: Page): Promise { await expect(page.getByTestId("chat-view")).toBeVisible(); } +test("assistant Markdown opens safe relative files without navigating the browser", async ({ + page, +}) => { + await openFixtureProject(page); + seedWorkspaceSession(repoCwd(), { + name: "assistant file links", + messages: [ + { role: "user", text: "show the evidence", timestamp: BASE_TS }, + { + role: "assistant", + text: [ + "[Open README](README.md)", + "[Open docs](https://example.com/docs)", + "[Outside workspace](../outside.md)", + ].join("\n\n"), + timestamp: BASE_TS + 1_000, + }, + ], + }); + + await expect(defaultWorkspaceRow(page)).toBeVisible(); + await enterDefaultWorkspace(page); + await openChatFromHistory(page, "assistant file links"); + + const message = page.locator('[data-testid="chat-message"][data-role="assistant"]').last(); + const local = message.getByRole("link", { name: "Open README" }); + await expect(local).toHaveAttribute("data-testid", "chat-file-link"); + await expect(local).toHaveAttribute("data-path", "README.md"); + await expect(local).not.toHaveAttribute("target", "_blank"); + await expect(message.getByRole("link", { name: "Open docs" })).toHaveAttribute( + "target", + "_blank", + ); + await expect(message.getByRole("link", { name: "Outside workspace" })).toHaveAttribute( + "target", + "_blank", + ); + + await local.click(); + const fileTab = page.locator('[data-testid="editor-tab"][data-kind="file"]'); + await expect(fileTab).toHaveCount(1); + await expect(fileTab).toContainText("README.md"); + await expect(fileTab).toHaveAttribute("data-preview", "true"); +}); + test("structured tool paths reuse the preview tab while rich tool results stay intentional", async ({ page, }) => { From a991cd02c189a9d005bfcb00d15f808fb8b1795d Mon Sep 17 00:00:00 2001 From: rsolmano Date: Sun, 30 Aug 2026 17:34:04 +0200 Subject: [PATCH 3/6] todo: Review the full diff and capture UI proof ThinkRail-Todo: 01a052bd-94dc-7921-b0ce-d561c74f094a/t_e420a5e12eb5 --- apps/web/src/chat/SPEC.md | 12 +++++++++++- apps/web/src/chat/assistantLinks.test.tsx | 12 +++++++++--- apps/web/src/chat/assistantLinks.tsx | 21 ++++++++++++--------- e2e/tool-file-links.spec.ts | 3 ++- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/apps/web/src/chat/SPEC.md b/apps/web/src/chat/SPEC.md index 686c3369..e07c5855 100644 --- a/apps/web/src/chat/SPEC.md +++ b/apps/web/src/chat/SPEC.md @@ -79,7 +79,17 @@ blocks in order into rows; `ChatTurnView` dispatches on row kind: summarized disappear immediately rather than only after reload. Its `summary` opens on click (`data-testid="chat-compaction"`). Hydration/reopen starts directly from that same durable form. Both forms share the **"Context compacted"** title; only facts unavailable after reload disappear. -- `markdown` — a non-empty assistant text block (react-markdown + remark-gfm + shiki). Safe worktree-relative links in assistant prose open the target in ThinkRail through `ChatTurnView`'s existing workspace-file callback; absolute paths are accepted only when they normalize inside the active worktree, while URL schemes, protocol-relative URLs, fragments, and unsafe/outside paths retain ordinary safe new-tab anchor behavior. The generic `Markdown` primitive remains props-driven and receives this behavior as an `a` component override only at the assistant-turn integration edge. That override keeps a stable component identity while its workspace inputs are unchanged: workbench focus can rerender a chat row between pointer-down and click, and replacing the anchor in that interval cancels activation. A fenced +- `markdown` — a non-empty assistant text block (react-markdown + remark-gfm + shiki). Safe + worktree-relative links in assistant prose open the target in ThinkRail through `ChatTurnView`'s existing + workspace-file callback; absolute paths are accepted only when they normalize inside the active worktree, + while URL schemes, protocol-relative URLs, fragments, and unsafe/outside paths retain ordinary safe + new-tab anchor behavior. Percent-encoded file paths are decoded once before validation, so encoded + separators and traversal cannot bypass containment. The generic `Markdown` primitive remains props-driven + and receives this behavior as an `a` component override only at the assistant-turn integration edge; + accepted workspace targets render as button controls without a raw browser `href`, so alternate native + anchor activation cannot escape into the SPA fallback. That override keeps a stable component identity + while its workspace inputs are unchanged: workbench focus can rerender a chat row between pointer-down and + click, and replacing the control in that interval cancels activation. A fenced ```mermaid block renders as a themed diagram via `tools/visualize`'s `MermaidView` (fullscreen pan-zoom, error → source fallback) — uniform across every `Markdown` surface (chat, file/specs preview); until mounted it renders as highlighted source, so static contexts (`RenderedDiff`'s diff --git a/apps/web/src/chat/assistantLinks.test.tsx b/apps/web/src/chat/assistantLinks.test.tsx index 7de2a674..17eecf02 100644 --- a/apps/web/src/chat/assistantLinks.test.tsx +++ b/apps/web/src/chat/assistantLinks.test.tsx @@ -8,6 +8,9 @@ describe("assistantFileTarget", () => { ".thinkrail/context/report.md", ); expect(assistantFileTarget("docs/../README.md#usage", "/repo")).toBe("README.md"); + expect(assistantFileTarget("reports/manual%20report.md", "/repo")).toBe( + "reports/manual report.md", + ); expect(assistantFileTarget("/repo/docs/report.md?raw=1", "/repo")).toBe("docs/report.md"); }); @@ -18,7 +21,9 @@ describe("assistantFileTarget", () => { "//example.com/report", "#report", "../report.md", + "%2e%2e/report.md", "/tmp/report.md", + "report%E0%A4%A.md", ]) { expect(assistantFileTarget(href, "/repo")).toBeNull(); } @@ -30,7 +35,7 @@ test("assistant Markdown distinguishes workspace files from ordinary anchors", ( const html = renderToStaticMarkup( )", "[site](https://example.com/report)", "[outside](../report.md)", ].join("\n\n")} @@ -39,8 +44,9 @@ test("assistant Markdown distinguishes workspace files from ordinary anchors", ( />, ); - expect(html).toContain('data-testid="chat-file-link"'); - expect(html).toContain('data-path=".thinkrail/context/report.md"'); + expect(html).toContain(' ); } diff --git a/e2e/tool-file-links.spec.ts b/e2e/tool-file-links.spec.ts index 60b6389a..d1e50b15 100644 --- a/e2e/tool-file-links.spec.ts +++ b/e2e/tool-file-links.spec.ts @@ -200,9 +200,10 @@ test("assistant Markdown opens safe relative files without navigating the browse await openChatFromHistory(page, "assistant file links"); const message = page.locator('[data-testid="chat-message"][data-role="assistant"]').last(); - const local = message.getByRole("link", { name: "Open README" }); + const local = message.getByRole("button", { name: "Open README" }); await expect(local).toHaveAttribute("data-testid", "chat-file-link"); await expect(local).toHaveAttribute("data-path", "README.md"); + await expect(local).not.toHaveAttribute("href", "README.md"); await expect(local).not.toHaveAttribute("target", "_blank"); await expect(message.getByRole("link", { name: "Open docs" })).toHaveAttribute( "target", From 0215fcb97ba9063845625f957eedf42b3ead7668 Mon Sep 17 00:00:00 2001 From: rsolmano Date: Mon, 31 Aug 2026 13:32:22 +0200 Subject: [PATCH 4/6] fix(web): address assistant link review findings --- apps/web/src/chat/Markdown.tsx | 3 ++ apps/web/src/chat/SPEC.md | 16 +++++--- apps/web/src/chat/assistantLinks.test.tsx | 39 ++++++++++++++++++- apps/web/src/chat/assistantLinks.tsx | 26 ++++++++++--- apps/web/src/chat/fileTargets.test.ts | 19 +++++++++ apps/web/src/chat/fileTargets.ts | 26 +++++++++++++ apps/web/src/chat/tools/SPEC.md | 8 ++-- apps/web/src/chat/tools/ToolFileLink.tsx | 18 ++------- apps/web/src/chat/tools/ToolFileLinks.test.ts | 16 -------- 9 files changed, 125 insertions(+), 46 deletions(-) create mode 100644 apps/web/src/chat/fileTargets.test.ts create mode 100644 apps/web/src/chat/fileTargets.ts diff --git a/apps/web/src/chat/Markdown.tsx b/apps/web/src/chat/Markdown.tsx index 5fe1a89a..0e1934e3 100644 --- a/apps/web/src/chat/Markdown.tsx +++ b/apps/web/src/chat/Markdown.tsx @@ -14,12 +14,14 @@ export function Markdown({ className = CHAT_PROSE, remarkPlugins, rehypePlugins, + urlTransform, components, }: { text: string; className?: string; remarkPlugins?: ComponentProps["remarkPlugins"]; rehypePlugins?: ComponentProps["rehypePlugins"]; + urlTransform?: ComponentProps["urlTransform"]; components?: ComponentProps["components"]; }) { return ( @@ -27,6 +29,7 @@ export function Markdown({ {text} diff --git a/apps/web/src/chat/SPEC.md b/apps/web/src/chat/SPEC.md index 7b8ad370..eeb3d3bc 100644 --- a/apps/web/src/chat/SPEC.md +++ b/apps/web/src/chat/SPEC.md @@ -84,8 +84,11 @@ blocks in order into rows; `ChatTurnView` dispatches on row kind: workspace-file callback; absolute paths are accepted only when they normalize inside the active worktree, while URL schemes, protocol-relative URLs, fragments, and unsafe/outside paths retain ordinary safe new-tab anchor behavior. Percent-encoded file paths are decoded once before validation, so encoded - separators and traversal cannot bypass containment. The generic `Markdown` primitive remains props-driven - and receives this behavior as an `a` component override only at the assistant-turn integration edge; + separators and traversal cannot bypass containment. A narrow assistant-only URL transform preserves + recognized Windows drive-letter anchor paths, including Markdown's percent-encoded backslash form, until + validation; every other value delegates to react-markdown's default sanitizer, and a rejected Windows path + is re-sanitized before fallback anchor rendering. The generic `Markdown` primitive remains props-driven and + receives this behavior only as an `a` component override at the assistant-turn integration edge; accepted workspace targets render as button controls without a raw browser `href`, so alternate native anchor activation cannot escape into the SPA fallback. That override keeps a stable component identity while its workspace inputs are unchanged: workbench focus can rerender a chat row between pointer-down and @@ -889,14 +892,15 @@ from their `toolCall` args and reply through **`ChatActions`** (see below). Work ## Boundary -- **Public surface:** the registry API (`toolRegistry`), the props-driven slash-completion primitive, and - the renderers (incl. the presentational `Markdown` — GFM + shiki, no store/transport; the rendering is fixed but the **prose skin** is the +- **Public surface:** the registry API (`toolRegistry`), the shared workspace-file target canonicalizer + (`fileTargets`), the props-driven slash-completion primitive, and the renderers (incl. the presentational + `Markdown` — GFM + shiki, no store/transport; the rendering is fixed but the **prose skin** is the caller's via an optional `className` — chat uses the compact bubble skin (`tr-prose-chat`), `panels/MarkdownPreview` the document skin (`tr-prose-doc`). A skin names exactly one generated `tr-prose-*` system and then carries only spacing/measure/chrome — no size, weight, leading or tracking (see `styles/TYPOGRAPHY.md`); a caller may - also **extend** the render with extra `remarkPlugins` + `components`, e.g. the file view's GitHub - alert callouts), the view types + also **extend** the render with an optional `urlTransform`, extra `remarkPlugins`, and `components`, e.g. + the file view's GitHub alert callouts), the view types (`types.ts`, incl. `ToolResultState` + `ExtUiDialogRequest`), and `ChatView` (lazy-mounted by the shell workbench resource renderer; diff --git a/apps/web/src/chat/assistantLinks.test.tsx b/apps/web/src/chat/assistantLinks.test.tsx index 17eecf02..261c953a 100644 --- a/apps/web/src/chat/assistantLinks.test.tsx +++ b/apps/web/src/chat/assistantLinks.test.tsx @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { renderToStaticMarkup } from "react-dom/server"; -import { AssistantMarkdown, assistantFileTarget } from "./assistantLinks"; +import { AssistantMarkdown, assistantFileTarget, assistantUrlTransform } from "./assistantLinks"; describe("assistantFileTarget", () => { test("accepts normalized relative paths and absolute paths inside the workspace", () => { @@ -12,6 +12,7 @@ describe("assistantFileTarget", () => { "reports/manual report.md", ); expect(assistantFileTarget("/repo/docs/report.md?raw=1", "/repo")).toBe("docs/report.md"); + expect(assistantFileTarget("C:/repo/docs/report.md", "C:/repo")).toBe("docs/report.md"); }); test("rejects external, fragment, outside-workspace, and context-free targets", () => { @@ -31,6 +32,20 @@ describe("assistantFileTarget", () => { }); }); +test("assistant URL transforms preserve only anchor-shaped Windows paths", () => { + const anchor = { tagName: "a" }; + expect(assistantUrlTransform("C:/repo/report.md", "href", anchor)).toBe("C:/repo/report.md"); + expect(assistantUrlTransform("C:%5Crepo%5Creport.md", "href", anchor)).toBe( + "C:%5Crepo%5Creport.md", + ); + expect(assistantUrlTransform("C:/repo/image.png", "src", { tagName: "img" })).toBe(""); + expect(assistantUrlTransform("C:/repo/style.css", "href", { tagName: "link" })).toBe(""); + expect(assistantUrlTransform("javascript:alert(1)", "href", anchor)).toBe(""); + expect(assistantUrlTransform("https://example.com/report", "href", anchor)).toBe( + "https://example.com/report", + ); +}); + test("assistant Markdown distinguishes workspace files from ordinary anchors", () => { const html = renderToStaticMarkup( { + const html = renderToStaticMarkup( + {}} + />, + ); + + expect(html).toContain('data-path="docs/report.md"'); + expect(html).toContain('data-path="docs/backslash.md"'); + expect(html).not.toContain('href="C:/repo/docs/report.md"'); + expect(html).not.toContain('href="C:%5Crepo%5Cdocs%5Cbackslash.md"'); + expect(html).not.toContain('href="D:/other/report.md"'); + expect(html).not.toContain('href="javascript:alert(1)"'); +}); diff --git a/apps/web/src/chat/assistantLinks.tsx b/apps/web/src/chat/assistantLinks.tsx index 7772d91c..a0736d9f 100644 --- a/apps/web/src/chat/assistantLinks.tsx +++ b/apps/web/src/chat/assistantLinks.tsx @@ -1,7 +1,22 @@ import { type ReactNode, useMemo } from "react"; -import type { Components } from "react-markdown"; +import { type Components, defaultUrlTransform } from "react-markdown"; +import { isWindowsAbsolutePath, workspaceFileTarget } from "./fileTargets"; import { Markdown } from "./Markdown"; -import { toolFileTarget } from "./tools/ToolFileLink"; + +export function assistantUrlTransform( + value: string, + property: string, + node: { tagName: string }, +): string { + if (property === "href" && node.tagName === "a") { + try { + if (isWindowsAbsolutePath(decodeURIComponent(value))) return value; + } catch { + return defaultUrlTransform(value); + } + } + return defaultUrlTransform(value); +} export function assistantFileTarget( href: string | undefined, @@ -14,7 +29,7 @@ export function assistantFileTarget( const encodedPath = candidate.split(/[?#]/, 1)[0]; if (!encodedPath) return null; try { - return toolFileTarget(decodeURIComponent(encodedPath), workspaceRoot); + return workspaceFileTarget(decodeURIComponent(encodedPath), workspaceRoot); } catch { return null; } @@ -33,8 +48,9 @@ function AssistantLink({ }) { const target = assistantFileTarget(href, workspaceRoot); if (!target || !onOpenFile) { + const safeHref = href === undefined ? undefined : defaultUrlTransform(href); return ( - + {children} ); @@ -71,5 +87,5 @@ export function AssistantMarkdown({ }), [workspaceRoot, onOpenFile], ); - return ; + return ; } diff --git a/apps/web/src/chat/fileTargets.test.ts b/apps/web/src/chat/fileTargets.test.ts new file mode 100644 index 00000000..9e2211c4 --- /dev/null +++ b/apps/web/src/chat/fileTargets.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from "bun:test"; +import { workspaceFileTarget } from "./fileTargets"; + +describe("workspaceFileTarget", () => { + it("canonicalizes only paths contained by the active worktree", () => { + for (const [path, root, expected] of [ + ["module-a/../README.md", "/repo", "README.md"], + ["/repo/module-a/SPEC.md", "/repo", "module-a/SPEC.md"], + ["C:\\repo\\module-a\\SPEC.md", "C:\\repo", "module-a/SPEC.md"], + ["/repo-other/SPEC.md", "/repo", null], + ["../outside.md", "/repo", null], + ["https://example.com/a.md", "/repo", null], + ["file:///repo/a.md", "/repo", null], + ["", "/repo", null], + ] as const) { + expect(workspaceFileTarget(path, root)).toBe(expected); + } + }); +}); diff --git a/apps/web/src/chat/fileTargets.ts b/apps/web/src/chat/fileTargets.ts new file mode 100644 index 00000000..96bbf7ad --- /dev/null +++ b/apps/web/src/chat/fileTargets.ts @@ -0,0 +1,26 @@ +import { isAbsolutePath, projectRelativePath } from "@/lib"; + +const URI_SCHEME = /^[A-Za-z][A-Za-z0-9+.-]*:/; +const WINDOWS_ABSOLUTE_PATH = /^[A-Za-z]:[\\/]/; + +export function hasUriScheme(value: string): boolean { + return URI_SCHEME.test(value); +} + +export function isWindowsAbsolutePath(value: string): boolean { + return WINDOWS_ABSOLUTE_PATH.test(value); +} + +export function workspaceFileTarget( + path: string, + workspaceRoot?: string | undefined, +): string | null { + const candidate = path.trim(); + if (!candidate) return null; + if (!isAbsolutePath(candidate) && hasUriScheme(candidate)) return null; + const relative = projectRelativePath(candidate, workspaceRoot); + if (!relative || isAbsolutePath(relative) || relative === ".." || relative.startsWith("../")) { + return null; + } + return relative; +} diff --git a/apps/web/src/chat/tools/SPEC.md b/apps/web/src/chat/tools/SPEC.md index 291e3fd9..f0b65a0e 100644 --- a/apps/web/src/chat/tools/SPEC.md +++ b/apps/web/src/chat/tools/SPEC.md @@ -200,8 +200,9 @@ registration runs once when the chat module mounts. Unregistered tools fall back also registered for `get_subagent_result`, routine), the `SubagentCompletionCard` turn card, and the pure `runDetails` readers/formatters; own child spec ([subagent/SPEC.md](subagent/SPEC.md)). - **Shared pieces** — `CodeBlock` (shiki), `Collapsible` ("Show all N lines" fold for long output), - `ToolFileLink` + exact-reference linked text, pure `toolHelpers` (arg readers, `resultText`, - `languageFromPath`) + `lib`'s `projectRelativePath`. `resultText` delegates canonical result parsing to + `ToolFileLink` + exact-reference linked text backed by the parent chat module's shared + `workspaceFileTarget`, pure `toolHelpers` (arg readers, `resultText`, `languageFromPath`) + `lib`'s + `projectRelativePath`. `resultText` delegates canonical result parsing to the parent chat primitive, so text extraction and the common image layer cannot disagree about what constitutes a valid content block. @@ -209,7 +210,8 @@ registration runs once when the chat module mounts. Unregistered tools fall back - **Public surface:** the side-effect `register` import + the shared `CodeBlock`/`Collapsible`/ `toolHelpers` for sibling renderers + `visualize/MermaidView` for the parent `Markdown` primitive. No barrel (chat pulls shiki — per-file imports, as in the parent). -- **Allowed deps:** parent chat primitives (`toolRegistry`, `Markdown`, `ChatActions`, `askState`); +- **Allowed deps:** parent chat primitives (`toolRegistry`, `Markdown`, `ChatActions`, `askState`, + `fileTargets`); `contracts` (type-only + the `ASK_USER_ANSWERS_CUSTOM_TYPE` constant); `components/ui`; `lib`; `@remixicon/react`; `mermaid` (**lazy, `visualize/` only**). - **Forbidden:** value-importing any `pi` package; `store`/`transport` (renderers stay presentational — diff --git a/apps/web/src/chat/tools/ToolFileLink.tsx b/apps/web/src/chat/tools/ToolFileLink.tsx index 2bfba67e..7fe08b87 100644 --- a/apps/web/src/chat/tools/ToolFileLink.tsx +++ b/apps/web/src/chat/tools/ToolFileLink.tsx @@ -1,17 +1,5 @@ import { cn, isAbsolutePath, projectRelativePath } from "@/lib"; - -const URI_SCHEME = /^[A-Za-z][A-Za-z0-9+.-]*:/; - -export function toolFileTarget(path: string, workspaceRoot?: string | undefined): string | null { - const candidate = path.trim(); - if (!candidate) return null; - if (!isAbsolutePath(candidate) && URI_SCHEME.test(candidate)) return null; - const relative = projectRelativePath(candidate, workspaceRoot); - if (!relative || isAbsolutePath(relative) || relative === ".." || relative.startsWith("../")) { - return null; - } - return relative; -} +import { hasUriScheme, workspaceFileTarget } from "../fileTargets"; export function ToolFileLink({ path, @@ -31,10 +19,10 @@ export function ToolFileLink({ const candidate = path.trim(); const displayPath = label ?? - (candidate && !isAbsolutePath(candidate) && URI_SCHEME.test(candidate) + (candidate && !isAbsolutePath(candidate) && hasUriScheme(candidate) ? candidate : projectRelativePath(candidate, workspaceRoot)); - const target = disabled ? null : toolFileTarget(candidate, workspaceRoot); + const target = disabled ? null : workspaceFileTarget(candidate, workspaceRoot); if (!target || !onOpenFile) { return ( { - it("canonicalizes only paths contained by the active worktree", () => { - for (const [path, root, expected] of [ - ["module-a/../README.md", "/repo", "README.md"], - ["/repo/module-a/SPEC.md", "/repo", "module-a/SPEC.md"], - ["C:\\repo\\module-a\\SPEC.md", "C:\\repo", "module-a/SPEC.md"], - ["/repo-other/SPEC.md", "/repo", null], - ["../outside.md", "/repo", null], - ["https://example.com/a.md", "/repo", null], - ["file:///repo/a.md", "/repo", null], - ["", "/repo", null], - ] as const) { - expect(toolFileTarget(path, root)).toBe(expected); - } - }); - it("opens relative and in-worktree absolute read paths under one canonical relative label", () => { const relative = markup(ReadCard, "read", "module-a/../README.md"); const absolute = markup(ReadCard, "read", "/repo/module-a/SPEC.md"); From b8216217fe9b5395aa23ad03d99f757a54c0bcae Mon Sep 17 00:00:00 2001 From: rsolmano Date: Mon, 31 Aug 2026 13:51:17 +0200 Subject: [PATCH 5/6] fix(web): match Windows paths case-insensitively --- apps/web/src/chat/SPEC.md | 3 ++- apps/web/src/chat/assistantLinks.test.tsx | 15 ++++++++------- apps/web/src/lib/SPEC.md | 3 ++- apps/web/src/lib/utils.test.ts | 4 ++++ apps/web/src/lib/utils.ts | 16 +++++++++++++--- 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/apps/web/src/chat/SPEC.md b/apps/web/src/chat/SPEC.md index eeb3d3bc..1cf787ea 100644 --- a/apps/web/src/chat/SPEC.md +++ b/apps/web/src/chat/SPEC.md @@ -86,7 +86,8 @@ blocks in order into rows; `ChatTurnView` dispatches on row kind: new-tab anchor behavior. Percent-encoded file paths are decoded once before validation, so encoded separators and traversal cannot bypass containment. A narrow assistant-only URL transform preserves recognized Windows drive-letter anchor paths, including Markdown's percent-encoded backslash form, until - validation; every other value delegates to react-markdown's default sanitizer, and a rejected Windows path + validation; drive-rooted containment compares case-insensitively while preserving the linked path's casing. + Every other value delegates to react-markdown's default sanitizer, and a rejected Windows path is re-sanitized before fallback anchor rendering. The generic `Markdown` primitive remains props-driven and receives this behavior only as an `a` component override at the assistant-turn integration edge; accepted workspace targets render as button controls without a raw browser `href`, so alternate native diff --git a/apps/web/src/chat/assistantLinks.test.tsx b/apps/web/src/chat/assistantLinks.test.tsx index 261c953a..6b825eea 100644 --- a/apps/web/src/chat/assistantLinks.test.tsx +++ b/apps/web/src/chat/assistantLinks.test.tsx @@ -12,7 +12,7 @@ describe("assistantFileTarget", () => { "reports/manual report.md", ); expect(assistantFileTarget("/repo/docs/report.md?raw=1", "/repo")).toBe("docs/report.md"); - expect(assistantFileTarget("C:/repo/docs/report.md", "C:/repo")).toBe("docs/report.md"); + expect(assistantFileTarget("c:/repo/docs/Report.md", "C:/Repo")).toBe("docs/Report.md"); }); test("rejects external, fragment, outside-workspace, and context-free targets", () => { @@ -71,20 +71,21 @@ test("assistant Markdown opens contained Windows paths without exposing unsafe U const html = renderToStaticMarkup( {}} />, ); - expect(html).toContain('data-path="docs/report.md"'); + expect(html.match(/data-testid="chat-file-link"/g)).toHaveLength(2); + expect(html).toContain('data-path="docs/Report.md"'); expect(html).toContain('data-path="docs/backslash.md"'); - expect(html).not.toContain('href="C:/repo/docs/report.md"'); - expect(html).not.toContain('href="C:%5Crepo%5Cdocs%5Cbackslash.md"'); + expect(html).not.toContain('href="c:/repo/docs/Report.md"'); + expect(html).not.toContain('href="c:%5CREPO%5Cdocs%5Cbackslash.md"'); expect(html).not.toContain('href="D:/other/report.md"'); expect(html).not.toContain('href="javascript:alert(1)"'); }); diff --git a/apps/web/src/lib/SPEC.md b/apps/web/src/lib/SPEC.md index 0ceda011..8182a20f 100644 --- a/apps/web/src/lib/SPEC.md +++ b/apps/web/src/lib/SPEC.md @@ -54,7 +54,8 @@ Tiny UI helpers shared across components. semantic resource key, so delimiters and stable noncanonical placement ids cannot split or alias identities. - **Public surface (barrel):** `cn`, `isMarkdownPath`, `stripFrontmatter`, `cssColorToHex`, `normalizePath`, `isAbsolutePath`, `projectRelativePath` (canonical worktree-relative POSIX identity; - collapses in-root `.`/`..` aliases but preserves an attempted leading escape for host rejection), + collapses in-root `.`/`..` aliases but preserves an attempted leading escape for host rejection; Windows + drive-rooted containment compares path/root case-insensitively while preserving the candidate's casing), `shallowEqualArrays`, `userText`, `parseSkillInvocation`, `matchesSkillInvocationCommand`, `relativeTime`, `platformShortcutLabel`, `hasPlatformModifier`, `copyText`, `randomId`, `DOUBLE_CLICK_SETTLE_MS`, `tupleKey`, `parseTupleKey`, `layoutResourceIdentity`, diff --git a/apps/web/src/lib/utils.test.ts b/apps/web/src/lib/utils.test.ts index c27ce6ad..fdd32303 100644 --- a/apps/web/src/lib/utils.test.ts +++ b/apps/web/src/lib/utils.test.ts @@ -121,6 +121,10 @@ test("projectRelativePath yields the worktree-relative tab identity from every r expect(projectRelativePath("src/./nested/../foo.ts", root)).toBe("src/foo.ts"); expect(projectRelativePath("../outside.ts", root)).toBe("../outside.ts"); expect(projectRelativePath("C:\\wt\\ws\\src\\..\\foo.ts", "C:\\wt\\ws")).toBe("foo.ts"); + expect(projectRelativePath("c:/WT/ws/Src/Foo.ts", "C:/wt/WS")).toBe("Src/Foo.ts"); + expect(projectRelativePath("C:/Repository/foo.ts", "c:/repo")).toBe("C:/Repository/foo.ts"); + expect(projectRelativePath("D:/Repo/foo.ts", "c:/repo")).toBe("D:/Repo/foo.ts"); + expect(projectRelativePath("/WT/ws/src/foo.ts", root)).toBe("/WT/ws/src/foo.ts"); expect(projectRelativePath("/src/foo.ts", "/")).toBe("src/foo.ts"); expect(projectRelativePath("C:/src/foo.ts", "C:/")).toBe("src/foo.ts"); expect(projectRelativePath("/elsewhere/foo.ts", root)).toBe("/elsewhere/foo.ts"); diff --git a/apps/web/src/lib/utils.ts b/apps/web/src/lib/utils.ts index 45aa93b0..b8ea21f1 100644 --- a/apps/web/src/lib/utils.ts +++ b/apps/web/src/lib/utils.ts @@ -94,9 +94,13 @@ export function normalizePath(path: string): string { return path.replaceAll("\\", "/").replace(/^\.\/+/, ""); } +function hasWindowsDriveRoot(path: string): boolean { + return /^[A-Za-z]:\//.test(normalizePath(path)); +} + export function isAbsolutePath(path: string): boolean { const normalized = normalizePath(path); - return normalized.startsWith("/") || /^[A-Za-z]:\//.test(normalized); + return normalized.startsWith("/") || hasWindowsDriveRoot(normalized); } export function shallowEqualArrays( @@ -142,8 +146,14 @@ export function projectRelativePath(path: string, workspaceRoot?: string | undef if (!canonical || !isAbsolutePath(canonical)) return canonical; const root = workspaceRoot ? trimTrailingSlashes(canonicalPosixPath(workspaceRoot)) : ""; - const rootPrefix = root.endsWith("/") ? root : `${root}/`; - if (root && (canonical === root || canonical.startsWith(rootPrefix))) { + const ignoreCase = hasWindowsDriveRoot(canonical) && hasWindowsDriveRoot(root); + const comparableCanonical = ignoreCase ? canonical.toLowerCase() : canonical; + const comparableRoot = ignoreCase ? root.toLowerCase() : root; + const rootPrefix = comparableRoot.endsWith("/") ? comparableRoot : `${comparableRoot}/`; + if ( + comparableRoot && + (comparableCanonical === comparableRoot || comparableCanonical.startsWith(rootPrefix)) + ) { return canonical.slice(root.length).replace(/^\/+/, "") || fileName(canonical); } From e9ae7da3948c7424c96ed6aaaa8423b8c05dac58 Mon Sep 17 00:00:00 2001 From: rsolmano Date: Tue, 1 Sep 2026 23:27:22 +0200 Subject: [PATCH 6/6] todo: Adjust tests to current behavior ThinkRail-Todo: 01a05edd-0611-7188-92a5-dee3853ba158/t_00250c42399e --- e2e/tool-file-links.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/tool-file-links.spec.ts b/e2e/tool-file-links.spec.ts index 0a15232a..3c18f0d5 100644 --- a/e2e/tool-file-links.spec.ts +++ b/e2e/tool-file-links.spec.ts @@ -192,7 +192,7 @@ test("assistant Markdown opens safe relative files without navigating the browse await expect(defaultWorkspaceRow(page)).toBeVisible(); await enterDefaultWorkspace(page); - await openChatFromHistory(page, "assistant file links"); + await expect(page.locator('[data-testid="editor-tab"][data-kind="chat"]')).toHaveCount(1); const message = page.locator('[data-testid="chat-message"][data-role="assistant"]').last(); const local = message.getByRole("button", { name: "Open README" });