chat: render images in a turn, and stop mangling the ones that were already links

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 <noreply@anthropic.com>
2026-08-17 02:13:28 +01:00
parent 8fa3f7da3e
commit ba0c8600b8
4 changed files with 151 additions and 24 deletions
@@ -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 (
+      <span className="inline-block rounded-lg border border-border bg-muted/40 px-3 py-2 text-xs text-muted-foreground">
+        Image could not be loaded{alt ? `: ${alt}` : ''}
+        <span className="block truncate opacity-60 max-w-[28rem]">{src}</span>
+      </span>
+    );
+  }
+
+  return (
+    <>
+      <img
+        src={src}
+        alt={alt ?? ''}
+        loading="lazy"
+        onError={() => 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"
+      />
+      <Dialog open={open} onOpenChange={setOpen}>
+        {/* 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. */}
+        <DialogContent className="max-w-[95vw] w-auto p-0 bg-transparent border-0 shadow-none">
+          <DialogTitle className="sr-only">{alt || 'Image'}</DialogTitle>
+          <img src={src} alt={alt ?? ''} className="max-h-[90vh] max-w-[95vw] w-auto rounded-lg" />
+        </DialogContent>
+      </Dialog>
+    </>
+  );
+};
@@ -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 = /^<frontmatter>([\s\S]*?)<\/frontmatter>\s*/;

 const CollapsibleBlock = ({ label, content }: { label: string; content: string }) => (
@@ -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)');
+});
@@ -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('');
+}