From 1708b11fab3668e0db1b59435f6d46cc9ecbdbab Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 26 Feb 2026 16:52:06 +0100 Subject: [PATCH] refactor(pi): simplify image reference detection --- .../pi-embedded-runner/run/images.test.ts | 12 +- src/agents/pi-embedded-runner/run/images.ts | 118 ++++++++---------- 2 files changed, 65 insertions(+), 65 deletions(-) diff --git a/src/agents/pi-embedded-runner/run/images.test.ts b/src/agents/pi-embedded-runner/run/images.test.ts index 410c9de8f..8a879a1bb 100644 --- a/src/agents/pi-embedded-runner/run/images.test.ts +++ b/src/agents/pi-embedded-runner/run/images.test.ts @@ -63,7 +63,6 @@ describe("detectImageReferences", () => { expect(refs).toHaveLength(1); expect(refs.some((r) => r.type === "path")).toBe(true); - expect(refs.some((r) => r.type === "url")).toBe(false); }); it("handles various image extensions", () => { @@ -83,6 +82,17 @@ describe("detectImageReferences", () => { expect(refs).toHaveLength(1); }); + it("dedupe casing follows host filesystem conventions", () => { + const prompt = "Look at /tmp/Image.png and /tmp/image.png"; + const refs = detectImageReferences(prompt); + + if (process.platform === "win32") { + expect(refs).toHaveLength(1); + return; + } + expect(refs).toHaveLength(2); + }); + it("returns empty array when no images found", () => { const prompt = "Just some text without any image references"; const refs = detectImageReferences(prompt); diff --git a/src/agents/pi-embedded-runner/run/images.ts b/src/agents/pi-embedded-runner/run/images.ts index 7b24dae94..bcd25e724 100644 --- a/src/agents/pi-embedded-runner/run/images.ts +++ b/src/agents/pi-embedded-runner/run/images.ts @@ -27,22 +27,13 @@ const IMAGE_EXTENSION_NAMES = [ ] as const; const IMAGE_EXTENSIONS = new Set(IMAGE_EXTENSION_NAMES.map((ext) => `.${ext}`)); const IMAGE_EXTENSION_PATTERN = IMAGE_EXTENSION_NAMES.join("|"); -const MEDIA_ATTACHED_PATH_PATTERN = new RegExp( - `^\\s*(.+?\\.(?:${IMAGE_EXTENSION_PATTERN}))\\s*(?:\\(|$|\\|)`, - "i", -); -const MESSAGE_IMAGE_PATTERN = new RegExp( - `\\[Image:\\s*source:\\s*([^\\]]+\\.(?:${IMAGE_EXTENSION_PATTERN}))\\]`, - "gi", -); -const FILE_URL_PATTERN = new RegExp( - `file://[^\\s<>"'\\\`\\]]+\\.(?:${IMAGE_EXTENSION_PATTERN})`, - "gi", -); -const PATH_PATTERN = new RegExp( - `(?:^|\\s|["'\\\`(])((\\.\\.?/|[~/])[^\\s"'\\\`()\\[\\]]*\\.(?:${IMAGE_EXTENSION_PATTERN}))`, - "gi", -); +const MEDIA_ATTACHED_PATH_REGEX_SOURCE = + "^\\s*(.+?\\.(?:" + IMAGE_EXTENSION_PATTERN + "))\\s*(?:\\(|$|\\|)"; +const MESSAGE_IMAGE_REGEX_SOURCE = + "\\[Image:\\s*source:\\s*([^\\]]+\\.(?:" + IMAGE_EXTENSION_PATTERN + "))\\]"; +const FILE_URL_REGEX_SOURCE = "file://[^\\s<>\"'`\\]]+\\.(?:" + IMAGE_EXTENSION_PATTERN + ")"; +const PATH_REGEX_SOURCE = + "(?:^|\\s|[\"'`(])((\\.\\.?/|[~/])[^\\s\"'`()\\[\\]]*\\.(?:" + IMAGE_EXTENSION_PATTERN + "))"; /** * Result of detecting an image reference in text. @@ -50,9 +41,9 @@ const PATH_PATTERN = new RegExp( export interface DetectedImageRef { /** The raw matched string from the prompt */ raw: string; - /** The type of reference (path or url) */ - type: "path" | "url"; - /** The resolved/normalized path or URL */ + /** The type of reference */ + type: "path"; + /** The resolved/normalized path */ resolved: string; } @@ -64,6 +55,10 @@ function isImageExtension(filePath: string): boolean { return IMAGE_EXTENSIONS.has(ext); } +function normalizeRefForDedupe(raw: string): string { + return process.platform === "win32" ? raw.toLowerCase() : raw; +} + async function sanitizeImagesWithLog( images: ImageContent[], label: string, @@ -100,7 +95,8 @@ export function detectImageReferences(prompt: string): DetectedImageRef[] { // Helper to add a path ref const addPathRef = (raw: string) => { const trimmed = raw.trim(); - if (!trimmed || seen.has(trimmed.toLowerCase())) { + const dedupeKey = normalizeRefForDedupe(trimmed); + if (!trimmed || seen.has(dedupeKey)) { return; } if (trimmed.startsWith("http://") || trimmed.startsWith("https://")) { @@ -109,7 +105,7 @@ export function detectImageReferences(prompt: string): DetectedImageRef[] { if (!isImageExtension(trimmed)) { return; } - seen.add(trimmed.toLowerCase()); + seen.add(dedupeKey); const resolved = trimmed.startsWith("~") ? resolveUserPath(trimmed) : trimmed; refs.push({ raw: trimmed, type: "path", resolved }); }; @@ -118,6 +114,10 @@ export function detectImageReferences(prompt: string): DetectedImageRef[] { // Each bracket = ONE file. The | separates path from URL, not multiple files. // Multi-file format uses separate brackets on separate lines. const mediaAttachedPattern = /\[media attached(?:\s+\d+\/\d+)?:\s*([^\]]+)\]/gi; + const mediaAttachedPathPattern = new RegExp(MEDIA_ATTACHED_PATH_REGEX_SOURCE, "i"); + const messageImagePattern = new RegExp(MESSAGE_IMAGE_REGEX_SOURCE, "gi"); + const fileUrlPattern = new RegExp(FILE_URL_REGEX_SOURCE, "gi"); + const pathPattern = new RegExp(PATH_REGEX_SOURCE, "gi"); let match: RegExpExecArray | null; while ((match = mediaAttachedPattern.exec(prompt)) !== null) { const content = match[1]; @@ -131,15 +131,14 @@ export function detectImageReferences(prompt: string): DetectedImageRef[] { // Format is: path (type) | url OR just: path (type) // Path may contain spaces (e.g., "ChatGPT Image Apr 21.png") // Use non-greedy .+? to stop at first image extension - const pathMatch = content.match(MEDIA_ATTACHED_PATH_PATTERN); + const pathMatch = content.match(mediaAttachedPathPattern); if (pathMatch?.[1]) { addPathRef(pathMatch[1].trim()); } } // Pattern for [Image: source: /path/...] format from messaging systems - MESSAGE_IMAGE_PATTERN.lastIndex = 0; - while ((match = MESSAGE_IMAGE_PATTERN.exec(prompt)) !== null) { + while ((match = messageImagePattern.exec(prompt)) !== null) { const raw = match[1]?.trim(); if (raw) { addPathRef(raw); @@ -149,13 +148,13 @@ export function detectImageReferences(prompt: string): DetectedImageRef[] { // Remote HTTP(S) URLs are intentionally ignored. Native image injection is local-only. // Pattern for file:// URLs - treat as paths since loadWebMedia handles them - FILE_URL_PATTERN.lastIndex = 0; - while ((match = FILE_URL_PATTERN.exec(prompt)) !== null) { + while ((match = fileUrlPattern.exec(prompt)) !== null) { const raw = match[0]; - if (seen.has(raw.toLowerCase())) { + const dedupeKey = normalizeRefForDedupe(raw); + if (seen.has(dedupeKey)) { continue; } - seen.add(raw.toLowerCase()); + seen.add(dedupeKey); // Use fileURLToPath for proper handling (e.g., file://localhost/path) try { const resolved = fileURLToPath(raw); @@ -171,8 +170,7 @@ export function detectImageReferences(prompt: string): DetectedImageRef[] { // - ./relative/path.ext // - ../parent/path.ext // - ~/home/path.ext - PATH_PATTERN.lastIndex = 0; - while ((match = PATH_PATTERN.exec(prompt)) !== null) { + while ((match = pathPattern.exec(prompt)) !== null) { // Use capture group 1 (the path without delimiter prefix); skip if undefined if (match[1]) { addPathRef(match[1]); @@ -183,7 +181,7 @@ export function detectImageReferences(prompt: string): DetectedImageRef[] { } /** - * Loads an image from a file path or URL and returns it as ImageContent. + * Loads an image from a file path and returns it as ImageContent. * * @param ref The detected image reference * @param workspaceDir The current workspace directory for resolving relative paths @@ -202,42 +200,34 @@ export async function loadImageFromRef( try { let targetPath = ref.resolved; - // Remote URL loading is disabled (local-only). - if (ref.type === "url") { - log.debug(`Native image: rejecting remote URL (local-only): ${ref.resolved}`); - return null; - } - // Resolve paths relative to sandbox or workspace as needed - if (ref.type === "path") { - if (options?.sandbox) { - try { - const resolved = await resolveSandboxedBridgeMediaPath({ - sandbox: { - root: options.sandbox.root, - bridge: options.sandbox.bridge, - workspaceOnly: options.workspaceOnly, - }, - mediaPath: targetPath, - }); - targetPath = resolved.resolved; - } catch (err) { - log.debug( - `Native image: sandbox validation failed for ${ref.resolved}: ${err instanceof Error ? err.message : String(err)}`, - ); - return null; - } - } else if (!path.isAbsolute(targetPath)) { - targetPath = path.resolve(workspaceDir, targetPath); - } - if (options?.workspaceOnly && !options?.sandbox) { - const root = options?.sandbox?.root ?? workspaceDir; - await assertSandboxPath({ - filePath: targetPath, - cwd: root, - root, + if (options?.sandbox) { + try { + const resolved = await resolveSandboxedBridgeMediaPath({ + sandbox: { + root: options.sandbox.root, + bridge: options.sandbox.bridge, + workspaceOnly: options.workspaceOnly, + }, + mediaPath: targetPath, }); + targetPath = resolved.resolved; + } catch (err) { + log.debug( + `Native image: sandbox validation failed for ${ref.resolved}: ${err instanceof Error ? err.message : String(err)}`, + ); + return null; } + } else if (!path.isAbsolute(targetPath)) { + targetPath = path.resolve(workspaceDir, targetPath); + } + if (options?.workspaceOnly && !options?.sandbox) { + const root = options?.sandbox?.root ?? workspaceDir; + await assertSandboxPath({ + filePath: targetPath, + cwd: root, + root, + }); } // loadWebMedia handles local file paths (including file:// URLs)