diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 0b0116be549cb..ee9c7f5ecc7d0 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -248,6 +248,9 @@ interface ChatMarkdownProps { /** Loads media a source control host keeps behind a credential through the server, which holds `cwd`'s; without it a private repository's uploads load unauthenticated and 404. */ sourceControlMedia?: boolean | undefined; + /** Text written on another machine (a pull request) names paths there; only files inside `cwd` + become file links, and other path links render as their label. */ + workspaceFileLinksOnly?: boolean | undefined; /** The change request's repository, when it can differ from the checkout at `cwd`. */ sourceControlMediaRepository?: string | undefined; headingLevelOffset?: number | undefined; @@ -445,35 +448,51 @@ const markdownServerConfigFieldsAtom = Atom.family((environmentId: EnvironmentId }).pipe(Atom.withLabel("markdown-server-config-fields")), ); -function fileLinkSourcesKey( - text: string, - cwd: string | undefined, - imageBaseDir: string | undefined, -): string { +interface FileLinkScope { + cwd: string | undefined; + imageBaseDir: string | undefined; + workspaceFileLinksOnly: boolean; +} + +function keepFileLink(meta: MarkdownFileLinkMeta | null, scope: FileLinkScope) { + return meta && (!scope.workspaceFileLinksOnly || meta.workspaceRelativePath !== null) + ? meta + : null; +} + +function markdownFileLinkMeta(href: string, scope: FileLinkScope) { + return keepFileLink( + resolveMarkdownFileLinkMeta(href, scope.cwd, scope.imageBaseDir ?? scope.cwd), + scope, + ); +} + +function inlineCodeFileLinkMeta(codeText: string, scope: FileLinkScope) { + return keepFileLink( + resolveInlineCodeFileLinkMeta(codeText, scope.cwd, scope.imageBaseDir ?? scope.cwd), + scope, + ); +} + +function fileLinkSourcesKey(text: string, scope: FileLinkScope): string { const hrefs = extractMarkdownLinkHrefs(renderCodexInlineDirectivesAsMarkdown(text)) .filter((href) => !parseComposerContextHref(href)) .map(normalizeMarkdownLinkHrefKey) - .filter((href) => resolveMarkdownFileLinkMeta(href, cwd, imageBaseDir ?? cwd)); - const spans = extractInlineCodeSpans(text).filter((span) => - resolveInlineCodeFileLinkMeta(span, cwd, imageBaseDir ?? cwd), - ); + .filter((href) => markdownFileLinkMeta(href, scope)); + const spans = extractInlineCodeSpans(text).filter((span) => inlineCodeFileLinkMeta(span, scope)); return JSON.stringify([[...new Set(hrefs)], [...new Set(spans)]]); } -function resolveFileLinkSources( - key: string, - cwd: string | undefined, - imageBaseDir: string | undefined, -) { +function resolveFileLinkSources(key: string, scope: FileLinkScope) { const [hrefs, spans] = JSON.parse(key) as [string[], string[]]; const markdownFileLinkMetaByHref = new Map(); for (const href of hrefs) { - const meta = resolveMarkdownFileLinkMeta(href, cwd, imageBaseDir ?? cwd); + const meta = markdownFileLinkMeta(href, scope); if (meta) markdownFileLinkMetaByHref.set(href, meta); } const inlineCodeFileLinkMetaByText = new Map(); for (const span of spans) { - const meta = resolveInlineCodeFileLinkMeta(span, cwd, imageBaseDir ?? cwd); + const meta = inlineCodeFileLinkMeta(span, scope); if (meta) inlineCodeFileLinkMetaByText.set(span, meta); } const fileLinkParentSuffixByPath = buildFileLinkParentSuffixByPath([ @@ -501,6 +520,7 @@ function useChatMarkdownState({ renderContextReference, headingLevelOffset = 0, sourceControlMedia = false, + workspaceFileLinksOnly = false, sourceControlMediaRepository, }: ChatMarkdownProps) { const resolvedTheme = useRenderedAppearance(); @@ -679,9 +699,10 @@ function useChatMarkdownState({ // Keyed by the file links found, not the text, so streamed text that adds none keeps the // renderer context, and every block reading it, unchanged. // A template literal marks the key as a string, so the compiler memoizes the maps on its value. - const fileLinksKey = `${fileLinkSourcesKey(text, cwd, imageBaseDir)}`; + const fileLinkScope = { cwd, imageBaseDir, workspaceFileLinksOnly }; + const fileLinksKey = `${fileLinkSourcesKey(text, fileLinkScope)}`; const { markdownFileLinkMetaByHref, inlineCodeFileLinkMetaByText, fileLinkParentSuffixByPath } = - resolveFileLinkSources(fileLinksKey, cwd, imageBaseDir); + resolveFileLinkSources(fileLinksKey, fileLinkScope); const resolveThreadPullRequest = ( href: string, ): (ThreadPullRequestKey & { readonly url: string }) | null => { @@ -908,6 +929,7 @@ function useChatMarkdownState({ environmentId, expandMedia, fileLinkChip, + fileLinkScope, sourceControlMedia, sourceControlMediaRepository, renderContextReference, @@ -1092,6 +1114,7 @@ const CHAT_MARKDOWN_COMPONENTS = { pullRequestsEnabled, updateThreadPullRequestLink, fileLinkChip, + fileLinkScope, renderContextReference, } = use(ChatMarkdownRendererContext); const text = use(ChatMarkdownTextContext); @@ -1122,6 +1145,8 @@ const CHAT_MARKDOWN_COMPONENTS = { ? (markdownFileLinkMetaByHref.get(normalizedHref) ?? resolveMarkdownFileLinkMeta(normalizedHref, cwd, imageBaseDir ?? cwd)) : null; + // As a web link a local path would still open on this machine, so it keeps only its label. + if (fileLinkMeta && !keepFileLink(fileLinkMeta, fileLinkScope)) return {children}; if (!fileLinkMeta) { const faviconHost = resolveExternalWebLinkHost(href); const hasText = hastHasText(node); @@ -1347,8 +1372,9 @@ const CHAT_MARKDOWN_COMPONENTS = { ); }, code: function MarkdownCode({ node, children, className, ...props }) { - const { cwd, imageBaseDir, inlineCodeFileLinkMetaByText, fileLinkChip, mathEnabled, inline } = - use(ChatMarkdownRendererContext); + const { fileLinkScope, inlineCodeFileLinkMetaByText, fileLinkChip, mathEnabled, inline } = use( + ChatMarkdownRendererContext, + ); const math = mathEnabled ? mathKind(className) : null; if (math) { return ( @@ -1363,7 +1389,7 @@ const CHAT_MARKDOWN_COMPONENTS = { const codeText = nodeToPlainText(children); const fileLinkMeta = inlineCodeFileLinkMetaByText.get(codeText.trim()) ?? - resolveInlineCodeFileLinkMeta(codeText, cwd, imageBaseDir ?? cwd); + inlineCodeFileLinkMeta(codeText, fileLinkScope); if (fileLinkMeta) { return fileLinkChip( fileLinkMeta, @@ -1395,6 +1421,7 @@ const CHAT_MARKDOWN_COMPONENTS = { workspaceMutationId, renderContextReference, fileLinkChip, + fileLinkScope, resolvedTheme, } = use(ChatMarkdownRendererContext); const imageExpand = use(MarkdownLinkContext) ? undefined : expandMedia; @@ -1455,9 +1482,12 @@ const CHAT_MARKDOWN_COMPONENTS = { /> ); } - if (mediaKind === null && imageSource._tag === "WorkspaceFile") { + if (mediaKind === null && imageSource._tag !== "Direct") { const fileLinkMeta = resolveMarkdownFileLinkMeta(srcString, cwd, imageBaseDir ?? cwd); - if (fileLinkMeta) return fileLinkChip(fileLinkMeta, copyMarkdown, srcString); + if (fileLinkMeta && !keepFileLink(fileLinkMeta, fileLinkScope)) return {altText}; + if (fileLinkMeta && imageSource._tag === "WorkspaceFile") { + return fileLinkChip(fileLinkMeta, copyMarkdown, srcString); + } } const kind = mediaKind ?? "image"; const directUri = imageSource._tag === "Direct" ? imageSource.uri : null; diff --git a/apps/web/src/components/pullRequest/PullRequestMarkdown.browser.test.tsx b/apps/web/src/components/pullRequest/PullRequestMarkdown.browser.test.tsx index fae5270d27123..ab5ae121f728e 100644 --- a/apps/web/src/components/pullRequest/PullRequestMarkdown.browser.test.tsx +++ b/apps/web/src/components/pullRequest/PullRequestMarkdown.browser.test.tsx @@ -20,7 +20,7 @@ function setup() { async function renderBody( host: string, - options: { lineBreaks?: boolean; repository?: string; text?: string } = {}, + options: { lineBreaks?: boolean; repository?: string; text?: string; cwd?: string } = {}, ) { const { repository = null, text = "first line\nsecond line", ...props } = options; const context = { @@ -62,6 +62,38 @@ it("lets text the host did not write opt out of the host's breaks", async () => expect(screen.container.querySelectorAll("p br")).toHaveLength(0); }); +it("chips only inline code paths inside the repository", async () => { + await using screen = await renderBody("github", { + cwd: "/home/alice/repo", + text: "Typing `~/Dév` hides it under `~/`. See `/etc/hosts`, `../outside.ts` and `packages/x/y.ts`.", + }); + await expect + .element(screen.getByRole("button", { name: "File options for y.ts" })) + .toBeInTheDocument(); + const chips = [...screen.container.querySelectorAll(".chat-markdown-file-link")]; + expect(chips.map((chip) => chip.getAttribute("data-markdown-copy"))).toEqual([ + "`packages/x/y.ts`", + ]); + const code = [...screen.container.querySelectorAll("code")].map((c) => c.textContent); + expect(code).toEqual(["~/Dév", "~/", "/etc/hosts", "../outside.ts"]); +}); + +it("chips only links and images to files inside the repository", async () => { + await using screen = await renderBody("github", { + cwd: "/home/alice/repo", + text: "See [hosts](/etc/hosts), [key](~/.ssh/id_rsa), [up](file:///etc/hosts), ![rc](/home/alice/.bashrc), ![profile](~/.profile) and [y](packages/x/y.ts).", + }); + await expect + .element(screen.getByRole("button", { name: "File options for y.ts" })) + .toBeInTheDocument(); + const chips = [...screen.container.querySelectorAll(".chat-markdown-file-link")]; + expect(chips.map((chip) => chip.getAttribute("data-markdown-copy"))).toEqual([ + "[y](packages/x/y.ts)", + ]); + expect(screen.container.querySelectorAll("a")).toHaveLength(0); + expect(screen.container.textContent).toContain("See hosts, key, up, rc, profile and"); +}); + it("loads the body's host media in the pull request's repository, not the checkout's", async () => { setup(); const image = "https://github.com/user-attachments/assets/7195f963-51a9-4331-be74-3e06be760422"; diff --git a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx index 449e901d0f0d3..99233a3df3d8a 100644 --- a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx +++ b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx @@ -125,6 +125,7 @@ function PullRequestBodySegmentView({ lineBreaks={lineBreaks} sourceControlMedia sourceControlMediaRepository={repository} + workspaceFileLinksOnly onTaskListChange={ onTaskListChange && segment.kind === "markdown" ? ({ markerOffset, checked }) =>