From f0ba280365b29016223183b38f261694ce7db586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=80=E7=89=87=E4=B8=80=E7=89=87?= <3037678834@qq.com> Date: Wed, 28 Oct 2026 07:35:01 +0800 Subject: [PATCH] fix(artifacts): parse write-file URLs through the guarded helper (#7609) `ArtifactFileDetail` and `loadArtifactContentFromToolCall` re-parsed `write-file:` URLs with `new URL` + `decodeURIComponent` after `parseWriteFileArtifact` had already rejected malformed values. The panel restores its selection from session storage and only type-checks it as a string, so a stale or hand-edited `write-file:%E0%A4%A` (or `write-file://[`) threw `URIError`/`TypeError` during render. Export the guarded parser and read `path` / `tool_call_id` from one parsed result in both places; a rejected URL degrades to the raw string or `undefined` instead of taking the panel down. --- .../artifacts/artifact-file-detail.tsx | 23 +++++----- frontend/src/core/artifacts/loader.ts | 11 +++-- frontend/src/core/artifacts/preview.ts | 2 +- .../artifact-file-detail.dom.test.tsx | 9 ++++ .../tests/unit/core/artifacts/loader.test.ts | 44 +++++++++++++++++++ .../tests/unit/core/artifacts/preview.test.ts | 20 +++++++++ 6 files changed, 92 insertions(+), 17 deletions(-) diff --git a/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx b/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx index 11455f8500..cbf7af2ae2 100644 --- a/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx +++ b/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx @@ -47,6 +47,7 @@ import { useArtifactContent } from "@/core/artifacts/hooks"; import { getArtifactViewState, getTabularDelimiter, + parseWriteFileArtifact, } from "@/core/artifacts/preview"; import { urlOfArtifact } from "@/core/artifacts/utils"; import { @@ -106,13 +107,15 @@ export function ArtifactFileDetail({ const isWriteFile = useMemo(() => { return filepathFromProps.startsWith("write-file:"); }, [filepathFromProps]); - const filepath = useMemo(() => { - if (isWriteFile) { - const url = new URL(filepathFromProps); - return decodeURIComponent(url.pathname); - } - return filepathFromProps; - }, [filepathFromProps, isWriteFile]); + // `write-file:` URLs come from messages and from the panel's persisted + // selection, so parse them through the guarded helper instead of `new URL` + // + `decodeURIComponent` directly: a malformed value must degrade to the + // raw string, not throw during render. + const parsedWriteFile = useMemo( + () => (isWriteFile ? parseWriteFileArtifact(filepathFromProps) : undefined), + [filepathFromProps, isWriteFile], + ); + const filepath = parsedWriteFile?.path ?? filepathFromProps; // Keep these local because ChatBox replaces context artifacts with thread state. const [openedPresentedFilepaths, setOpenedPresentedFilepaths] = useState< string[] @@ -178,11 +181,7 @@ export function ArtifactFileDetail({ const isSupportPreview = language === "html" || language === "markdown" || isTabular; const toolResult = (() => { - if (!isWriteFile) { - return undefined; - } - const url = new URL(filepathFromProps); - const toolCallId = url.searchParams.get("tool_call_id"); + const toolCallId = parsedWriteFile?.toolCallId; if (!toolCallId) { return undefined; } diff --git a/frontend/src/core/artifacts/loader.ts b/frontend/src/core/artifacts/loader.ts index b68a043644..6192ce92f0 100644 --- a/frontend/src/core/artifacts/loader.ts +++ b/frontend/src/core/artifacts/loader.ts @@ -4,7 +4,7 @@ import { fetch } from "@/core/api/fetcher"; import type { AgentThreadState } from "../threads"; -import { buildWriteFileDraftContent } from "./preview"; +import { buildWriteFileDraftContent, parseWriteFileArtifact } from "./preview"; import { urlOfArtifact } from "./utils"; function fnv1aHash(value: string): string { @@ -122,6 +122,11 @@ export function loadArtifactContentFromToolCall({ url: string; thread: BaseStream; }) { + const target = parseWriteFileArtifact(urlString); + if (!target) { + return undefined; + } + const draftContent = buildWriteFileDraftContent({ filepath: urlString, messages: thread.messages, @@ -130,9 +135,7 @@ export function loadArtifactContentFromToolCall({ return draftContent; } - const url = new URL(urlString); - const toolCallId = url.searchParams.get("tool_call_id"); - const messageId = url.searchParams.get("message_id"); + const { messageId, toolCallId } = target; if (messageId && toolCallId) { const message = thread.messages.find((message) => message.id === messageId); if (message?.type === "ai" && message.tool_calls) { diff --git a/frontend/src/core/artifacts/preview.ts b/frontend/src/core/artifacts/preview.ts index bca22e108c..6bf75c260a 100644 --- a/frontend/src/core/artifacts/preview.ts +++ b/frontend/src/core/artifacts/preview.ts @@ -68,7 +68,7 @@ function findToolResult( return undefined; } -function parseWriteFileArtifact(filepath: string) { +export function parseWriteFileArtifact(filepath: string) { if (!isWriteFileArtifact(filepath)) { return undefined; } diff --git a/frontend/tests/unit/components/workspace/artifacts/artifact-file-detail.dom.test.tsx b/frontend/tests/unit/components/workspace/artifacts/artifact-file-detail.dom.test.tsx index 23e3766efe..0871cbe848 100644 --- a/frontend/tests/unit/components/workspace/artifacts/artifact-file-detail.dom.test.tsx +++ b/frontend/tests/unit/components/workspace/artifacts/artifact-file-detail.dom.test.tsx @@ -167,3 +167,12 @@ describe("ArtifactFileDetail browser-preview iframe", () => { ); }); }); + +describe("ArtifactFileDetail malformed write-file URLs", () => { + it.each(["write-file:%E0%A4%A", "write-file://["])( + "renders the raw value without throwing: %s", + (filepath) => { + expect(() => renderDetail(filepath)).not.toThrow(); + }, + ); +}); diff --git a/frontend/tests/unit/core/artifacts/loader.test.ts b/frontend/tests/unit/core/artifacts/loader.test.ts index 353f2cb461..21d4fa7ecc 100644 --- a/frontend/tests/unit/core/artifacts/loader.test.ts +++ b/frontend/tests/unit/core/artifacts/loader.test.ts @@ -3,6 +3,7 @@ import { afterEach, describe, expect, it, rs } from "@rstest/core"; import { ARTIFACT_PREVIEW_MAX_BYTES, loadArtifactContent, + loadArtifactContentFromToolCall, } from "@/core/artifacts/loader"; describe("loadArtifactContent", () => { @@ -190,3 +191,46 @@ describe("loadArtifactContent", () => { expect(loaded.sha256).toHaveLength(8); }); }); + +describe("loadArtifactContentFromToolCall", () => { + type ToolCallThread = Parameters< + typeof loadArtifactContentFromToolCall + >[0]["thread"]; + + const thread = { + messages: [ + { + type: "ai", + id: "ai-1", + tool_calls: [ + { + id: "call-1", + name: "str_replace", + args: { + path: "/mnt/user-data/outputs/report.md", + content: "patched", + }, + }, + ], + }, + ], + } as unknown as ToolCallThread; + + it("returns undefined instead of throwing for a malformed write-file URL", () => { + expect( + loadArtifactContentFromToolCall({ url: "write-file://[", thread }), + ).toBeUndefined(); + expect( + loadArtifactContentFromToolCall({ url: "write-file:%E0%A4%A", thread }), + ).toBeUndefined(); + }); + + it("reads non-write_file tool output through the parsed coordinates", () => { + expect( + loadArtifactContentFromToolCall({ + url: "write-file:/mnt/user-data/outputs/report.md?message_id=ai-1&tool_call_id=call-1", + thread, + }), + ).toBe("patched"); + }); +}); diff --git a/frontend/tests/unit/core/artifacts/preview.test.ts b/frontend/tests/unit/core/artifacts/preview.test.ts index 1c15a16f22..129f7cc5d4 100644 --- a/frontend/tests/unit/core/artifacts/preview.test.ts +++ b/frontend/tests/unit/core/artifacts/preview.test.ts @@ -5,6 +5,7 @@ import { buildWriteFileDraftContent, createHtmlPreviewScrollKey, getArtifactViewState, + parseWriteFileArtifact, } from "@/core/artifacts/preview"; const ARTIFACT_PATH = "/artifact-fixtures/report.html"; @@ -283,3 +284,22 @@ test("scopes HTML scroll restoration without exposing the artifact path", () => expect(html).not.toContain("/artifact-fixtures/a"); expect(html).not.toContain("