From 4210854a9f1ca25cf3b262ce9708c64eac4b5f40 Mon Sep 17 00:00:00 2001 From: haotianqi666 Date: Wed, 14 Oct 2026 08:59:53 +0800 Subject: [PATCH] fix(frontend): resolve local images in Markdown artifact previews (#6827) * fix(frontend): resolve local images in Markdown artifact previews * fix(frontend): key artifact images by resolver context --------- Co-authored-by: haotianqi666 <250384991+haotianqi666@users.noreply.github.com> Co-authored-by: Willem Jiang --- README.md | 5 + frontend/src/AGENTS.md | 6 +- .../artifacts/artifact-file-detail.tsx | 1 + .../artifacts/artifact-file-preview.tsx | 26 +++- .../workspace/artifacts/artifact-viewer.tsx | 1 + frontend/src/core/artifacts/utils.ts | 48 ++++++ .../e2e/artifact-markdown-images.spec.ts | 125 +++++++++++++++ .../artifact-markdown-images.dom.test.tsx | 145 ++++++++++++++++++ .../artifacts/artifact-viewer.dom.test.tsx | 32 ++++ .../tests/unit/core/artifacts/utils.test.ts | 99 ++++++++++++ 10 files changed, 483 insertions(+), 5 deletions(-) create mode 100644 frontend/tests/e2e/artifact-markdown-images.spec.ts create mode 100644 frontend/tests/unit/components/workspace/artifacts/artifact-markdown-images.dom.test.tsx diff --git a/README.md b/README.md index d8328f1fed..0645c092a8 100644 --- a/README.md +++ b/README.md @@ -2390,6 +2390,11 @@ CSV and TSV artifacts open as tables in the artifact panel and in a separate win HTML artifact previews resolve relative assets from the artifact directory unless the document contains a real `` element. Base-tag examples in comments or script text do not change that resolution. +Markdown report previews in the artifact panel and separate window resolve local +images from the report's own directory and conversation. Both relative paths +such as `./charts/summary.png` and absolute `/mnt/...` paths are supported; +external image URLs keep their existing behavior. + If the sample cuts a CRLF line ending in half, the preview keeps the earlier complete rows and omits the incomplete final record, including when its last field is quoted. Text artifacts are streamed with HTTP byte-range support. The Web UI initially diff --git a/frontend/src/AGENTS.md b/frontend/src/AGENTS.md index 8e770d97bd..63d49d01a6 100644 --- a/frontend/src/AGENTS.md +++ b/frontend/src/AGENTS.md @@ -10,9 +10,9 @@ failure/replay-gap paths. Draft and confirmation regressions live in Answer details use `workspace/message-details` descriptors; skill run scoping is documented in `docs/skill-usage-ui.md`. -Artifact URLs encode raw filesystem paths, preserving literal percent sequences. -Only Markdown destinations decode once; relative images match decoded names -against raw artifact paths before encoding. +Encode raw paths preserving literal percent escapes. Decode Markdown destinations +once; match image names to raw paths. Preserve sanitization; key Streamdown by +file/thread/mock context plus scroll key. File-type detection uses the basename so extensionless `Dockerfile` and `Makefile` artifacts remain recognizable under nested or dotted directories. HTML preview base detection skips DOM construction without case-insensitive `base` diff --git a/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx b/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx index a9465576ec..11455f8500 100644 --- a/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx +++ b/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx @@ -707,6 +707,7 @@ export function ArtifactFileDetail({ content={editorContent} language={language ?? "text"} scrollKey={`${threadId}:${filepathFromProps}`} + artifact={{ filepath, threadId, isMock }} url={url} truncated={truncated} active={effectiveViewMode === "preview" && !isLoading} diff --git a/frontend/src/components/workspace/artifacts/artifact-file-preview.tsx b/frontend/src/components/workspace/artifacts/artifact-file-preview.tsx index 42d4c4315c..125ea2ec22 100644 --- a/frontend/src/components/workspace/artifacts/artifact-file-preview.tsx +++ b/frontend/src/components/workspace/artifacts/artifact-file-preview.tsx @@ -2,7 +2,8 @@ import { DownloadIcon } from "lucide-react"; import dynamic from "next/dynamic"; -import { useEffect, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { defaultUrlTransform, type UrlTransform } from "streamdown"; import { Button } from "@/components/ui/button"; import { @@ -12,7 +13,11 @@ import { createHtmlPreviewScrollKey, HTML_PREVIEW_SCROLL_MESSAGE_SOURCE, } from "@/core/artifacts/preview"; -import { urlOfArtifact } from "@/core/artifacts/utils"; +import { + type ArtifactImageContext, + resolveArtifactImageURL, + urlOfArtifact, +} from "@/core/artifacts/utils"; import { extractCitationSources } from "@/core/citations/sources"; import { SafeStreamdown, @@ -129,6 +134,7 @@ export function ArtifactFilePreview({ language, scrollKey, url, + artifact, truncated = false, active = true, }: { @@ -136,9 +142,22 @@ export function ArtifactFilePreview({ language: string; scrollKey: string; url?: string; + artifact?: ArtifactImageContext; truncated?: boolean; active?: boolean; }) { + const { filepath, threadId, isMock } = artifact ?? {}; + const transformUrl = useCallback( + (src, key, node) => + defaultUrlTransform( + node.tagName === "img" && key === "src" && filepath && threadId + ? resolveArtifactImageURL(src, { filepath, threadId, isMock }) + : src, + key, + node, + ), + [filepath, threadId, isMock], + ); const iframeRef = useRef(null); const scrollPositionRef = useRef({ x: 0, y: 0 }); const scrollMessageKey = useMemo( @@ -232,8 +251,11 @@ export function ArtifactFilePreview({ return (
{content ?? ""} diff --git a/frontend/src/components/workspace/artifacts/artifact-viewer.tsx b/frontend/src/components/workspace/artifacts/artifact-viewer.tsx index 86cf336827..cd714cae63 100644 --- a/frontend/src/components/workspace/artifacts/artifact-viewer.tsx +++ b/frontend/src/components/workspace/artifacts/artifact-viewer.tsx @@ -119,6 +119,7 @@ export function ArtifactViewer({ content={content} language={language} scrollKey={`${threadId}:${filepath}`} + artifact={{ filepath, threadId, isMock }} truncated={truncated} url={url} /> diff --git a/frontend/src/core/artifacts/utils.ts b/frontend/src/core/artifacts/utils.ts index e0fc803b59..adf6d2e82e 100644 --- a/frontend/src/core/artifacts/utils.ts +++ b/frontend/src/core/artifacts/utils.ts @@ -105,6 +105,54 @@ export function resolveMarkdownArtifactURL(src: string, threadId: string) { return `${resolveArtifactURL(decodeRelativeArtifactPath(path), threadId)}${suffix}`; } +export interface ArtifactImageContext { + filepath: string; + threadId: string; + isMock?: boolean; +} + +export function resolveArtifactImageURL( + src: string, + { filepath, threadId, isMock }: ArtifactImageContext, +) { + const { path, suffix } = splitPathSuffix(src); + if ( + !path || + !filepath.startsWith("/mnt/") || + (path.startsWith("/") && !path.startsWith("/mnt/")) || + /^[a-z][a-z\d+.-]*:/i.test(path) + ) { + return src; + } + + try { + const decodedPath = decodeRelativeArtifactPath(path); + if (decodedPath.includes("\\")) return src; + // Skill previews read SKILL.md inside the archive, as loadArtifactContent does. + const documentPath = filepath.endsWith(".skill") + ? `${filepath}/SKILL.md` + : filepath; + const base = new URL( + encodeArtifactPath(documentPath), + "https://artifact.invalid", + ); + const resolved = new URL(encodeArtifactPath(decodedPath), base); + if ( + resolved.origin !== base.origin || + !resolved.pathname.startsWith("/mnt/") + ) { + return src; + } + return `${urlOfArtifact({ + filepath: decodeRelativeArtifactPath(resolved.pathname), + threadId, + isMock, + })}${suffix}`; + } catch { + return src; + } +} + export function resolveMessageImageURL( src: string, threadId: string, diff --git a/frontend/tests/e2e/artifact-markdown-images.spec.ts b/frontend/tests/e2e/artifact-markdown-images.spec.ts new file mode 100644 index 0000000000..e23f534731 --- /dev/null +++ b/frontend/tests/e2e/artifact-markdown-images.spec.ts @@ -0,0 +1,125 @@ +import { expect, test } from "@playwright/test"; + +import { mockLangGraphAPI } from "./utils/mock-api"; + +const THREAD_ID = "00000000-0000-0000-0000-000000003141"; +const REPORT = "/mnt/user-data/outputs/report.md"; +const IMAGE = "/mnt/user-data/outputs/chart.png"; +const IMAGE_API = `/api/threads/${THREAD_ID}/artifacts${IMAGE}`; +const softExpect = expect.configure({ soft: true }); +const REPORT_BODY = [ + "# Fare analysis", + "", + "The chart below is stored beside this report.", + "", + `![Absolute chart](${IMAGE})`, + "", + "![Relative chart](./chart.png)", +].join("\n"); + +for (const width of [1280, 375]) { + test(`loads Markdown report images in the panel and viewer at ${width}px`, async ({ + page, + context, + }, testInfo) => { + await page.setViewportSize({ width, height: 900 }); + mockLangGraphAPI(page, { + threads: [ + { + thread_id: THREAD_ID, + title: "Report image regression", + messages: [ + { type: "human", id: "request", content: "Create a fare report." }, + { + type: "ai", + id: "report", + content: "", + tool_calls: [ + { + id: "present-report", + name: "present_files", + args: { filepaths: [REPORT] }, + }, + ], + }, + { + type: "tool", + id: "presented", + tool_call_id: "present-report", + name: "present_files", + content: "Report presented.", + }, + { type: "ai", id: "answer", content: `![Chat chart](${IMAGE})` }, + ], + artifacts: [REPORT, IMAGE], + }, + ], + }); + await context.route( + `**/api/threads/${THREAD_ID}/artifacts${REPORT}`, + (route) => + route.fulfill({ contentType: "text/markdown", body: REPORT_BODY }), + ); + await context.route(`**${IMAGE_API}`, (route) => + route.fulfill({ + contentType: "image/png", + path: "public/demo/threads/ad76c455-5bf9-4335-8517-fc03834ab828/user-data/outputs/visualizations/fare_analysis.png", + }), + ); + await context.route("**/artifacts/archive", (route) => + route.fulfill({ status: 404, body: "No archive in this fixture" }), + ); + + await page.goto(`/workspace/chats/${THREAD_ID}`); + await expect( + page.getByRole("img", { name: "Chat chart", exact: true }), + ).toBeVisible({ timeout: 30000 }); + await expect + .poll(() => + page + .getByRole("img", { name: "Chat chart", exact: true }) + .evaluate((img: HTMLImageElement) => img.naturalWidth), + ) + .toBeGreaterThan(0); + await page.getByText("report.md", { exact: true }).first().click(); + const panel = + width < 768 + ? page.getByRole("dialog", { name: "Artifacts", exact: true }) + : page.locator("#artifacts"); + await expect( + panel.getByRole("heading", { name: "Fare analysis" }), + ).toBeVisible({ timeout: 30000 }); + + for (const alt of ["Absolute chart", "Relative chart"]) { + const image = panel.getByRole("img", { name: alt, exact: true }); + await softExpect + .poll(() => + image.evaluateAll((images: HTMLImageElement[]) => + images.some((img) => img.complete && img.naturalWidth > 0), + ), + ) + .toBe(true); + } + + await page.screenshot({ path: testInfo.outputPath("report-panel.png") }); + + const popup = context.waitForEvent("page"); + await panel.getByRole("button", { name: "Open in new window" }).click(); + const viewer = await popup; + await viewer.setViewportSize({ width, height: 900 }); + await expect( + viewer.getByRole("heading", { name: "Fare analysis" }), + ).toBeVisible({ timeout: 30000 }); + for (const alt of ["Absolute chart", "Relative chart"]) { + const image = viewer.getByRole("img", { name: alt, exact: true }); + await softExpect + .poll(() => + image.evaluateAll((images: HTMLImageElement[]) => + images.some((img) => img.complete && img.naturalWidth > 0), + ), + ) + .toBe(true); + } + await viewer.close(); + }); +} diff --git a/frontend/tests/unit/components/workspace/artifacts/artifact-markdown-images.dom.test.tsx b/frontend/tests/unit/components/workspace/artifacts/artifact-markdown-images.dom.test.tsx new file mode 100644 index 0000000000..62f8155a85 --- /dev/null +++ b/frontend/tests/unit/components/workspace/artifacts/artifact-markdown-images.dom.test.tsx @@ -0,0 +1,145 @@ +import { afterEach, describe, expect, it } from "@rstest/core"; +import { cleanup, render, screen, waitFor } from "@testing-library/react"; + +import { ArtifactFilePreview } from "@/components/workspace/artifacts/artifact-file-preview"; +import { I18nProvider } from "@/core/i18n/context"; + +afterEach(cleanup); + +function preview( + filepath: string, + threadId: string, + isMock = false, + scrollKey = `${threadId}:${filepath}`, +) { + return ( + + + + ); +} + +describe("Markdown artifact images", () => { + it("resolves write-file preview images without a fetched document URL", async () => { + render(preview("/mnt/user-data/outputs/report.md", "first")); + expect((await screen.findByAltText("Chart")).getAttribute("src")).toBe( + "/api/threads/first/artifacts/mnt/user-data/outputs/chart.png", + ); + }); + + it("updates image destinations when unchanged content moves to another artifact or thread", async () => { + const { rerender } = render( + preview("/mnt/user-data/outputs/one/report.md", "first"), + ); + await screen.findByAltText("Chart"); + rerender(preview("/mnt/user-data/outputs/two/report.md", "first")); + await waitFor(() => + expect(screen.getByAltText("Chart").getAttribute("src")).toBe( + "/api/threads/first/artifacts/mnt/user-data/outputs/two/chart.png", + ), + ); + rerender(preview("/mnt/user-data/outputs/two/report.md", "second")); + await waitFor(() => + expect(screen.getByAltText("Chart").getAttribute("src")).toBe( + "/api/threads/second/artifacts/mnt/user-data/outputs/two/chart.png", + ), + ); + }); + + it("updates unchanged content when switching to mock artifacts", async () => { + const filepath = "/mnt/user-data/outputs/report.md"; + const { rerender } = render(preview(filepath, "first")); + await screen.findByAltText("Chart"); + rerender(preview(filepath, "first", true)); + await waitFor(() => + expect(screen.getByAltText("Chart").getAttribute("src")).toBe( + "/mock/api/threads/first/artifacts/mnt/user-data/outputs/chart.png", + ), + ); + }); + + it.each([ + { + name: "file path", + filepath: "/mnt/user-data/outputs/two/report.md", + threadId: "first", + isMock: false, + expected: + "/api/threads/first/artifacts/mnt/user-data/outputs/two/chart.png", + }, + { + name: "thread", + filepath: "/mnt/user-data/outputs/one/report.md", + threadId: "second", + isMock: false, + expected: + "/api/threads/second/artifacts/mnt/user-data/outputs/one/chart.png", + }, + { + name: "mock mode", + filepath: "/mnt/user-data/outputs/one/report.md", + threadId: "first", + isMock: true, + expected: + "/mock/api/threads/first/artifacts/mnt/user-data/outputs/one/chart.png", + }, + ])( + "updates image destinations when $name changes with a constant scroll key", + async ({ filepath, threadId, isMock, expected }) => { + const scrollKey = "stable-preview"; + const { rerender } = render( + preview( + "/mnt/user-data/outputs/one/report.md", + "first", + false, + scrollKey, + ), + ); + expect((await screen.findByAltText("Chart")).getAttribute("src")).toBe( + "/api/threads/first/artifacts/mnt/user-data/outputs/one/chart.png", + ); + rerender(preview(filepath, threadId, isMock, scrollKey)); + await waitFor(() => + expect(screen.getByAltText("Chart").getAttribute("src")).toBe(expected), + ); + }, + ); + + it.each([false, true])( + "keeps sanitization with artifact context: %s", + async (withContext) => { + const { container } = render( + + ' + } + language="markdown" + scrollKey="project-document" + artifact={ + withContext + ? { + filepath: "/mnt/user-data/outputs/report.md", + threadId: "first", + } + : undefined + } + /> + , + ); + expect((await screen.findByAltText("Chart")).getAttribute("src")).toBe( + withContext + ? "/api/threads/first/artifacts/mnt/user-data/outputs/chart.png" + : "chart.png", + ); + expect( + container.querySelector("script, [onerror], [src^='javascript:']"), + ).toBeNull(); + }, + ); +}); diff --git a/frontend/tests/unit/components/workspace/artifacts/artifact-viewer.dom.test.tsx b/frontend/tests/unit/components/workspace/artifacts/artifact-viewer.dom.test.tsx index 5027ea901d..d7277aa236 100644 --- a/frontend/tests/unit/components/workspace/artifacts/artifact-viewer.dom.test.tsx +++ b/frontend/tests/unit/components/workspace/artifacts/artifact-viewer.dom.test.tsx @@ -83,6 +83,38 @@ describe("ArtifactViewer", () => { }); }); + it("resolves local report images without changing external images or links", async () => { + mockedLoadArtifactContent.mockResolvedValue({ + content: [ + "![Absolute](/mnt/user-data/outputs/chart.png)", + "![Relative](./chart.png)", + "![External](https://example.com/chart.png)", + "[External link](https://example.com/report)", + ].join("\n\n"), + url: urlOfArtifact({ filepath, threadId }), + truncated: false, + previewBytes: 200, + sha256: undefined, + totalBytes: 200, + }); + renderViewer(); + for (const alt of ["Absolute", "Relative"]) { + const image = await screen.findByAltText(alt); + expect(image.getAttribute("src")).toBe( + urlOfArtifact({ + filepath: "/mnt/user-data/outputs/chart.png", + threadId, + }), + ); + } + expect((await screen.findByAltText("External")).getAttribute("src")).toBe( + "https://example.com/chart.png", + ); + expect( + screen.getByRole("link", { name: "External link" }).getAttribute("href"), + ).toBe("https://example.com/report"); + }); + it("fetches the whole file when a truncated preview is expanded", async () => { mockedLoadArtifactContent.mockImplementation(async ({ full }) => ({ content: full ? "# Full report\n\nEverything." : "# Full rep", diff --git a/frontend/tests/unit/core/artifacts/utils.test.ts b/frontend/tests/unit/core/artifacts/utils.test.ts index fd6f034d09..8955c43cce 100644 --- a/frontend/tests/unit/core/artifacts/utils.test.ts +++ b/frontend/tests/unit/core/artifacts/utils.test.ts @@ -57,6 +57,105 @@ describe("artifact URL helpers", () => { restoreEnv(saved); }); + describe("Markdown artifact image destinations", () => { + const context = { + filepath: "/mnt/user-data/outputs/reports/report.md", + threadId: "thread/1", + }; + + test.each([ + ["chart.png", "/mnt/user-data/outputs/reports/chart.png"], + ["./charts/a.png", "/mnt/user-data/outputs/reports/charts/a.png"], + ["../chart.png", "/mnt/user-data/outputs/chart.png"], + ["%2e%2e/chart.png", "/mnt/user-data/outputs/chart.png"], + ["/mnt/user-data/uploads/chart.png", "/mnt/user-data/uploads/chart.png"], + ["chart%20one.png", "/mnt/user-data/outputs/reports/chart%20one.png"], + ["chart%2520one.png", "/mnt/user-data/outputs/reports/chart%2520one.png"], + [ + "chart%23%3F.png?v=2#crop", + "/mnt/user-data/outputs/reports/chart%23%3F.png?v=2#crop", + ], + ["chart%.png", "/mnt/user-data/outputs/reports/chart%25.png"], + ["%E5%9B%BE.png", "/mnt/user-data/outputs/reports/%E5%9B%BE.png"], + ])("resolves %s against the report directory", async (src, path) => { + const { resolveArtifactImageURL } = await loadFreshArtifactUtils(); + expect(resolveArtifactImageURL(src, context)).toBe( + `/api/threads/thread%2F1/artifacts${path}`, + ); + }); + + test.each([ + "https://example.com/chart.png", + "//example.com/chart.png", + "data:image/png;base64,abc", + "blob:https://example.com/123", + "javascript:alert(1)", + "/api/other.png", + "", + "#chart", + "?v=2", + "../../../../api/secret.png", + "%2F%2Fexample.com/chart.png", + "..%5Cchart.png", + "\ud800.png", + ])("does not remap %s into the artifact route", async (src) => { + const { resolveArtifactImageURL } = await loadFreshArtifactUtils(); + expect(resolveArtifactImageURL(src, context)).toBe(src); + }); + + test("preserves raw percent sequences in the report directory", async () => { + const { resolveArtifactImageURL } = await loadFreshArtifactUtils(); + expect( + resolveArtifactImageURL("chart.png", { + ...context, + filepath: "/mnt/user-data/outputs/report%20folder/report.md", + }), + ).toBe( + "/api/threads/thread%2F1/artifacts/mnt/user-data/outputs/report%2520folder/chart.png", + ); + }); + + test("uses the configured backend and mock route", async () => { + setEnv("NEXT_PUBLIC_BACKEND_BASE_URL", "https://backend.example/root"); + const { resolveArtifactImageURL } = await loadFreshArtifactUtils(); + expect( + resolveArtifactImageURL("chart.png", { ...context, isMock: true }), + ).toBe( + "https://backend.example/root/mock/api/threads/thread%2F1/artifacts/mnt/user-data/outputs/reports/chart.png", + ); + }); + + test("uses bundled demo paths in static mode", async () => { + setEnv("NEXT_PUBLIC_STATIC_WEBSITE_ONLY", "true"); + const { resolveArtifactImageURL } = await loadFreshArtifactUtils(); + expect(resolveArtifactImageURL("../chart.png", context)).toBe( + "/demo/threads/thread%2F1/user-data/outputs/chart.png", + ); + }); + + test("bases skill previews on their embedded SKILL.md", async () => { + const { resolveArtifactImageURL } = await loadFreshArtifactUtils(); + expect( + resolveArtifactImageURL("images/chart.png", { + ...context, + filepath: "/mnt/user-data/outputs/example.skill", + }), + ).toBe( + "/api/threads/thread%2F1/artifacts/mnt/user-data/outputs/example.skill/images/chart.png", + ); + }); + + test("does not invent a virtual path for a non-artifact document", async () => { + const { resolveArtifactImageURL } = await loadFreshArtifactUtils(); + expect( + resolveArtifactImageURL("chart.png", { + ...context, + filepath: "report.md", + }), + ).toBe("chart.png"); + }); + }); + test("maps static demo artifact paths to bundled public files", async () => { setEnv("NEXT_PUBLIC_STATIC_WEBSITE_ONLY", "true");