From ba0c8600b8c20e4391fc33863b340182eed6cb22 Mon Sep 17 00:00:00 2001 From: Andre Padez Date: Mon, 17 Aug 2026 02:13:16 +0100 Subject: [PATCH] chat: render images in a turn, and stop mangling the ones that were already links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remote images were broken in the web chat and the failure looked like a network fault. It was a rewrite. injectImages scans a turn for absolute file paths ending in an image extension and points them at the file browser — good behaviour, since an agent that writes /tmp/shot.png should show it rather than make you go and open it. But 'https://host/images/duck.png' CONTAINS '/images/duck.png', so a perfectly good markdown image became ![duck.png](/api/file-browser/raw?path=//host/images/duck.png) scheme stripped, host read as a directory. Every remote image failed. injectImages now skips regions that are already links — !?[..](..) alongside the code fences it already skipped — and refuses any match preceded by ':' or '/', or starting with '//', since those are the tail or the whole of a URL somebody wrote deliberately. Moved to chat-images.ts with tests, because a regex with three exceptions is exactly the thing that regresses silently. Rendering added with it: markdown images now go through ChatImage, capped at 14rem HIGH — not wide, so a wide screenshot and a tall phone capture take comparable room — and open full size in a dialog on click. A failure names the alt text and the URL, because the useful question is always which host did not answer. Co-Authored-By: Claude Opus 5 --- .../src/apps/Chat/components/ChatImage.tsx | 56 +++++++++++++++++++ .../apps/Chat/components/MessageBubble.tsx | 27 +-------- .../apps/Chat/components/chat-images.test.ts | 44 +++++++++++++++ .../src/apps/Chat/components/chat-images.ts | 48 ++++++++++++++++ 4 files changed, 151 insertions(+), 24 deletions(-) create mode 100644 src/workspaces/officerdev/src/apps/Chat/components/ChatImage.tsx create mode 100644 src/workspaces/officerdev/src/apps/Chat/components/chat-images.test.ts create mode 100644 src/workspaces/officerdev/src/apps/Chat/components/chat-images.ts diff --git a/src/workspaces/officerdev/src/apps/Chat/components/ChatImage.tsx b/src/workspaces/officerdev/src/apps/Chat/components/ChatImage.tsx new file mode 100644 index 00000000..bd876fe4 --- /dev/null +++ b/src/workspaces/officerdev/src/apps/Chat/components/ChatImage.tsx @@ -0,0 +1,56 @@ +import { useState } from 'react'; +import { Dialog, DialogContent, DialogTitle } from '@/components/ui/dialog'; + +/** + * An image inside a chat turn: a thumbnail in the bubble, full size when clicked. + * + * A conversation is made of words, so a picture in one is a reference to something rather than the + * thing itself — capped small enough that a screenshot cannot push the reply off the screen, and + * opened properly when you actually want to look at it. + * + * Capped by HEIGHT rather than width, deliberately: a wide screenshot and a tall phone capture then + * take comparable room, instead of one of them owning the pane. Same rule the mobile client uses. + */ +export const ChatImage = ({ src, alt }: { src?: string; alt?: string }) => { + const [open, setOpen] = useState(false); + const [failed, setFailed] = useState(false); + + if (!src) return null; + + /* + A broken image says which one and where from. + + These URLs come from the model or from a tool, and the interesting question when one fails is + almost always WHICH host did not answer — a machine that is asleep, a tailnet that is down, a link + that expired. A broken-image icon answers none of that. + */ + if (failed) { + return ( + + Image could not be loaded{alt ? `: ${alt}` : ''} + {src} + + ); + } + + return ( + <> + {alt setFailed(true)} + onClick={() => setOpen(true)} + className="max-h-56 w-auto max-w-full rounded-lg border border-border/60 my-1 cursor-zoom-in hover:opacity-90 transition-opacity" + /> + + {/* No chrome and no padding: the picture is the content, and a header would take height from + it on the one screen where height is what you came for. */} + + {alt || 'Image'} + {alt + + + + ); +}; diff --git a/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx b/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx index 7792b624..2654b437 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx @@ -27,10 +27,12 @@ import { getRawUrl } from '../../FileViewer/file-types'; import { useFilesAPI } from '../../../hooks/useFilesAPI'; import { CopyButton } from './CopyButton'; import { CodeBlock } from './CodeBlock'; +import { ChatImage } from './ChatImage'; +import { injectImages } from './chat-images'; import { randomId } from 'helpers/random-id'; /** Shared by the settled bubble and the streaming one, so a block gains nothing when the turn ends. */ -const MD_COMPONENTS = { pre: CodeBlock }; +const MD_COMPONENTS = { pre: CodeBlock, img: ChatImage }; const sanitizeSchema = { ...defaultSchema, @@ -43,29 +45,6 @@ const sanitizeSchema = { }, }; -// Matches absolute image file paths, e.g. /home/user/pic.png or /tmp/photo.jpg -const IMAGE_PATH_RE = /(\/(?:home\/[^/\s]+\/)?[^\s`"'<>\n\r[\]()]+\.(?:png|jpg|jpeg|gif|webp|svg|bmp|ico))/gi; - -function pathToImageUrl(filePath: string): string { - // Strip /home/{username} prefix — the file browser root=home serves from the user's home dir - const relative = filePath.replace(/^\/home\/[^/]+/, '') || '/'; - return getRawUrl(relative, 'home'); -} - -function injectImages(text: string): string { - // Split on fenced code blocks and inline code so we don't touch paths inside backticks - const parts = text.split(/(```[\s\S]*?```|`[^`\n]+`)/g); - return parts - .map((part, i) => { - if (i % 2 === 1) return part; // inside code — leave untouched - return part.replace(IMAGE_PATH_RE, (match) => { - const filename = match.split('/').pop() ?? 'image'; - return `![${filename}](${pathToImageUrl(match)})`; - }); - }) - .join(''); -} - const FRONTMATTER_RE = /^([\s\S]*?)<\/frontmatter>\s*/; const CollapsibleBlock = ({ label, content }: { label: string; content: string }) => ( diff --git a/src/workspaces/officerdev/src/apps/Chat/components/chat-images.test.ts b/src/workspaces/officerdev/src/apps/Chat/components/chat-images.test.ts new file mode 100644 index 00000000..88254512 --- /dev/null +++ b/src/workspaces/officerdev/src/apps/Chat/components/chat-images.test.ts @@ -0,0 +1,44 @@ +import { expect, test, mock } from 'bun:test'; + +mock.module('../../FileViewer/file-types', () => ({ + getRawUrl: (path: string, root: string) => `/api/file-browser/raw?path=${path}&root=${root}`, +})); + +const { injectImages } = await import('./chat-images'); + +test('leaves an existing remote markdown image untouched', () => { + const src = '![Vault](https://atelier.officer.dev/images/aaf40b28.png)'; + expect(injectImages(src)).toBe(src); +}); + +test('leaves a bare remote URL untouched', () => { + const src = 'see https://oas.officer.dev/preview/textcmp.png for the comparison'; + expect(injectImages(src)).toBe(src); +}); + +test('leaves a protocol-relative URL untouched', () => { + const src = 'at //host.example/images/x.png there'; + expect(injectImages(src)).toBe(src); +}); + +test('still turns a bare local path into an image', () => { + expect(injectImages('here it is /home/pastilhas/shot.png ok')).toContain( + '![shot.png](/api/file-browser/raw?path=/shot.png&root=home)', + ); +}); + +test('leaves paths inside inline code alone', () => { + const src = 'run `magick /tmp/a.png out.png` now'; + expect(injectImages(src)).toBe(src); +}); + +test('leaves paths inside a fenced block alone', () => { + const src = '```\ncp /tmp/a.png /tmp/b.png\n```'; + expect(injectImages(src)).toBe(src); +}); + +test('handles a local path and a remote image in the same message', () => { + const out = injectImages('local /tmp/a.png and ![r](https://h/x/y.png)'); + expect(out).toContain('![a.png](/api/file-browser/raw?path=/tmp/a.png&root=home)'); + expect(out).toContain('![r](https://h/x/y.png)'); +}); diff --git a/src/workspaces/officerdev/src/apps/Chat/components/chat-images.ts b/src/workspaces/officerdev/src/apps/Chat/components/chat-images.ts new file mode 100644 index 00000000..326401dc --- /dev/null +++ b/src/workspaces/officerdev/src/apps/Chat/components/chat-images.ts @@ -0,0 +1,48 @@ +import { getRawUrl } from '../../FileViewer/file-types'; + +/** + * Turning bare file paths in a turn into images, without touching anything that is already a link. + * + * An agent that has just written a screenshot says so in prose — "saved to /tmp/shot.png" — and + * showing it beats making you go and open it. That is what injectImages is for. + */ + +/** Absolute image file paths, e.g. `/home/user/pic.png` or `/tmp/photo.jpg`. */ +export const IMAGE_PATH_RE = /(\/(?:home\/[^/\s]+\/)?[^\s`"'<>\n\r[\]()]+\.(?:png|jpg|jpeg|gif|webp|svg|bmp|ico))/gi; + +/* + Regions to leave alone: fenced code, inline code, and markdown links or images that ALREADY exist — + `[text](url)` and `![alt](url)`. + + That last one is not hypothetical. A URL like `https://host/images/duck.png` contains + `/images/duck.png`, which IMAGE_PATH_RE matches happily, so a perfectly good markdown image was + rewritten into `![duck.png](/api/file-browser/raw?path=//host/images/duck.png)` — scheme stripped, + host read as a directory. Every remote image in chat broke, and it looked like a network fault + rather than a rewrite, which is why it survived as long as it did. +*/ +export const SKIP_RE = /(```[\s\S]*?```|`[^`\n]+`|!?\[[^\]]*\]\([^)]*\))/g; + +/** Strip `/home/{username}` — the file browser's `home` root already serves from there. */ +export function pathToImageUrl(filePath: string): string { + const relative = filePath.replace(/^\/home\/[^/]+/, '') || '/'; + return getRawUrl(relative, 'home'); +} + +export function injectImages(text: string): string { + return text + .split(SKIP_RE) + .map((part, i) => { + if (i % 2 === 1) return part; // code, or a link somebody already wrote + return part.replace(IMAGE_PATH_RE, (match, _g, offset: number, whole: string) => { + // A bare path only. Two ways it can be part of a URL instead: + // preceded by `:` or `/` — the tail of `https://host/x.png` + // starting with `//` — a protocol-relative `//host/x.png`, matched whole + // Either way it belongs to whoever wrote it, not to the file browser. + const before = whole[offset - 1]; + if (before === ':' || before === '/' || match.startsWith('//')) return match; + const filename = match.split('/').pop() ?? 'image'; + return `![${filename}](${pathToImageUrl(match)})`; + }); + }) + .join(''); +}