diff --git a/apps/server/src/imageMime.ts b/apps/server/src/imageMime.ts index 4cf63079..e00e9b4b 100644 --- a/apps/server/src/imageMime.ts +++ b/apps/server/src/imageMime.ts @@ -44,6 +44,29 @@ export const IMAGE_MIME_TYPE_BY_EXTENSION: Record = { ".webp": "image/webp", }; +/** + * The mime type a byte prefix actually is, for the raster formats a browser + * renders inline, or null when the bytes match no known format. + * + * Extensions lie; this reads the file's own signature. SVG is deliberately + * absent: it is text, so no signature can tell a drawing from a renamed script. + */ +export function detectRasterImageMimeType(bytes: Uint8Array): string | null { + const startsWith = (...signature: ReadonlyArray) => + signature.length <= bytes.length && signature.every((byte, index) => bytes[index] === byte); + const asciiAt = (offset: number, text: string) => + offset + text.length <= bytes.length && + [...text].every((char, index) => bytes[offset + index] === char.charCodeAt(0)); + + if (startsWith(0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a)) return "image/png"; + if (startsWith(0xff, 0xd8, 0xff)) return "image/jpeg"; + if (asciiAt(0, "GIF87a") || asciiAt(0, "GIF89a")) return "image/gif"; + if (asciiAt(0, "RIFF") && asciiAt(8, "WEBP")) return "image/webp"; + if (asciiAt(0, "BM")) return "image/bmp"; + if (asciiAt(4, "ftyp") && (asciiAt(8, "avif") || asciiAt(8, "avis"))) return "image/avif"; + return null; +} + export function parseBase64DataUrl( dataUrl: string, ): { readonly mimeType: string; readonly base64: string } | null { diff --git a/apps/server/src/workspace/Layers/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/Layers/WorkspaceFileSystem.test.ts index 9134b281..e028acba 100644 --- a/apps/server/src/workspace/Layers/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/Layers/WorkspaceFileSystem.test.ts @@ -369,6 +369,79 @@ it.layer(TestLayer)("WorkspaceFileSystemLive", (it) => { }), ); + it.effect("serves an image outside the workspace root when its bytes are one", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const outside = yield* makeTempDir; + const cwd = yield* makeTempDir; + const bytes = new Uint8Array([137, 80, 78, 71, 13, 10, 26, 10, 1, 2, 3]); + yield* fileSystem.writeFile(path.join(outside, "shot.png"), bytes).pipe(Effect.orDie); + const relativePath = path + .relative(cwd, path.join(outside, "shot.png")) + .replaceAll("\\", "/"); + + const result = yield* workspaceFileSystem.readFile({ cwd, relativePath }); + + expect(result).toEqual({ + kind: "image", + relativePath, + mimeType: "image/png", + base64: Buffer.from(bytes).toString("base64"), + size: bytes.length, + }); + }), + ); + + it.effect("still rejects text files outside the workspace root", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const outside = yield* makeTempDir; + const cwd = yield* makeTempDir; + yield* fileSystem + .writeFileString(path.join(outside, "secret.txt"), "secret") + .pipe(Effect.orDie); + + const error = yield* workspaceFileSystem + .readFile({ + cwd, + relativePath: path + .relative(cwd, path.join(outside, "secret.txt")) + .replaceAll("\\", "/"), + }) + .pipe(Effect.flip); + + expect(error._tag).toBe("WorkspacePathOutsideRootError"); + }), + ); + + it.effect("rejects an outside-root .png whose bytes are not an image", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const outside = yield* makeTempDir; + const cwd = yield* makeTempDir; + yield* fileSystem + .writeFileString(path.join(outside, "not-really.png"), "#!/bin/sh\nrm -rf /\n") + .pipe(Effect.orDie); + + const error = yield* workspaceFileSystem + .readFile({ + cwd, + relativePath: path + .relative(cwd, path.join(outside, "not-really.png")) + .replaceAll("\\", "/"), + }) + .pipe(Effect.flip); + + expect(error._tag).toBe("WorkspacePathOutsideRootError"); + }), + ); + it.effect("rejects symlinks that escape the workspace root", () => Effect.gen(function* () { const workspaceFileSystem = yield* WorkspaceFileSystem; diff --git a/apps/server/src/workspace/Layers/WorkspaceFileSystem.ts b/apps/server/src/workspace/Layers/WorkspaceFileSystem.ts index 76e0c007..dcaf7c9d 100644 --- a/apps/server/src/workspace/Layers/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/Layers/WorkspaceFileSystem.ts @@ -8,7 +8,11 @@ import * as Path from "effect/Path"; import type { ProjectReadFileResult, ProjectWriteFileResult } from "@threadlines/contracts"; -import { IMAGE_MIME_TYPE_BY_EXTENSION, SAFE_IMAGE_FILE_EXTENSIONS } from "../../imageMime.ts"; +import { + detectRasterImageMimeType, + IMAGE_MIME_TYPE_BY_EXTENSION, + SAFE_IMAGE_FILE_EXTENSIONS, +} from "../../imageMime.ts"; import { WorkspaceFileSystem, WorkspaceFileSystemError, @@ -127,13 +131,71 @@ export const makeWorkspaceFileSystem = Effect.gen(function* () { }), ); - const readFile: WorkspaceFileSystemShape["readFile"] = Effect.fn("WorkspaceFileSystem.readFile")( - function* (input) { - const target = yield* workspacePaths.resolveRelativePathWithinRoot({ + /** + * Serves a target that resolved outside the workspace root, but only when its + * bytes really are a raster image. + * + * Agents save screenshots wherever the OS puts temp files, so a chat + * reference to one is ordinary and refusing it is exactly what leaves the + * picture blank. A client authenticated to this server already drives an + * agent with shell access on this machine, so handing it verified image bytes + * grants no capability it lacked. Everything else -- text, SVG (text too, so + * a renamed script sniffs as nothing), a `.png` that is not one, a missing + * path -- stays refused exactly as before. + */ + const readImageOutsideRoot = Effect.fn("WorkspaceFileSystem.readImageOutsideRoot")(function* ( + input: { readonly cwd: string; readonly relativePath: string }, + absolutePath: string, + ) { + const rejectOutsideRoot = () => + new WorkspacePathOutsideRootError({ workspaceRoot: input.cwd, relativePath: input.relativePath, }); + const targetStat = yield* fileSystem + .stat(absolutePath) + .pipe(Effect.catch(() => Effect.succeed(null))); + if ( + targetStat === null || + targetStat.type !== "File" || + Number(targetStat.size) > WORKSPACE_IMAGE_READ_MAX_BYTES + ) { + return yield* rejectOutsideRoot(); + } + + const bytes = yield* fileSystem + .readFile(absolutePath) + .pipe(Effect.catch(() => Effect.succeed(null))); + const mimeType = bytes === null ? null : detectRasterImageMimeType(bytes); + if (bytes === null || mimeType === null) { + return yield* rejectOutsideRoot(); + } + + return { + kind: "image", + relativePath: input.relativePath, + mimeType, + base64: Buffer.from(bytes).toString("base64"), + size: bytes.length, + } satisfies ProjectReadFileResult; + }); + + const readFile: WorkspaceFileSystemShape["readFile"] = Effect.fn("WorkspaceFileSystem.readFile")( + function* (input) { + const target = yield* workspacePaths + .resolveRelativePathWithinRoot({ + workspaceRoot: input.cwd, + relativePath: input.relativePath, + }) + .pipe(Effect.catch(() => Effect.succeed(null))); + if (target === null) { + return yield* readImageOutsideRoot( + input, + path.resolve(input.cwd, input.relativePath.trim()), + ); + } + // Lexical checks above cannot see symlinks; compare real paths so reads // never follow a link out of the workspace root. const rootRealPath = yield* fileSystem @@ -161,10 +223,7 @@ export const makeWorkspaceFileSystem = Effect.gen(function* () { realRelativePath.startsWith(`..${path.sep}`) || path.isAbsolute(realRelativePath) ) { - return yield* new WorkspacePathOutsideRootError({ - workspaceRoot: input.cwd, - relativePath: input.relativePath, - }); + return yield* readImageOutsideRoot(input, targetRealPath); } // Same NotFound handling as above: the file can vanish between the diff --git a/apps/web/src/components/ChatMarkdown.browser.tsx b/apps/web/src/components/ChatMarkdown.browser.tsx index 1ea2ca7b..99cc3bfa 100644 --- a/apps/web/src/components/ChatMarkdown.browser.tsx +++ b/apps/web/src/components/ChatMarkdown.browser.tsx @@ -2,6 +2,8 @@ import "../index.css"; import { scopeThreadRef } from "@threadlines/client-runtime"; import { EnvironmentId, type EnvironmentApi, ThreadId } from "@threadlines/contracts"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { ReactElement, ReactNode } from "react"; import { page } from "vite-plus/test/browser"; import { afterEach, describe, expect, it, vi } from "vite-plus/test"; import { render } from "vitest-browser-react"; @@ -63,6 +65,26 @@ const CHAT_MARKDOWN_THREAD_REF = scopeThreadRef( CHAT_MARKDOWN_THREAD_ID, ); +// The inline image loader reads files through react-query, so those renders +// need the provider the app root supplies. +function renderWithQueryClient(ui: ReactElement) { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false, gcTime: 0 } }, + }); + return render(ui, { + wrapper: ({ children }: { children: ReactNode }) => ( + {children} + ), + }); +} + +function installReadFileEnvironment(readFile: EnvironmentApi["projects"]["readFile"]) { + __setEnvironmentApiOverrideForTests(CHAT_MARKDOWN_ENVIRONMENT_ID, { + filesystem: { browse: filesystemBrowseMock }, + projects: { readFile }, + } as unknown as EnvironmentApi); +} + function installFilesystemBrowseEnvironment() { __setEnvironmentApiOverrideForTests(CHAT_MARKDOWN_ENVIRONMENT_ID, { filesystem: { browse: filesystemBrowseMock }, @@ -637,6 +659,71 @@ describe("ChatMarkdown", () => { } }); + it("shows a picture for an image path an agent wrote in prose", async () => { + const pixelBase64 = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+/p9sAAAAASUVORK5CYII="; + const readFile = vi.fn(async (_input: { cwd: string; relativePath: string }) => ({ + kind: "image" as const, + relativePath: "../Temp/shot.png", + mimeType: "image/png", + base64: pixelBase64, + size: 70, + })); + installReadFileEnvironment(readFile as unknown as EnvironmentApi["projects"]["readFile"]); + + const screen = await renderWithQueryClient( + , + ); + + try { + const thumbnail = page.getByRole("img", { name: "shot.png" }); + await expect.element(thumbnail).toBeInTheDocument(); + await expect + .element(thumbnail) + .toHaveAttribute("src", `data:image/png;base64,${pixelBase64}`); + // The chip stays under the picture and still opens the file. + await expect.element(page.getByRole("link", { name: "shot.png" })).toBeInTheDocument(); + expect(readFile.mock.calls[0]?.[0]).toEqual({ + cwd: "/tmp/project", + relativePath: "../Temp/shot.png", + }); + } finally { + await screen.unmount(); + } + }); + + it("shows only the chip when a referenced image is gone", async () => { + const readFile = vi.fn(async () => ({ + kind: "missing" as const, + relativePath: "shots/gone.png", + })); + installReadFileEnvironment(readFile as unknown as EnvironmentApi["projects"]["readFile"]); + + const screen = await renderWithQueryClient( + , + ); + + try { + await expect.element(page.getByRole("link", { name: "before" })).toBeInTheDocument(); + await vi.waitFor(() => { + expect(readFile).toHaveBeenCalled(); + }); + // Only the chip's file-type glyph; no thumbnail and no error box. + expect(document.querySelector('img[alt="before"]')).toBeNull(); + expect(document.querySelector('button[aria-label="Preview before"]')).toBeNull(); + } finally { + await screen.unmount(); + } + }); + it("wraps long fenced text and shows copy feedback", async () => { const code = `Please run ${"a-very-long-unbroken-value".repeat(20)} when ready.`; const writeText = vi.fn(async () => undefined); diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 4a3ed918..26b84e02 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -72,6 +72,8 @@ import { useMarkdownFileLinkKinds, } from "../hooks/useMarkdownFileLinkKinds"; import { cn } from "../lib/utils"; +import { isImageFilePath } from "../lib/imageFilePaths"; +import { LocalImageThumbnail } from "./chat/LocalImageThumbnail"; import { parseCodexInlineVisualizations } from "../lib/codexInlineVisualization"; import { CodexInlineVisualization } from "./chat/CodexInlineVisualization"; @@ -200,6 +202,8 @@ function MarkdownCodeBlock({ code, children }: { code: string; children: ReactNo /** Everything a markdown renderer needs from the document around it. */ interface MarkdownDocumentContextValue { readonly cwd: string | undefined; + /** Environment whose `projects.readFile` RPC backs inline image previews. */ + readonly environmentId: EnvironmentId | undefined; readonly threadRef: ScopedThreadRef | null; readonly resolvedTheme: "light" | "dark"; readonly themeName: DiffThemeName; @@ -224,6 +228,7 @@ interface MarkdownDocumentContextValue { */ const MarkdownDocumentContext = createContext({ cwd: undefined, + environmentId: undefined, threadRef: null, resolvedTheme: "dark", themeName: resolveDiffThemeName("dark"), @@ -463,11 +468,108 @@ function MarkdownListItem({ node: _node, children, ...props }: MarkdownRendererP return
  • {renderSkillInlineMarkdownChildren(children, inlineContext)}
    • ; } +/** + * A referenced image, shown as a picture with its file chip underneath. + * + * The chip is the constant: it is what an image reference has always looked + * like, it still opens the file viewer, and it is all that is left while the + * bytes are in flight or when the file is gone. The picture is the addition. + */ +function MarkdownImageFigure({ + filePath, + name, + children, +}: { + filePath: string; + name: string; + children: ReactNode; +}) { + const { cwd, environmentId } = useContext(MarkdownDocumentContext); + return ( + + + {children} + + ); +} + +/** + * `![alt](path)` where the destination is a file on the agent's machine. + * A plain `` would ask the browser to load a `file://` url it will always + * refuse; http(s) and data sources still render as ordinary images. + */ +function MarkdownImage({ node: _node, src, alt, ...props }: MarkdownRendererProps<"img">) { + const { cwd, resolvedTheme, searchHighlightQuery, fileLinkKindByPath } = + useContext(MarkdownDocumentContext); + const source = typeof src === "string" ? src : undefined; + const fileLinkMeta = + source && !searchHighlightQuery?.trim() ? resolveMarkdownFileLinkMeta(source, cwd) : null; + if (!fileLinkMeta || !isImageFilePath(fileLinkMeta.filePath)) { + return ; + } + const name = alt && alt.length > 0 ? alt : fileLinkMeta.basename; + return ( + + + + ); +} + +/** + * An image path written bare in prose (`C:\Users\me\AppData\Local\Temp\ui.png`). + * Only reached for settled, unhighlighted text; see {@link findBareImagePaths}. + */ +function MarkdownBareImagePath({ rawPath }: { rawPath: string }) { + const { cwd, resolvedTheme, fileLinkKindByPath, fileLinkParentSuffixByPath } = + useContext(MarkdownDocumentContext); + const fileLinkMeta = resolveMarkdownFileLinkMeta(rawPath, cwd); + if (!fileLinkMeta) { + return <>{rawPath}; + } + return ( + + + + ); +} + +/** Module-level so the inline pass never changes component identity. */ +const renderBareImagePath: NonNullable = (input) => ( + +); + function MarkdownAnchor({ node: _node, href, children, ...props }: MarkdownRendererProps<"a">) { const { cwd, threadRef, resolvedTheme, + searchHighlightQuery, markdownFileLinkMetaByHref, fileLinkKindByPath, fileLinkParentSuffixByPath, @@ -497,7 +599,7 @@ function MarkdownAnchor({ node: _node, href, children, ...props }: MarkdownRende ); } - return ( + const chip = ( ); + if (!isImageFilePath(fileLinkMeta.filePath) || searchHighlightQuery?.trim()) { + return chip; + } + return ( + + {chip} + + ); } /** @@ -567,7 +677,7 @@ function MarkdownCode({ ? inlineCodeFileLinkMetaBySpan.get(inlineFileReference) : undefined; if (inlineFileReference && inlineFileLinkMeta) { - return ( + const chip = ( ); + if (!isImageFilePath(inlineFileLinkMeta.filePath)) { + return chip; + } + return ( + + {chip} + + ); } if (className || !text || !parseChatFileReference(text)) { return ( @@ -644,11 +765,12 @@ function MarkdownPre({ node: _node, children, ...props }: MarkdownRendererProps< } /** - * An image the text points at. A host bot links images that later go missing, - * and a broken-image glyph says nothing; the alt text at least says what was - * meant to be there. + * An image with an http(s) or data source, the case {@link MarkdownImage} does + * not turn into a local thumbnail. A host bot links images that later go + * missing, and a broken-image glyph says nothing; the alt text at least says + * what was meant to be there. */ -function MarkdownImage({ alt, src, ...rest }: React.ComponentProps<"img">) { +function RemoteMarkdownImage({ alt, src, ...rest }: React.ComponentProps<"img">) { const [failed, setFailed] = useState(false); if (failed || !src) { return alt ? {alt} : null; @@ -1359,9 +1481,17 @@ function ChatMarkdownDocument({ }, [cwd], ); + // A streaming tail is re-parsed on every delta, so a path halfway through + // being typed would fetch a file that does not exist yet; bare paths only + // become pictures once the block has settled. const inlineContext = useMemo( - () => ({ skills, searchHighlightQuery, threadRef }), - [searchHighlightQuery, skills, threadRef], + () => ({ + skills, + searchHighlightQuery, + threadRef, + ...(isStreaming || searchHighlightQuery?.trim() ? {} : { renderBareImagePath }), + }), + [isStreaming, searchHighlightQuery, skills, threadRef], ); // Rebuilt whenever the document's data changes, which on a streaming tail is // every delta. That is fine and is the point: the renderers re-render, they @@ -1369,6 +1499,7 @@ function ChatMarkdownDocument({ const documentContext = useMemo( () => ({ cwd, + environmentId, threadRef, resolvedTheme, themeName: diffThemeName, @@ -1382,6 +1513,7 @@ function ChatMarkdownDocument({ }), [ cwd, + environmentId, diffThemeName, fileLinkKindByPath, fileLinkParentSuffixByPath, diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index d4570079..96d0a9bc 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -227,7 +227,10 @@ import { type PickedElementContextDraft, } from "../lib/pickedElementContext"; import type { ThreadBackgroundRunItem } from "./chat/threadActivity"; -import { type ExpandedImagePreview } from "./chat/ExpandedImagePreview"; +import { + type ExpandedImagePreview, + setActiveExpandedImageOpener, +} from "./chat/ExpandedImagePreview"; import { FilePreviewDialog, type FilePreviewRequest } from "./chat/FilePreviewDialog"; import { NoActiveThreadState } from "./NoActiveThreadState"; import { resolveEffectiveEnvMode, resolveEnvironmentOptionLabel } from "./BranchToolbar.logic"; @@ -6452,6 +6455,12 @@ export default function ChatView(props: ChatViewProps) { const onExpandTimelineImage = useCallback((preview: ExpandedImagePreview) => { setExpandedImage(preview); }, []); + // Inline pictures in chat markdown are rendered far below here and open the + // same dialog; registering the opener beats drilling it through every layer. + useEffect(() => { + setActiveExpandedImageOpener(onExpandTimelineImage); + return () => setActiveExpandedImageOpener(null); + }, [onExpandTimelineImage]); /** A "view diff" link in the conversation opens or retargets the one Diff * tab, leaving the rest of the strip alone. */ const onOpenTurnDiff = useCallback( diff --git a/apps/web/src/components/chat/ExpandedImagePreview.tsx b/apps/web/src/components/chat/ExpandedImagePreview.tsx index 565dde98..09fe3e10 100644 --- a/apps/web/src/components/chat/ExpandedImagePreview.tsx +++ b/apps/web/src/components/chat/ExpandedImagePreview.tsx @@ -30,3 +30,28 @@ export function buildExpandedImagePreview( index: selectedIndex, }; } + +let activeExpandedImageOpener: ((preview: ExpandedImagePreview) => void) | null = null; + +/** + * Registered by the view that owns the full-screen image dialog. + * + * Same reasoning as `setActiveFileViewerContext`: a picture can be rendered + * far below the dialog's owner (a chat markdown thumbnail inside a subagent + * excerpt), and prop-drilling the opener through every renderer in between + * buys nothing. + */ +export function setActiveExpandedImageOpener( + opener: ((preview: ExpandedImagePreview) => void) | null, +): void { + activeExpandedImageOpener = opener; +} + +/** Opens the full-screen viewer; false when no view owns one right now. */ +export function openExpandedImagePreview(preview: ExpandedImagePreview): boolean { + if (!activeExpandedImageOpener) { + return false; + } + activeExpandedImageOpener(preview); + return true; +} diff --git a/apps/web/src/components/chat/LocalImageThumbnail.tsx b/apps/web/src/components/chat/LocalImageThumbnail.tsx new file mode 100644 index 00000000..4681c539 --- /dev/null +++ b/apps/web/src/components/chat/LocalImageThumbnail.tsx @@ -0,0 +1,68 @@ +import type { EnvironmentId } from "@threadlines/contracts"; +import { memo } from "react"; + +import { openFileInActiveViewer } from "../../fileViewerStore"; +import { useLocalImagePreview } from "../../hooks/useLocalImagePreview"; +import { cn } from "../../lib/utils"; +import { openExpandedImagePreview } from "./ExpandedImagePreview"; + +/** The one thumbnail shape every chat surface shows a local image at. */ +const THUMBNAIL_BUTTON_CLASS_NAME = + "block max-w-[420px] cursor-zoom-in overflow-hidden rounded-lg border border-border/80 bg-background/70"; + +interface LocalImageThumbnailProps { + readonly environmentId: EnvironmentId | undefined; + readonly cwd: string | undefined; + /** Absolute or workspace-relative path to the image on the agent's machine. */ + readonly filePath: string; + /** Alt text and the caption in the expanded viewer. */ + readonly name: string; + readonly className?: string | undefined; +} + +/** + * A picture for a path an agent referred to, or nothing at all. + * + * Renders nothing while the bytes are in flight and nothing when they never + * arrive, so a reference whose file is gone reads exactly as it did before the + * preview existed rather than growing an error box. Whatever the caller renders + * alongside (a file chip) stays the visible thing in both cases. + */ +export const LocalImageThumbnail = memo(function LocalImageThumbnail({ + environmentId, + cwd, + filePath, + name, + className, +}: LocalImageThumbnailProps) { + const preview = useLocalImagePreview({ environmentId, cwd, path: filePath }); + const dataUrl = preview.status === "ready" ? preview.dataUrl : undefined; + if (!dataUrl) { + return null; + } + + return ( + + ); +}); diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx index 371f3917..e80302b1 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -147,6 +147,7 @@ import { import { formatWorkspaceRelativePath } from "../../filePathDisplay"; import { formatProviderDriverKindLabel } from "../../providerModels"; import { useCopyToClipboard } from "~/hooks/useCopyToClipboard"; +import { useLocalImagePreview } from "~/hooks/useLocalImagePreview"; import type { ParsedTranscriptHighlightContextEntry, TranscriptHighlightContextSelection, @@ -1804,6 +1805,9 @@ type TimelineImagePreviewItem = { id: string; name: string; previewUrl?: string; + /** Set instead of `previewUrl` when the provider only named the file; the + * grid loads it over the workspace RPC. */ + path?: string; }; const TimelineRowContent = memo(function TimelineRowContent({ row }: { row: TimelineRow }) { @@ -2063,7 +2067,6 @@ const TimelineImagePreviewGrid = memo(function TimelineImagePreviewGrid(props: { className?: string | undefined; imageClassName?: string | undefined; }) { - const ctx = use(TimelineRowCtx); if (props.images.length === 0) { return null; } @@ -2077,38 +2080,70 @@ const TimelineImagePreviewGrid = memo(function TimelineImagePreviewGrid(props: { )} > {props.images.map((image) => ( -
    - {image.previewUrl ? ( - - ) : ( -
    - {image.name} -
    - )} -
    + image={image} + images={props.images} + imageClassName={props.imageClassName} + /> ))} ); }); +/** + * One tile in the grid. A row that only named its image (a Codex `view_image`, + * a Claude `Read` of a screenshot) loads the bytes here, on mount — the + * timeline is virtualized, so only rows the reader has actually reached ask for + * anything. Until they arrive, and forever if they never do, the tile shows the + * file name exactly as it did before. + */ +function TimelineImagePreviewTile(props: { + image: TimelineImagePreviewItem; + images: ReadonlyArray; + imageClassName?: string | undefined; +}) { + const ctx = use(TimelineRowCtx); + const loaded = useLocalImagePreview({ + environmentId: ctx.activeThreadEnvironmentId, + cwd: ctx.markdownCwd, + path: props.image.previewUrl ? undefined : props.image.path, + }); + const previewUrl = props.image.previewUrl ?? loaded.dataUrl; + + return ( +
    + {previewUrl ? ( + + ) : ( +
    + {props.image.name} +
    + )} +
    + ); +} + function RevertUserMessageButton({ messageId }: { messageId: MessageId }) { const ctx = use(TimelineRowCtx); const activity = use(TimelineRowActivityCtx); diff --git a/apps/web/src/components/chat/SkillInlineText.tsx b/apps/web/src/components/chat/SkillInlineText.tsx index 90ab3fe6..a3a1e4fb 100644 --- a/apps/web/src/components/chat/SkillInlineText.tsx +++ b/apps/web/src/components/chat/SkillInlineText.tsx @@ -9,7 +9,7 @@ import { SKILL_CHIP_ICON_SVG, } from "../composerInlineChip"; import { splitSearchTextHighlightSegments } from "../../lib/searchTextHighlight"; -import { findBareLocalhostUrls } from "../../markdown-links"; +import { findBareImagePaths, findBareLocalhostUrls } from "../../markdown-links"; import { ChatWebLink } from "./ChatWebLink"; const SKILL_TOKEN_REGEX = /(^|\s)\$([a-zA-Z][a-zA-Z0-9:_-]*)(?=\s|$)/g; @@ -25,6 +25,14 @@ export interface InlineMarkdownContext { readonly skills: ReadonlyArray; readonly searchHighlightQuery?: string | undefined; readonly threadRef?: ScopedThreadRef | null; + /** + * Turns an image path written bare in prose into a preview. Supplied by the + * markdown document, which owns the file chip; absent on a streaming tail (a + * half-typed path would fetch) and while a search hit is highlighted. + */ + readonly renderBareImagePath?: + | ((input: { readonly path: string; readonly key: string }) => ReactNode) + | undefined; } interface InlineToken { @@ -75,6 +83,19 @@ function collectInlineTokens(text: string, context: InlineMarkdownContext): Inli }); } + if (context.renderBareImagePath) { + for (const match of findBareImagePaths(text)) { + tokens.push({ + start: match.start, + end: match.end, + node: context.renderBareImagePath({ + path: match.text, + key: `image:${match.start}`, + }), + }); + } + } + tokens.sort((left, right) => left.start - right.start); const ordered: InlineToken[] = []; diff --git a/apps/web/src/fileViewerStore.ts b/apps/web/src/fileViewerStore.ts index ed9d8142..7e486086 100644 --- a/apps/web/src/fileViewerStore.ts +++ b/apps/web/src/fileViewerStore.ts @@ -12,6 +12,8 @@ import { create } from "zustand"; import type { EnvironmentId, ScopedThreadRef } from "@threadlines/contracts"; import { isWindowsAbsolutePath } from "@threadlines/shared/path"; +import { isImageFilePath } from "./lib/imageFilePaths"; + export interface FileViewerContext { environmentId: EnvironmentId; /** Workspace root (or worktree path) that relative file paths resolve against. */ @@ -378,6 +380,55 @@ export function isPathWithinCwd(targetPath: string, cwd: string): boolean { return relativePathWithinCwdOrRoot(targetPath, cwd) !== null; } +/** + * The `relativePath` the `projects.readFile` RPC takes for a target, which may + * sit outside the workspace root — agents save screenshots to temp directories, + * and the server serves those when the bytes are a verified image. + * + * Inside the root this is the plain workspace-relative path; outside it walks + * up with `../`. Null when no relative path exists at all (a different Windows + * drive), since there is nothing the server could resolve. + */ +export function workspaceReadRelativePath(targetPath: string, cwd: string): string | null { + const inside = relativePathWithinCwdOrRoot(targetPath, cwd); + if (inside) { + return inside; + } + + const target = normalizeFileViewerPath(targetPath).replace(/\/+$/u, ""); + const root = normalizeFileViewerPath(cwd).replace(/\/+$/u, ""); + if (target.length === 0 || root.length === 0) { + return null; + } + if (!target.startsWith("/") && !isWindowsAbsolutePath(target)) { + // Already relative to the workspace root; the server resolves it as-is. + return target; + } + + const targetSegments = target.split("/"); + const rootSegments = root.split("/"); + const comparableTarget = normalizePathForComparison(target).split("/"); + const comparableRoot = normalizePathForComparison(root).split("/"); + let shared = 0; + while ( + shared < comparableTarget.length && + shared < comparableRoot.length && + comparableTarget[shared] === comparableRoot[shared] + ) { + shared += 1; + } + // Nothing in common means separate roots (`D:/` against `C:/`), where no + // number of `..` segments ever reaches the target. + if (shared === 0) { + return null; + } + + const upwards = Array.from({ length: rootSegments.length - shared }, () => ".."); + const downwards = targetSegments.slice(shared); + const relativePath = [...upwards, ...downwards].join("/"); + return relativePath.length > 0 ? relativePath : null; +} + function resolveViewerWorkspacePath(targetPath: string, cwd: string): string | null { const normalizedTarget = normalizeFileViewerPath(targetPath); const isAbsolute = @@ -646,7 +697,12 @@ export function openFileInViewer(input: OpenFileInViewerInput): boolean { line = Number.parseInt(lineSuffix[1] ?? "", 10) || undefined; } } - const relativePath = resolveViewerWorkspacePath(targetPath, input.cwd); + // An image outside the root still opens here: the server serves verified + // image bytes for an escaping path, so the viewer can show the picture + // instead of bouncing the click to an external editor. + const relativePath = + resolveViewerWorkspacePath(targetPath, input.cwd) ?? + (isImageFilePath(targetPath) ? workspaceReadRelativePath(targetPath, input.cwd) : null); if (!relativePath) { return false; } diff --git a/apps/web/src/hooks/useLocalImagePreview.ts b/apps/web/src/hooks/useLocalImagePreview.ts new file mode 100644 index 00000000..0eeb45ba --- /dev/null +++ b/apps/web/src/hooks/useLocalImagePreview.ts @@ -0,0 +1,61 @@ +import type { EnvironmentId } from "@threadlines/contracts"; +import { useQuery } from "@tanstack/react-query"; + +import { workspaceReadRelativePath } from "../fileViewerStore"; +import { isImageFilePath } from "../lib/imageFilePaths"; +import { projectReadFileQueryOptions } from "../lib/projectReactQuery"; + +export interface LocalImagePreview { + readonly status: "loading" | "ready" | "unavailable"; + readonly dataUrl?: string | undefined; +} + +const UNAVAILABLE: LocalImagePreview = { status: "unavailable" }; +const LOADING: LocalImagePreview = { status: "loading" }; + +/** + * The one way a chat surface turns a path an agent mentioned into pixels. + * + * Bytes travel over the `projects.readFile` WebSocket RPC and nothing else: a + * relay-paired phone has no HTTP route to the server, so an `` pointed + * at one would simply never load. Going through the shared react-query cache + * also means the same screenshot cited in prose, in a link, and on a tool row + * is fetched once. + * + * Every failure -- missing file, binary, text, outside-root refusal, RPC error + * -- is one `unavailable`. A reference to a file that has since been deleted is + * an ordinary thing in a transcript, not an error worth a message. + */ +export function useLocalImagePreview(input: { + readonly environmentId: EnvironmentId | undefined; + readonly cwd: string | undefined; + /** Absolute, `../`-relative, or workspace-relative path to the image. */ + readonly path: string | undefined; +}): LocalImagePreview { + const relativePath = + input.cwd && input.path && isImageFilePath(input.path) + ? workspaceReadRelativePath(input.path, input.cwd) + : null; + const enabled = Boolean(input.environmentId && input.cwd && relativePath); + const query = useQuery({ + ...projectReadFileQueryOptions({ + environmentId: input.environmentId ?? null, + cwd: input.cwd ?? null, + relativePath, + enabled, + }), + // A refused or unreadable path fails the same way every time, and a long + // transcript can hold many of them; retrying each one is pure traffic. + retry: false, + }); + + if (!enabled) { + return UNAVAILABLE; + } + if (query.data) { + return query.data.kind === "image" + ? { status: "ready", dataUrl: `data:${query.data.mimeType};base64,${query.data.base64}` } + : UNAVAILABLE; + } + return query.isError ? UNAVAILABLE : LOADING; +} diff --git a/apps/web/src/lib/imageFilePaths.ts b/apps/web/src/lib/imageFilePaths.ts new file mode 100644 index 00000000..efae7d94 --- /dev/null +++ b/apps/web/src/lib/imageFilePaths.ts @@ -0,0 +1,30 @@ +/** + * Which paths the chat treats as pictures. + * + * One list, shared by every surface that can show an inline preview (markdown + * links, inline-code references, bare paths in prose, tool rows), so a + * screenshot referenced any of those ways is recognised the same way. The + * server keeps its own set for what it is willing to serve; these are only + * about what the client bothers to ask for. + */ +export const IMAGE_FILE_EXTENSIONS: ReadonlySet = new Set([ + "avif", + "bmp", + "gif", + "jpeg", + "jpg", + "png", + "svg", + "webp", +]); + +const IMAGE_FILE_EXTENSION_PATTERN = /\.([A-Za-z0-9]+)$/u; +const POSITION_SUFFIX_PATTERN = /:\d+(?::\d+)?$/u; + +/** Whether a path (optionally carrying a `:line[:column]` suffix) names an image. */ +export function isImageFilePath(value: string): boolean { + const withoutPosition = value.trim().replace(POSITION_SUFFIX_PATTERN, ""); + const withoutQuery = withoutPosition.split(/[?#]/u)[0] ?? withoutPosition; + const extension = IMAGE_FILE_EXTENSION_PATTERN.exec(withoutQuery)?.[1]; + return extension !== undefined && IMAGE_FILE_EXTENSIONS.has(extension.toLowerCase()); +} diff --git a/apps/web/src/markdown-links.test.ts b/apps/web/src/markdown-links.test.ts index 60a4fc1e..3cb737c7 100644 --- a/apps/web/src/markdown-links.test.ts +++ b/apps/web/src/markdown-links.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vite-plus/test"; import { + findBareImagePaths, findBareLocalhostUrls, localhostUrlFromText, resolveMarkdownFileLinkMeta, @@ -223,3 +224,29 @@ describe("localhostUrlFromText", () => { expect(localhostUrlFromText("example.com:5173")).toBeNull(); }); }); + +describe("findBareImagePaths", () => { + it("finds unambiguous image paths written in prose", () => { + const windowsPath = String.raw`C:\Users\me\AppData\Local\Temp\ui.png`; + expect( + findBareImagePaths(`Saved to ${windowsPath} and /tmp/before.jpg, plus ./docs/after.webp`).map( + (match) => match.text, + ), + ).toEqual([windowsPath, "/tmp/before.jpg", "./docs/after.webp"]); + }); + + it("drops sentence punctuation that is not part of the path", () => { + expect(findBareImagePaths("See ~/shots/home.png.").map((match) => match.text)).toEqual([ + "~/shots/home.png", + ]); + expect(findBareImagePaths("(../out/diff.gif)").map((match) => match.text)).toEqual([ + "../out/diff.gif", + ]); + }); + + it("ignores bare names, non-image paths, and paths inside longer runs", () => { + expect(findBareImagePaths("open screenshot.png now")).toEqual([]); + expect(findBareImagePaths("edit ./src/main.ts now")).toEqual([]); + expect(findBareImagePaths("https://example.com/logo.png")).toEqual([]); + }); +}); diff --git a/apps/web/src/markdown-links.ts b/apps/web/src/markdown-links.ts index 2327a4ef..c2733093 100644 --- a/apps/web/src/markdown-links.ts +++ b/apps/web/src/markdown-links.ts @@ -1,4 +1,5 @@ import { formatWorkspaceRelativePath } from "./filePathDisplay"; +import { isImageFilePath } from "./lib/imageFilePaths"; import { resolvePathLinkTarget, splitPathAndPosition } from "./terminal-links"; const WINDOWS_DRIVE_PATH_PATTERN = /^[A-Za-z]:[\\/]/; @@ -131,6 +132,51 @@ export function localhostUrlFromText(text: string): string | null { return scheme ? trimmed : `http://${trimmed}`; } +/** + * A path shaped like nothing else, so it can be picked out of running prose. + * + * Only the four forms that cannot be mistaken for an English word: a Windows + * drive (`C:\`, `C:/`), a POSIX absolute (`/`), a home path (`~/`), and an + * explicit relative (`./`, `../`). A bare `screenshot.png` is deliberately not + * here — it is as likely to be the name of a file being discussed as a file to + * go and fetch, and guessing wrong means an unexpected picture in the middle of + * a sentence. + */ +const BARE_IMAGE_PATH_REGEX = /(?:[A-Za-z]:[\\/]|~[\\/]|\.{1,2}[\\/]|\/)[^\s"'`<>|*?]+/g; +/** Characters that make a path run part of something longer. */ +const BARE_PATH_BOUNDARY_BLOCKERS = /[A-Za-z0-9._\-/\\:@]/; + +export interface BareImagePathMatch { + readonly start: number; + readonly end: number; + /** The path as written, so the transcript still reads the way it was sent. */ + readonly text: string; +} + +/** + * Every bare image path in a stretch of plain text, in order. + * + * Scoped to plain text on purpose: paths inside links and backticks are already + * recognised by the markdown pre-pass, and this only has to cover the case + * where an agent simply typed where it put the screenshot. + */ +export function findBareImagePaths(text: string): BareImagePathMatch[] { + const matches: BareImagePathMatch[] = []; + for (const match of text.matchAll(BARE_IMAGE_PATH_REGEX)) { + const start = match.index ?? 0; + const previous = start === 0 ? "" : text[start - 1]!; + if (previous !== "" && BARE_PATH_BOUNDARY_BLOCKERS.test(previous)) { + continue; + } + const raw = trimUrlTrailingPunctuation(match[0]); + if (raw.length === 0 || !isImageFilePath(raw) || !isLikelyPathCandidate(raw)) { + continue; + } + matches.push({ start, end: start + raw.length, text: raw }); + } + return matches; +} + export interface MarkdownFileLinkMeta { filePath: string; targetPath: string; diff --git a/apps/web/src/session-logic.test.ts b/apps/web/src/session-logic.test.ts index 44f0cdef..2e9297d9 100644 --- a/apps/web/src/session-logic.test.ts +++ b/apps/web/src/session-logic.test.ts @@ -2685,6 +2685,81 @@ describe("deriveWorkLogEntries", () => { }); }); + it("carries the file path when a viewed image has no inline bytes", () => { + const activities: OrchestrationThreadActivity[] = [ + makeActivity({ + id: "codex-view-image", + kind: "tool.completed", + summary: "C:\\Users\\wilfr\\AppData\\Local\\Temp\\shot.png", + payload: { + itemType: "image_view", + title: "Image view", + data: { + item: { + id: "iv_789", + path: "C:\\Users\\wilfr\\AppData\\Local\\Temp\\shot.png", + type: "imageView", + }, + }, + }, + }), + ]; + + const [entry] = deriveWorkLogEntries(activities); + expect(entry).toMatchObject({ + itemType: "image_view", + images: [ + { + id: "iv_789", + name: "shot.png", + path: "C:\\Users\\wilfr\\AppData\\Local\\Temp\\shot.png", + }, + ], + }); + expect(entry?.images?.[0]?.previewUrl).toBeUndefined(); + }); + + it("previews the image blocks a screenshot tool returned inline", () => { + const screenshotBase64 = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+/p9sAAAAASUVORK5CYII="; + const activities: OrchestrationThreadActivity[] = [ + makeActivity({ + id: "mcp-screenshot", + kind: "tool.completed", + summary: "Tool call", + payload: { + itemType: "dynamic_tool_call", + title: "browser_screenshot", + data: { + toolName: "browser_screenshot", + input: {}, + result: { + type: "tool_result", + tool_use_id: "toolu_shot", + content: [ + { type: "text", text: "Captured the page." }, + { + type: "image", + source: { type: "base64", media_type: "image/png", data: screenshotBase64 }, + }, + ], + }, + item: { id: "toolu_shot" }, + }, + }, + }), + ]; + + const [entry] = deriveWorkLogEntries(activities); + expect(entry?.images).toEqual([ + { + id: "toolu_shot", + name: "toolu_shot.png", + previewUrl: `data:image/png;base64,${screenshotBase64}`, + }, + ]); + }); + it("extracts changed file paths for file-change tool activities", () => { const activities: OrchestrationThreadActivity[] = [ makeActivity({ diff --git a/apps/web/src/session-logic.ts b/apps/web/src/session-logic.ts index 6a7d1863..710ef1df 100644 --- a/apps/web/src/session-logic.ts +++ b/apps/web/src/session-logic.ts @@ -50,6 +50,7 @@ import { type ExtensionMcpOAuthActionIntent, } from "./mcpAuthStatus"; import { filterSupersededManualContextCompactionActivities } from "./lib/contextCompactionActivities"; +import { isImageFilePath } from "./lib/imageFilePaths"; import type { ChatMessage, @@ -76,7 +77,13 @@ export const PROVIDER_OPTIONS: Array<{ export interface WorkLogImagePreview { id: string; name: string; - previewUrl: string; + /** Directly renderable source (a data, http, or blob url), when the provider + * gave us the bytes. */ + previewUrl?: string; + /** Where the image lives on the agent's machine, when the provider only named + * it. The renderer loads it over the `projects.readFile` RPC; carrying the + * path instead of the bytes keeps screenshots out of the stored event log. */ + path?: string; } export interface ProviderAuthReconnectAction { @@ -3790,6 +3797,52 @@ function extractToolTitle(payload: Record | null): string | nul return semanticToolPresentation(payload)?.title ?? asTrimmedString(payload?.title); } +/** The path a tool row names its image by, wherever the provider put it. */ +function imagePathFromPayload(payload: Record | null): unknown { + const data = asRecord(payload?.data); + const item = asRecord(data?.item); + const input = asRecord(data?.input ?? item?.input); + return ( + item?.savedPath ?? + item?.saved_path ?? + item?.path ?? + data?.savedPath ?? + data?.path ?? + input?.file_path ?? + input?.filePath ?? + input?.path + ); +} + +/** + * Image bytes a tool result carried inline, as `{mimeType, base64}` blocks. + * + * The Claude driver stores the raw `tool_result` block on the activity, so a + * screenshot tool's `{type: "image", source: {type: "base64", ...}}` content + * blocks are already here; reading them is what makes an MCP screenshot row a + * picture without the event log holding a second copy. + */ +function imageBlocksFromPayload( + payload: Record | null, +): Array<{ mimeType: string; base64: string }> { + const data = asRecord(payload?.data); + const item = asRecord(data?.item); + const content = asRecord(data?.result ?? item?.result)?.content; + if (!Array.isArray(content)) { + return []; + } + return content.flatMap((entry) => { + const block = asRecord(entry); + const source = asRecord(block?.source); + if (block?.type !== "image" || source?.type !== "base64") { + return []; + } + const mimeType = asTrimmedString(source.media_type); + const base64 = asTrimmedString(source.data); + return mimeType && base64 ? [{ mimeType, base64 }] : []; + }); +} + function extractWorkLogImages(payload: Record | null): WorkLogImagePreview[] { if (!isImagePreviewPayload(payload)) { return []; @@ -3803,32 +3856,62 @@ function extractWorkLogImages(payload: Record | null): WorkLogI asTrimmedString(data?.id) ?? "generated-image"; const result = asTrimmedString(item?.result ?? data?.result ?? payload?.result); - const path = item?.savedPath ?? item?.saved_path ?? item?.path ?? data?.savedPath ?? data?.path; + const path = imagePathFromPayload(payload); + + const imageBlocks = imageBlocksFromPayload(payload); + if (imageBlocks.length > 0) { + return imageBlocks.map((block, index) => ({ + id: index === 0 ? imageId : `${imageId}:${index}`, + name: generatedImageName({ id: imageId, mimeType: block.mimeType, path }), + previewUrl: `data:${block.mimeType};base64,${block.base64}`, + })); + } + const source = result ? imageSourceFromValue(result, { allowBase64: true }) : imageSourceFromValue(asTrimmedString(path) ?? "", { allowBase64: false }); - if (!source) { - return []; + if (source) { + return [ + { + id: imageId, + name: generatedImageName({ + id: imageId, + mimeType: source.mimeType, + path, + }), + previewUrl: source.previewUrl, + }, + ]; + } + + // A local path is not a source the browser can load, but it is a source the + // renderer can fetch over the workspace RPC. Carrying the path here is what + // turns "Viewed image" and Claude's `Read` of a screenshot into a picture. + const localPath = asTrimmedString(path); + if (localPath && isImageFilePath(localPath)) { + return [ + { + id: imageId, + name: generatedImageName({ id: imageId, mimeType: undefined, path }), + path: localPath, + }, + ]; } - return [ - { - id: imageId, - name: generatedImageName({ - id: imageId, - mimeType: source.mimeType, - path, - }), - previewUrl: source.previewUrl, - }, - ]; + return []; } function isImagePreviewPayload(payload: Record | null): boolean { if (extractWorkLogItemType(payload) === "image_view") { return true; } + // A payload carrying image bytes needs no guessing from its name: a + // screenshot tool can be called anything (`browser_screenshot`, + // `take_screenshot`) and none of those words is "image". + if (imageBlocksFromPayload(payload).length > 0) { + return true; + } const data = asRecord(payload?.data); const item = asRecord(data?.item); @@ -3836,9 +3919,7 @@ function isImagePreviewPayload(payload: Record | null): boolean const namespace = asTrimmedString(item?.namespace ?? data?.namespace)?.toLowerCase(); const tool = asTrimmedString(item?.tool ?? data?.tool)?.toLowerCase(); const itemType = asTrimmedString(item?.type ?? data?.type)?.toLowerCase(); - const path = asTrimmedString( - item?.savedPath ?? item?.saved_path ?? item?.path ?? data?.savedPath ?? data?.path, - )?.toLowerCase(); + const path = asTrimmedString(imagePathFromPayload(payload))?.toLowerCase(); return [title, namespace, tool, itemType, path].some( (value) =>