Fix Windows image previews (#1811)
* fix(images): preserve Windows preview paths Provider image markdown used escaped backslash paths on Windows, which markdown parsing could turn into encoded drive paths that the app treated as workspace-relative. Emit drive-letter paths as file URIs and normalize existing encoded local paths before file preview. * test(server): accept file URI image sources Windows provider image output now renders drive-letter paths as file URIs. Decode those markdown sources before filesystem assertions so the image rendering tests keep checking the materialized file. * fix(images): keep plain local paths literal Limit legacy URI decoding to markdown-encoded Windows drive paths so ordinary local paths with percent sequences remain unchanged. Document why provider image markdown matching still accepts old doubled-backslash history.
This commit is contained in:
@@ -1,5 +1,12 @@
|
|||||||
import { describe, expect, it } from "vitest";
|
import { describe, expect, it } from "vitest";
|
||||||
import { createImageSourceCacheKey, parseDataUrl, parseImageDataUrl, pathToFileUri } from "./utils";
|
import {
|
||||||
|
createImageSourceCacheKey,
|
||||||
|
fileUriToPath,
|
||||||
|
localFileSourceToPath,
|
||||||
|
parseDataUrl,
|
||||||
|
parseImageDataUrl,
|
||||||
|
pathToFileUri,
|
||||||
|
} from "./utils";
|
||||||
|
|
||||||
describe("pathToFileUri", () => {
|
describe("pathToFileUri", () => {
|
||||||
it("converts POSIX absolute paths to file URIs", () => {
|
it("converts POSIX absolute paths to file URIs", () => {
|
||||||
@@ -23,6 +30,24 @@ describe("pathToFileUri", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("fileUriToPath", () => {
|
||||||
|
it("converts Windows drive-letter file URIs back to paths", () => {
|
||||||
|
expect(fileUriToPath("file:///C:/Users/file.txt")).toBe("C:/Users/file.txt");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("localFileSourceToPath", () => {
|
||||||
|
it("decodes markdown-encoded Windows drive-letter paths", () => {
|
||||||
|
expect(localFileSourceToPath("C:%5CUsers%5Cfile.txt")).toBe("C:/Users/file.txt");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("preserves literal percent sequences in plain local paths", () => {
|
||||||
|
expect(localFileSourceToPath("/tmp/image%20with%20literal%20percent.png")).toBe(
|
||||||
|
"/tmp/image%20with%20literal%20percent.png",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("parseDataUrl", () => {
|
describe("parseDataUrl", () => {
|
||||||
it("accepts base64 data URLs with media-type parameters", () => {
|
it("accepts base64 data URLs with media-type parameters", () => {
|
||||||
expect(parseDataUrl("data:image/png;charset=utf-8;name=preview;base64,AAECAw==")).toEqual({
|
expect(parseDataUrl("data:image/png;charset=utf-8;name=preview;base64,AAECAw==")).toEqual({
|
||||||
|
|||||||
@@ -139,11 +139,41 @@ export function pathToFileUri(path: string): string {
|
|||||||
return `file:///${path.replace(/\\/g, "/")}`;
|
return `file:///${path.replace(/\\/g, "/")}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function decodeFilePathSource(source: string): string {
|
||||||
|
try {
|
||||||
|
return decodeURIComponent(source);
|
||||||
|
} catch {
|
||||||
|
return source;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function normalizeWindowsDrivePath(path: string): string {
|
||||||
|
if (!/^[A-Za-z]:[\\/]/.test(path)) {
|
||||||
|
return path;
|
||||||
|
}
|
||||||
|
return path.replace(/\\/g, "/");
|
||||||
|
}
|
||||||
|
|
||||||
|
function isMarkdownEncodedWindowsDrivePath(source: string): boolean {
|
||||||
|
return /^[A-Za-z]:(?:%5[Cc]|%2[Ff])/.test(source);
|
||||||
|
}
|
||||||
|
|
||||||
export function fileUriToPath(uri: string): string {
|
export function fileUriToPath(uri: string): string {
|
||||||
if (!uri.startsWith("file://")) {
|
if (!uri.startsWith("file://")) {
|
||||||
return uri;
|
return uri;
|
||||||
}
|
}
|
||||||
return decodeURIComponent(uri.replace(/^file:\/\//, ""));
|
const decodedPath = decodeFilePathSource(uri.replace(/^file:\/\//, ""));
|
||||||
|
return normalizeWindowsDrivePath(decodedPath.replace(/^\/([A-Za-z]:[\\/])/, "$1"));
|
||||||
|
}
|
||||||
|
|
||||||
|
export function localFileSourceToPath(source: string): string {
|
||||||
|
let path = source;
|
||||||
|
if (source.startsWith("file://")) {
|
||||||
|
path = fileUriToPath(source);
|
||||||
|
} else if (isMarkdownEncodedWindowsDrivePath(source)) {
|
||||||
|
path = decodeFilePathSource(source);
|
||||||
|
}
|
||||||
|
return normalizeWindowsDrivePath(path);
|
||||||
}
|
}
|
||||||
|
|
||||||
export function getFileExtensionFromName(fileName: string | null | undefined): string {
|
export function getFileExtensionFromName(fileName: string | null | undefined): string {
|
||||||
|
|||||||
@@ -79,6 +79,19 @@ describe("resolveAssistantImageSource", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("normalizes markdown-encoded Windows paths into file RPC requests", () => {
|
||||||
|
expect(
|
||||||
|
resolveAssistantImageSource({
|
||||||
|
source: "C:%5CUsers%5Chanse%5CAppData%5CLocal%5CTemp%5Cpaseo-attachments%5Cimage.png",
|
||||||
|
workspaceRoot: "C:/Users/hanse/eatingkat",
|
||||||
|
}),
|
||||||
|
).toEqual({
|
||||||
|
kind: "file_rpc",
|
||||||
|
cwd: "C:/",
|
||||||
|
path: "C:/Users/hanse/AppData/Local/Temp/paseo-attachments/image.png",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it("falls back to the drive root for Windows absolute paths", () => {
|
it("falls back to the drive root for Windows absolute paths", () => {
|
||||||
expect(
|
expect(
|
||||||
resolveAssistantImageSource({
|
resolveAssistantImageSource({
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { fileUriToPath } from "@/attachments/utils";
|
import { localFileSourceToPath } from "@/attachments/utils";
|
||||||
import { resolveFilePreviewReadTarget } from "@/file-explorer/preview-target";
|
import { resolveFilePreviewReadTarget } from "@/file-explorer/preview-target";
|
||||||
|
|
||||||
export type AssistantImageSourceResolution =
|
export type AssistantImageSourceResolution =
|
||||||
@@ -18,13 +18,8 @@ export function resolveAssistantImageSource(input: {
|
|||||||
return { kind: "direct", uri: source };
|
return { kind: "direct", uri: source };
|
||||||
}
|
}
|
||||||
|
|
||||||
const sourcePath = source.startsWith("file://") ? fileUriToPath(source) : source;
|
|
||||||
if (!sourcePath) {
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
const readTarget = resolveFilePreviewReadTarget({
|
const readTarget = resolveFilePreviewReadTarget({
|
||||||
path: sourcePath,
|
path: localFileSourceToPath(source),
|
||||||
workspaceRoot: input.workspaceRoot,
|
workspaceRoot: input.workspaceRoot,
|
||||||
});
|
});
|
||||||
if (!readTarget) {
|
if (!readTarget) {
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import { existsSync, rmSync } from "node:fs";
|
import { existsSync, rmSync } from "node:fs";
|
||||||
|
import { fileURLToPath } from "node:url";
|
||||||
import type { SDKMessage } from "@anthropic-ai/claude-agent-sdk";
|
import type { SDKMessage } from "@anthropic-ai/claude-agent-sdk";
|
||||||
import { describe, expect, test } from "vitest";
|
import { describe, expect, test } from "vitest";
|
||||||
|
|
||||||
@@ -146,7 +147,8 @@ function markdownImageSource(markdown: string): string {
|
|||||||
throw new Error(`Expected markdown image, got: ${markdown}`);
|
throw new Error(`Expected markdown image, got: ${markdown}`);
|
||||||
}
|
}
|
||||||
// Reverse escapeMarkdownImageSource: "\\" -> "\" and "\)" -> ")" (Windows paths are escaped).
|
// Reverse escapeMarkdownImageSource: "\\" -> "\" and "\)" -> ")" (Windows paths are escaped).
|
||||||
return match[1].replace(/\\(.)/g, "$1");
|
const source = match[1].replace(/\\(.)/g, "$1");
|
||||||
|
return source.startsWith("file://") ? fileURLToPath(source) : source;
|
||||||
}
|
}
|
||||||
|
|
||||||
describe("Claude tool_result image rendering", () => {
|
describe("Claude tool_result image rendering", () => {
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import { mkdtemp } from "node:fs/promises";
|
|||||||
import { tmpdir } from "node:os";
|
import { tmpdir } from "node:os";
|
||||||
import path from "node:path";
|
import path from "node:path";
|
||||||
import { PassThrough } from "node:stream";
|
import { PassThrough } from "node:stream";
|
||||||
|
import { fileURLToPath } from "node:url";
|
||||||
|
|
||||||
import type {
|
import type {
|
||||||
AgentLaunchContext,
|
AgentLaunchContext,
|
||||||
@@ -111,7 +112,8 @@ function markdownImageSource(markdown: string): string {
|
|||||||
if (!match) {
|
if (!match) {
|
||||||
throw new Error(`Expected markdown image, got: ${markdown}`);
|
throw new Error(`Expected markdown image, got: ${markdown}`);
|
||||||
}
|
}
|
||||||
return match[1].replace(/\\\)/g, ")");
|
const source = match[1].replace(/\\\)/g, ")");
|
||||||
|
return source.startsWith("file://") ? fileURLToPath(source) : source;
|
||||||
}
|
}
|
||||||
|
|
||||||
function emitCodexUserMessage(
|
function emitCodexUserMessage(
|
||||||
|
|||||||
@@ -1,9 +1,20 @@
|
|||||||
import { describe, expect, test } from "vitest";
|
import { describe, expect, test } from "vitest";
|
||||||
|
|
||||||
import { isProviderImageMarkdown } from "./provider-image-output.js";
|
import {
|
||||||
|
isProviderImageMarkdown,
|
||||||
|
renderProviderImageOutputAsAssistantMarkdown,
|
||||||
|
} from "./provider-image-output.js";
|
||||||
|
|
||||||
const HASH = "a".repeat(64);
|
const HASH = "a".repeat(64);
|
||||||
|
|
||||||
|
function renderImageMarkdown(path: string): string {
|
||||||
|
const item = renderProviderImageOutputAsAssistantMarkdown({ path });
|
||||||
|
if (!item || item.type !== "assistant_message") {
|
||||||
|
throw new Error("Expected provider image output to render as assistant markdown.");
|
||||||
|
}
|
||||||
|
return item.text;
|
||||||
|
}
|
||||||
|
|
||||||
describe("isProviderImageMarkdown", () => {
|
describe("isProviderImageMarkdown", () => {
|
||||||
test("matches the markdown emitted for a materialized attachment", () => {
|
test("matches the markdown emitted for a materialized attachment", () => {
|
||||||
expect(isProviderImageMarkdown(``)).toBe(true);
|
expect(isProviderImageMarkdown(``)).toBe(true);
|
||||||
@@ -18,6 +29,17 @@ describe("isProviderImageMarkdown", () => {
|
|||||||
).toBe(true);
|
).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("emits Windows file paths as file URIs", () => {
|
||||||
|
const markdown = renderImageMarkdown(
|
||||||
|
`C:\\Users\\me\\AppData\\Local\\Temp\\paseo-attachments\\${HASH}.png`,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(markdown).toBe(
|
||||||
|
``,
|
||||||
|
);
|
||||||
|
expect(isProviderImageMarkdown(markdown)).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
test("rejects user-authored markdown that is not a materialized attachment", () => {
|
test("rejects user-authored markdown that is not a materialized attachment", () => {
|
||||||
// No content hash — a hand-written path, not something the writer produced.
|
// No content hash — a hand-written path, not something the writer produced.
|
||||||
expect(isProviderImageMarkdown("")).toBe(false);
|
expect(isProviderImageMarkdown("")).toBe(false);
|
||||||
|
|||||||
@@ -66,11 +66,10 @@ export function materializeProviderImage(image: {
|
|||||||
return { path: filePath };
|
return { path: filePath };
|
||||||
}
|
}
|
||||||
|
|
||||||
// Recognizes the markdown renderProviderImageOutputAsAssistantMarkdown emits for a materialized
|
// Recognizes markdown rendered for a materialized provider image: its source is a content-hashed
|
||||||
// provider image: its source is a content-hashed file in the attachments dir. Matching the full
|
// file in the attachments dir. Matching the full <hash>.<ext> shape (not just a leading "![")
|
||||||
// <hash>.<ext> shape (not just a leading "![") keeps user-authored text from being mistaken for a
|
// keeps user-authored text from being mistaken for a provider image during history replay. The
|
||||||
// provider image when it reaches the history-replay filter. The separator class allows one-or-more
|
// separator still accepts old doubled-backslash Windows history; new Windows output uses file URIs.
|
||||||
// because on Windows the path uses "\\" and escapeMarkdownImageSource doubles each backslash.
|
|
||||||
const PROVIDER_IMAGE_MARKDOWN = new RegExp(
|
const PROVIDER_IMAGE_MARKDOWN = new RegExp(
|
||||||
`^!\\[[^\\]]*\\]\\([^)]*${PROVIDER_IMAGE_ATTACHMENT_DIR}[/\\\\]+[0-9a-f]{64}\\.[a-z0-9]+\\)`,
|
`^!\\[[^\\]]*\\]\\([^)]*${PROVIDER_IMAGE_ATTACHMENT_DIR}[/\\\\]+[0-9a-f]{64}\\.[a-z0-9]+\\)`,
|
||||||
);
|
);
|
||||||
@@ -96,8 +95,15 @@ function escapeMarkdownImageAlt(value: string): string {
|
|||||||
return value.replace(/\\/g, "\\\\").replace(/\]/g, "\\]");
|
return value.replace(/\\/g, "\\\\").replace(/\]/g, "\\]");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function markdownImageSource(value: string): string {
|
||||||
|
if (/^[A-Za-z]:[\\/]/.test(value)) {
|
||||||
|
return `file:///${value.replace(/\\/g, "/")}`;
|
||||||
|
}
|
||||||
|
return value;
|
||||||
|
}
|
||||||
|
|
||||||
function escapeMarkdownImageSource(value: string): string {
|
function escapeMarkdownImageSource(value: string): string {
|
||||||
return value.replace(/\\/g, "\\\\").replace(/\)/g, "\\)");
|
return markdownImageSource(value).replace(/\\/g, "\\\\").replace(/\)/g, "\\)");
|
||||||
}
|
}
|
||||||
|
|
||||||
export function renderProviderImageOutputAsAssistantMarkdown(
|
export function renderProviderImageOutputAsAssistantMarkdown(
|
||||||
|
|||||||
Reference in New Issue
Block a user