From 29b26b9cfc41a9715d48159215e17df28b3dbe5a Mon Sep 17 00:00:00 2001 From: Aditya Garud Date: Sun, 2 Aug 2026 07:31:16 +0000 Subject: [PATCH 1/2] fix(web): recognize markdown file links with angle brackets and parentheses The file-link classifier scanned message text with a single regex that captured fewer CommonMark link destination forms than react-markdown renders. Angle-bracket destinations ([x]()) captured nothing and bare destinations with balanced parentheses ([x](/tmp/a(1).txt)) were truncated at the first ')', so those links rendered as plain anchors that did nothing when clicked. Replace the regex with extractMarkdownLinkHrefs, a small CommonMark-aware scanner that understands both destination forms, and add normalizeMarkdownLinkHrefKey which decodes percent-encoding so a rendered href (spaces become %20) matches the unencoded destination scanned from the source text. File URIs keep their single-decode semantics. Both helpers move to markdown-links.ts with focused unit tests. Fixes pingdotgg/t3code#5158 --- apps/web/src/components/ChatMarkdown.tsx | 21 +--- apps/web/src/markdown-links.test.ts | 57 +++++++++++ apps/web/src/markdown-links.ts | 117 +++++++++++++++++++++++ 3 files changed, 177 insertions(+), 18 deletions(-) diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index fac3bf7d245..e5fbf226c4a 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -66,7 +66,8 @@ import { } from "../markdown-clipboard"; import { remarkNormalizeListItemIndentation } from "../markdown-list-indentation"; import { - normalizeMarkdownLinkDestination, + extractMarkdownLinkHrefs, + normalizeMarkdownLinkHrefKey, resolveMarkdownFileLinkMeta, rewriteMarkdownFileUriHref, } from "../markdown-links"; @@ -752,7 +753,6 @@ interface MarkdownFileLinkProps { className?: string | undefined; } -const MARKDOWN_LINK_HREF_PATTERN = /\[[^\]]*]\(([^)\s]+)(?:\s+["'][^"']*["'])?\)/g; const MARKDOWN_FILE_LINK_CLASS_NAME = "chat-markdown-file-link cursor-pointer transition-colors hover:bg-accent/70"; @@ -816,21 +816,6 @@ function buildFileLinkParentSuffixByPath(filePaths: ReadonlyArray): Map< return suffixByPath; } -function extractMarkdownLinkHrefs(text: string): string[] { - const hrefs: string[] = []; - for (const match of text.matchAll(MARKDOWN_LINK_HREF_PATTERN)) { - const href = match[1]?.trim(); - if (!href) continue; - hrefs.push(href); - } - return hrefs; -} - -function normalizeMarkdownLinkHrefKey(href: string): string { - const normalizedHref = normalizeMarkdownLinkDestination(href); - return rewriteMarkdownFileUriHref(normalizedHref) ?? normalizedHref; -} - const MARKDOWN_LINK_FAVICON_CLASS_NAME = "block size-full shrink-0 select-none"; /** Hosts whose favicon request already failed this session — skip straight to the globe. */ @@ -1475,7 +1460,7 @@ function ChatMarkdown({ workspaceRelativePath={fileLinkMeta.workspaceRelativePath} line={fileLinkMeta.line} label={labelParts.join(" · ")} - copyMarkdown={`[${fileLinkMeta.basename}](${normalizedHref})`} + copyMarkdown={`[${fileLinkMeta.basename}](${href ?? normalizedHref})`} theme={resolvedTheme} threadRef={threadRef} onOpen={openInPreferredEditor} diff --git a/apps/web/src/markdown-links.test.ts b/apps/web/src/markdown-links.test.ts index 5691ffa8895..2cb9402bc63 100644 --- a/apps/web/src/markdown-links.test.ts +++ b/apps/web/src/markdown-links.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it } from "vite-plus/test"; import { + extractMarkdownLinkHrefs, + normalizeMarkdownLinkHrefKey, resolveMarkdownFileLinkMeta, resolveMarkdownFileLinkTarget, rewriteMarkdownFileUriHref, @@ -34,6 +36,61 @@ describe("rewriteMarkdownFileUriHref", () => { }); }); +describe("extractMarkdownLinkHrefs", () => { + it("extracts bare destinations", () => { + expect(extractMarkdownLinkHrefs("[control](/tmp/link%20repro/manifest.tsv)")).toEqual([ + "/tmp/link%20repro/manifest.tsv", + ]); + }); + + it("extracts angle-bracket destinations that contain spaces", () => { + expect(extractMarkdownLinkHrefs("[angle]()")).toEqual([ + "/tmp/link repro/manifest.tsv", + ]); + }); + + it("keeps balanced parentheses inside bare destinations", () => { + expect(extractMarkdownLinkHrefs("[parens](/tmp/a(1).txt)")).toEqual(["/tmp/a(1).txt"]); + }); + + it("ignores a trailing title after the destination", () => { + expect(extractMarkdownLinkHrefs('[t](/tmp/a.txt "title")')).toEqual(["/tmp/a.txt"]); + }); + + it("extracts every link in a multi-line message", () => { + const text = [ + "[angle]()", + "[parens](/tmp/a(1).txt)", + "[control](/tmp/link%20repro/manifest.tsv)", + ].join("\n\n"); + expect(extractMarkdownLinkHrefs(text)).toEqual([ + "/tmp/link repro/manifest.tsv", + "/tmp/a(1).txt", + "/tmp/link%20repro/manifest.tsv", + ]); + }); +}); + +describe("normalizeMarkdownLinkHrefKey", () => { + it("matches a percent-encoded href to its unencoded source destination", () => { + // The angle-bracket source keeps a literal space; react-markdown renders it + // as %20. Both must normalize to the same key so the file link is detected. + expect(normalizeMarkdownLinkHrefKey("/tmp/link repro/manifest.tsv")).toBe( + normalizeMarkdownLinkHrefKey("/tmp/link%20repro/manifest.tsv"), + ); + }); + + it("is stable for bare destinations with balanced parentheses", () => { + expect(normalizeMarkdownLinkHrefKey("/tmp/a(1).txt")).toBe("/tmp/a(1).txt"); + }); + + it("preserves single-decode semantics for file URIs", () => { + expect(normalizeMarkdownLinkHrefKey("file:///Users/julius/project/file%2520name.md")).toBe( + "/Users/julius/project/file%2520name.md", + ); + }); +}); + describe("resolveMarkdownFileLinkTarget", () => { it("resolves absolute posix file paths", () => { expect(resolveMarkdownFileLinkTarget("/Users/julius/project/AGENTS.md")).toBe( diff --git a/apps/web/src/markdown-links.ts b/apps/web/src/markdown-links.ts index 1e24de8bb1d..3cf4bfe7df9 100644 --- a/apps/web/src/markdown-links.ts +++ b/apps/web/src/markdown-links.ts @@ -92,6 +92,123 @@ export function rewriteMarkdownFileUriHref(href: string | undefined): string | n return `${target.path}${target.hash}`; } +/** + * Extract the destination of every markdown inline link found in `text`. + * + * This mirrors the CommonMark link-destination grammar closely enough for file + * link classification: it understands both the angle-bracket form + * (`[label]()`) and the bare form, where the destination may + * contain balanced parentheses (`[label](/tmp/a(1).txt)`). The previous + * single-regex implementation captured neither form, so links whose paths held + * spaces or parentheses were never recognized as file links. + */ +export function extractMarkdownLinkHrefs(text: string): string[] { + const hrefs: string[] = []; + const length = text.length; + let index = 0; + + while (index < length) { + const labelStart = text.indexOf("[", index); + if (labelStart === -1) break; + + const labelEnd = text.indexOf("]", labelStart + 1); + if (labelEnd === -1) break; + + if (text[labelEnd + 1] !== "(") { + index = labelEnd + 1; + continue; + } + + let pos = labelEnd + 2; + while (pos < length && (text[pos] === " " || text[pos] === "\t")) { + pos += 1; + } + + let destination = ""; + + if (text[pos] === "<") { + // Angle-bracket destination: may contain spaces, ends at an unescaped ">". + pos += 1; + let closed = false; + while (pos < length) { + const char = text[pos]; + if (char === undefined) break; + if (char === "\\" && pos + 1 < length) { + destination += text[pos + 1]; + pos += 2; + continue; + } + if (char === "\n") break; + if (char === ">") { + closed = true; + pos += 1; + break; + } + destination += char; + pos += 1; + } + if (!closed) { + index = labelEnd + 2; + continue; + } + } else { + // Bare destination: balanced parentheses are part of the destination; it + // ends at whitespace, a control character, or an unbalanced ")". + let depth = 0; + while (pos < length) { + const char = text[pos]; + if (char === undefined) break; + if (char === "\\" && pos + 1 < length) { + destination += text[pos + 1]; + pos += 2; + continue; + } + if (char === " " || char === "\t" || char === "\n" || char.charCodeAt(0) < 0x20) { + break; + } + if (char === "(") { + depth += 1; + destination += char; + pos += 1; + continue; + } + if (char === ")") { + if (depth === 0) break; + depth -= 1; + destination += char; + pos += 1; + continue; + } + destination += char; + pos += 1; + } + } + + const href = destination.trim(); + if (href.length > 0) hrefs.push(href); + index = Math.max(pos, labelEnd + 2); + } + + return hrefs; +} + +/** + * Canonical lookup key for a markdown link destination. + * + * react-markdown percent-encodes some destination characters when it renders a + * link (a literal space becomes `%20`), while the raw text the link was authored + * from may contain the unencoded character. Decoding both the rendered href and + * the destination scanned out of the source text lets the file-link classifier + * match the two. File URIs keep their single-decode semantics so paths that + * embed literally percent-encoded octets are not decoded twice. + */ +export function normalizeMarkdownLinkHrefKey(href: string): string { + const normalizedHref = normalizeMarkdownLinkDestination(href); + const rewritten = rewriteMarkdownFileUriHref(normalizedHref); + if (rewritten !== null) return rewritten; + return safeDecode(normalizedHref); +} + function looksLikePosixFilesystemPath(path: string): boolean { if (!path.startsWith("/")) return false; if (POSIX_FILE_ROOT_PREFIXES.some((prefix) => path.startsWith(prefix))) return true; From c9550c9879549915a581ce78fb2c97f88202ebaa Mon Sep 17 00:00:00 2001 From: Aditya Garud Date: Sun, 2 Aug 2026 08:34:18 +0000 Subject: [PATCH 2/2] fix(web): harden markdown link extraction from review feedback Address review findings on the file-link scanner: - Resolve the link label end across escaped (\]) and balanced nested ([foo [bar]]) brackets instead of stopping at the first ], so those CommonMark links are classified correctly. - Skip the optional link title and closing paren after the destination so a [label](url) sequence embedded inside a title is no longer extracted as a real link. - Decode file:// keys consistently in normalizeMarkdownLinkHrefKey so the pre-scan key (from the raw file:// href) matches the render-time key (from the already-rewritten path). The file path is now resolved from the original href to keep single-decode semantics and avoid double decoding percent-encoded octets. Extend markdown-links tests to cover escaped/nested labels, titles that contain link-looking text, and file:// key matching. --- apps/web/src/components/ChatMarkdown.tsx | 10 ++- apps/web/src/markdown-links.test.ts | 25 ++++++- apps/web/src/markdown-links.ts | 95 ++++++++++++++++++++---- 3 files changed, 111 insertions(+), 19 deletions(-) diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 58aacdaecd5..f4f3d8fe389 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -1267,11 +1267,13 @@ function ChatMarkdown({ NonNullable> >(); for (const href of extractMarkdownLinkHrefs(text)) { - const normalizedHref = normalizeMarkdownLinkHrefKey(href); - if (metaByHref.has(normalizedHref)) continue; - const meta = resolveMarkdownFileLinkMeta(normalizedHref, cwd); + const key = normalizeMarkdownLinkHrefKey(href); + if (metaByHref.has(key)) continue; + // Resolve the path from the original href (single decode); the decoded key + // is only used to match the render-time anchor href. + const meta = resolveMarkdownFileLinkMeta(href, cwd); if (meta) { - metaByHref.set(normalizedHref, meta); + metaByHref.set(key, meta); } } return metaByHref; diff --git a/apps/web/src/markdown-links.test.ts b/apps/web/src/markdown-links.test.ts index dda7d7d6d0e..e45d97cb93a 100644 --- a/apps/web/src/markdown-links.test.ts +++ b/apps/web/src/markdown-links.test.ts @@ -58,6 +58,20 @@ describe("extractMarkdownLinkHrefs", () => { expect(extractMarkdownLinkHrefs('[t](/tmp/a.txt "title")')).toEqual(["/tmp/a.txt"]); }); + it("finds the label end across escaped brackets", () => { + expect(extractMarkdownLinkHrefs("[foo \\] bar](/tmp/a.txt)")).toEqual(["/tmp/a.txt"]); + }); + + it("finds the label end across balanced nested brackets", () => { + expect(extractMarkdownLinkHrefs("[foo [bar]](/tmp/a.txt)")).toEqual(["/tmp/a.txt"]); + }); + + it("does not extract link-looking sequences inside a title", () => { + expect(extractMarkdownLinkHrefs('[t](/tmp/a.txt "see [x](/tmp/should-not-match.txt)")')).toEqual( + ["/tmp/a.txt"], + ); + }); + it("extracts every link in a multi-line message", () => { const text = [ "[angle]()", @@ -85,10 +99,17 @@ describe("normalizeMarkdownLinkHrefKey", () => { expect(normalizeMarkdownLinkHrefKey("/tmp/a(1).txt")).toBe("/tmp/a(1).txt"); }); - it("preserves single-decode semantics for file URIs", () => { - expect(normalizeMarkdownLinkHrefKey("file:///Users/julius/project/file%2520name.md")).toBe( + it("matches a file URI key to its rewritten-path render form", () => { + // Pre-scan sees the raw file:// href; after markdownUrlTransform the anchor + // href is the rewritten path. Both must produce the same lookup key. + const fromFileUri = normalizeMarkdownLinkHrefKey( + "file:///Users/julius/project/file%2520name.md", + ); + const fromRewrittenPath = normalizeMarkdownLinkHrefKey( "/Users/julius/project/file%2520name.md", ); + expect(fromFileUri).toBe(fromRewrittenPath); + expect(fromFileUri).toBe("/Users/julius/project/file%20name.md"); }); }); diff --git a/apps/web/src/markdown-links.ts b/apps/web/src/markdown-links.ts index e4a8f617667..f6dc9fa293d 100644 --- a/apps/web/src/markdown-links.ts +++ b/apps/web/src/markdown-links.ts @@ -108,15 +108,83 @@ export function rewriteMarkdownFileUriHref(href: string | undefined): string | n return `${target.path}${target.hash}`; } +/** + * Find the `]` that closes the link label starting at `labelStart` (`[`). + * + * Tracks bracket depth and honors backslash escapes so escaped brackets (`\]`, + * `\[`) and balanced nested runs (`[foo [bar]]`) are handled the way CommonMark + * does, instead of stopping at the first `]`. Returns -1 when no matching close + * bracket exists. + */ +function findMarkdownLinkLabelEnd(text: string, labelStart: number): number { + let depth = 0; + for (let index = labelStart; index < text.length; index += 1) { + const char = text[index]; + if (char === "\\") { + index += 1; + continue; + } + if (char === "[") { + depth += 1; + continue; + } + if (char === "]") { + depth -= 1; + if (depth === 0) return index; + } + } + return -1; +} + +/** + * Advance past an inline link's optional title and its closing `)`, starting + * from the character after the destination. Returns the index just after the + * closing parenthesis so a `[label](url)` sequence embedded in a title is not + * mistaken for a real link on the next scan iteration. + */ +function skipMarkdownLinkTitleAndClose(text: string, start: number): number { + const length = text.length; + const isSpace = (char: string | undefined): boolean => + char === " " || char === "\t" || char === "\n" || char === "\r"; + + let pos = start; + while (pos < length && isSpace(text[pos])) pos += 1; + + const opener = text[pos]; + if (opener === '"' || opener === "'" || opener === "(") { + const closer = opener === "(" ? ")" : opener; + pos += 1; + while (pos < length) { + const char = text[pos]; + if (char === undefined) break; + if (char === "\\" && pos + 1 < length) { + pos += 2; + continue; + } + if (char === closer) { + pos += 1; + break; + } + pos += 1; + } + while (pos < length && isSpace(text[pos])) pos += 1; + } + + if (text[pos] === ")") pos += 1; + return pos; +} + /** * Extract the destination of every markdown inline link found in `text`. * - * This mirrors the CommonMark link-destination grammar closely enough for file - * link classification: it understands both the angle-bracket form - * (`[label]()`) and the bare form, where the destination may - * contain balanced parentheses (`[label](/tmp/a(1).txt)`). The previous - * single-regex implementation captured neither form, so links whose paths held - * spaces or parentheses were never recognized as file links. + * This mirrors the CommonMark link grammar closely enough for file link + * classification: it resolves the label end across escaped/nested brackets, and + * understands both the angle-bracket destination form (`[label]()`) and the bare form, where the destination may contain balanced + * parentheses (`[label](/tmp/a(1).txt)`). It also skips the optional title so a + * `[..](..)` sequence inside a title is not extracted. The previous single-regex + * implementation captured none of this, so links whose paths held spaces or + * parentheses were never recognized as file links. */ export function extractMarkdownLinkHrefs(text: string): string[] { const hrefs: string[] = []; @@ -127,7 +195,7 @@ export function extractMarkdownLinkHrefs(text: string): string[] { const labelStart = text.indexOf("[", index); if (labelStart === -1) break; - const labelEnd = text.indexOf("]", labelStart + 1); + const labelEnd = findMarkdownLinkLabelEnd(text, labelStart); if (labelEnd === -1) break; if (text[labelEnd + 1] !== "(") { @@ -202,7 +270,7 @@ export function extractMarkdownLinkHrefs(text: string): string[] { const href = destination.trim(); if (href.length > 0) hrefs.push(href); - index = Math.max(pos, labelEnd + 2); + index = Math.max(skipMarkdownLinkTitleAndClose(text, pos), labelEnd + 2); } return hrefs; @@ -215,14 +283,15 @@ export function extractMarkdownLinkHrefs(text: string): string[] { * link (a literal space becomes `%20`), while the raw text the link was authored * from may contain the unencoded character. Decoding both the rendered href and * the destination scanned out of the source text lets the file-link classifier - * match the two. File URIs keep their single-decode semantics so paths that - * embed literally percent-encoded octets are not decoded twice. + * match the two. `file://` destinations are decoded the same way after rewriting + * so the pre-scan key (built from the raw `file://` href) agrees with the + * render-time key (built from the already-rewritten path `markdownUrlTransform` + * hands to the anchor). This key is only for map lookup; the file path itself is + * resolved separately from the original href to avoid double-decoding. */ export function normalizeMarkdownLinkHrefKey(href: string): string { const normalizedHref = normalizeMarkdownLinkDestination(href); - const rewritten = rewriteMarkdownFileUriHref(normalizedHref); - if (rewritten !== null) return rewritten; - return safeDecode(normalizedHref); + return safeDecode(rewriteMarkdownFileUriHref(normalizedHref) ?? normalizedHref); } function looksLikePosixFilesystemPath(path: string): boolean {