From 04f7a07addab2c3f6e2f51017acf8bdf6eeaf829 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Tue, 5 May 2026 19:39:37 -0700 Subject: [PATCH] feat(canvas/chat): inline image preview + fullscreen lightbox (RFC #2991 PR-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First specialized renderer landing under RFC #2991 — chat attachment preview. Adds the dispatch infrastructure that PR-2 (video/audio) and PR-3 (PDF/text) will extend. Architecture (RFC #2991 Phase 2 design) --------------------------------------- - preview-kind.ts: pure helper that maps mimeType (+ extension fallback for missing/generic MIME) to one of: image | video | audio | pdf | text | file. Single source of truth; the dispatch axis for every attachment renderer. - AttachmentPreview.tsx: SSOT dispatch component. ChatTab no longer imports kind-specific components — it imports AttachmentPreview, which switches on the kind and renders the right child. - AttachmentImage.tsx: inline thumbnail (max 240×180) + click → lightbox. Auth-aware: for platform URIs (workspace: / platform-pending: / etc) the bytes are fetched via JS-injected headers, wrapped in a Blob, served as ObjectURL — bare would not include the cookie/token. - AttachmentLightbox.tsx: shared fullscreen modal (image now; PDF will use it in PR-3). Esc / backdrop click / X button to close, focus trap on close button, focus restoration on close. - AttachmentChip retained as the kind=file fallback. No breaking change for existing renderable shapes. External-workspace coverage --------------------------- The wire shape (ChatAttachment.mimeType + uri) is identical for internal + external workspaces — both go through AgentMessageWriter (PR #2949). External claude-code agents that attach images via send_message_to_user automatically get the new preview surface; no runtime-side change needed. Failure modes ------------- - Fetch failure (404, 403, network) → AttachmentChip fallback so the user still gets a working download. Pinned by tests. - Decoded as non-image (corrupt bytes, wrong Content-Type) → onError on the swaps to AttachmentChip. Pinned by tests. - Non-platform URIs (http/https external image hosts) → skip the auth-fetch flow, use the raw URL via resolveAttachmentHref. Pinned by extension-fallback tests. Tests ----- preview-kind.test.ts (49 cases): - Strict MIME match across image/video/audio/pdf/text/unknown - Extension fallback when MIME is missing or application/octet-stream - URL with query string + fragment → strip before parsing - MIME wins over extension (regression: don't render image-named zip) - SVG is image (not text) despite being XML - Non-canonical MIME like application/javascript → text AttachmentPreview.test.tsx (9 component tests): - Dispatch: kind=file → chip, kind=image → image path - Loading state shows placeholder, NOT chip (proves dispatch routed) - Extension fallback (no mimeType) routes to image path - Fetch fail (404) and network error → fall back to chip - Image success: renders ObjectURL, click opens lightbox - Lightbox: Esc closes, backdrop click closes, content click doesn't - Universal fallback: unknown MIME → chip even when extension hints at a renderable kind Hostile self-review (3 weakest spots, addressed) ------------------------------------------------ 1. auth: bare would NOT include our auth headers. Resolved via fetch+Blob+ObjectURL pattern. Pinned by the image-success test (asserts src === "blob:test-url"). 2. Server-side allowed-roots mismatch: pre-fix tests used /tmp/ paths which the server doesn't allow. Caught when the dispatch test fell into the non-platform path. Updated tests to use /workspace/ subpaths matching templates.go's allowedRoots. 3. Bundle size creep: each kind component adds bytes. Lightbox is currently always-bundled. Lazy-loading is plausible but defer until measured-needed. Verified - tsc --noEmit clean - 168 chat tests green (49 unit + 9 component + 110 pre-existing) PR-2 (video + audio) and PR-3 (PDF + text) extend the dispatch in AttachmentPreview.tsx with their own kind-specific components. Refs RFC #2991. Co-Authored-By: Claude Opus 4.7 (1M context) --- canvas/src/components/tabs/ChatTab.tsx | 6 +- .../components/tabs/chat/AttachmentImage.tsx | 198 ++++++++++++++++++ .../tabs/chat/AttachmentLightbox.tsx | 122 +++++++++++ .../tabs/chat/AttachmentPreview.tsx | 56 +++++ .../chat/__tests__/AttachmentPreview.test.tsx | 165 +++++++++++++++ .../tabs/chat/__tests__/preview-kind.test.ts | 112 ++++++++++ .../src/components/tabs/chat/preview-kind.ts | 154 ++++++++++++++ 7 files changed, 811 insertions(+), 2 deletions(-) create mode 100644 canvas/src/components/tabs/chat/AttachmentImage.tsx create mode 100644 canvas/src/components/tabs/chat/AttachmentLightbox.tsx create mode 100644 canvas/src/components/tabs/chat/AttachmentPreview.tsx create mode 100644 canvas/src/components/tabs/chat/__tests__/AttachmentPreview.test.tsx create mode 100644 canvas/src/components/tabs/chat/__tests__/preview-kind.test.ts create mode 100644 canvas/src/components/tabs/chat/preview-kind.ts diff --git a/canvas/src/components/tabs/ChatTab.tsx b/canvas/src/components/tabs/ChatTab.tsx index 2bc3939f..f343b63c 100644 --- a/canvas/src/components/tabs/ChatTab.tsx +++ b/canvas/src/components/tabs/ChatTab.tsx @@ -8,7 +8,8 @@ import { useCanvasStore, type WorkspaceNodeData } from "@/store/canvas"; import { useSocketEvent } from "@/hooks/useSocketEvent"; import { type ChatMessage, type ChatAttachment, createMessage, appendMessageDeduped } from "./chat/types"; import { uploadChatFiles, downloadChatFile, isPlatformAttachment } from "./chat/uploads"; -import { AttachmentChip, PendingAttachmentPill } from "./chat/AttachmentViews"; +import { PendingAttachmentPill } from "./chat/AttachmentViews"; +import { AttachmentPreview } from "./chat/AttachmentPreview"; import { extractFilesFromTask } from "./chat/message-parser"; import { AgentCommsPanel } from "./chat/AgentCommsPanel"; import { appendActivityLine } from "./chat/activityLog"; @@ -1137,8 +1138,9 @@ function MyChatPanel({ workspaceId, data }: Props) { {msg.attachments && msg.attachments.length > 0 && (
{msg.attachments.map((att, i) => ( - WILL NOT +// include our cookie + Origin headers when the browser loads it — +// even for same-origin canvas-server, the auth chain (cookie + token +// + X-Molecule-Org-Slug header) is JS-injected, not browser-default. +// +// Solution: same auth path the chip download uses. Fetch the bytes +// with the JS auth headers, wrap in a Blob, hand the browser an +// ObjectURL. The image renders from local memory; no second request, +// no auth leakage, no CORS pain. +// +// That same blob URL is what the lightbox shows on click — single +// fetch, cached for the lifetime of the message bubble. +// +// Failure modes +// ------------- +// +// - Fetch fails (404, 403, network) → fall back to AttachmentChip +// (the existing file-pill download flow). The user still gets a +// working download; we just lose the inline preview. +// - Decoded as non-image (server returned wrong Content-Type, or +// bytes are corrupt) → onError handler swaps to AttachmentChip. +// - Bytes too large — no enforcement here; the server caps at 25MB +// per file (chat_files.go), which is too big for a thumbnail but +// acceptable for a chat-attached image. If we hit pain we can +// downscale via canvas, but defer that to v2. + +import { useState, useEffect, useRef } from "react"; +import type { ChatAttachment } from "./types"; +import { downloadChatFile, isPlatformAttachment, resolveAttachmentHref } from "./uploads"; +import { AttachmentLightbox } from "./AttachmentLightbox"; +import { AttachmentChip } from "./AttachmentViews"; + +interface Props { + workspaceId: string; + attachment: ChatAttachment; + onDownload: (a: ChatAttachment) => void; + tone: "user" | "agent"; +} + +type FetchState = + | { kind: "idle" } + | { kind: "loading" } + | { kind: "ready"; blobUrl: string } + | { kind: "error" }; + +export function AttachmentImage({ workspaceId, attachment, onDownload, tone }: Props) { + const [state, setState] = useState({ kind: "idle" }); + const [open, setOpen] = useState(false); + // Track whether we created the ObjectURL so cleanup runs on the + // exact value we minted (state could change between effect setup + // and effect cleanup if a new fetch fires). + const blobUrlRef = useRef(null); + + useEffect(() => { + let cancelled = false; + setState({ kind: "loading" }); + + // For non-platform URIs (http/https external image hosts) we can + // skip the auth fetch — browser loads them directly. We bail out + // of the auth-fetch flow and use the raw URL via resolveAttachmentHref. + if (!isPlatformAttachment(attachment.uri)) { + const href = resolveAttachmentHref(workspaceId, attachment.uri); + if (!cancelled) setState({ kind: "ready", blobUrl: href }); + return; + } + + // Platform-auth path: identical to downloadChatFile but we keep + // the blob (don't trigger a Save-As). Use the same headers it does + // by going through it indirectly — no, downloadChatFile triggers a + // Save-As. Need a separate fetch. + void (async () => { + try { + const href = resolveAttachmentHref(workspaceId, attachment.uri); + const headers: Record = {}; + // Read the same env var downloadChatFile reads — single source + // of truth would be cleaner; refactor opportunity for PR-2 if + // we add the same path to AttachmentVideo. + const adminToken = process.env.NEXT_PUBLIC_ADMIN_TOKEN; + if (adminToken) headers["Authorization"] = `Bearer ${adminToken}`; + const slug = getTenantSlug(); + if (slug) headers["X-Molecule-Org-Slug"] = slug; + const res = await fetch(href, { + headers, + credentials: "include", + signal: AbortSignal.timeout(30_000), + }); + if (!res.ok) { + if (!cancelled) setState({ kind: "error" }); + return; + } + const blob = await res.blob(); + const url = URL.createObjectURL(blob); + blobUrlRef.current = url; + if (cancelled) { + URL.revokeObjectURL(url); + return; + } + setState({ kind: "ready", blobUrl: url }); + } catch { + if (!cancelled) setState({ kind: "error" }); + } + })(); + + return () => { + cancelled = true; + // Free the ObjectURL when the bubble unmounts — keeps memory + // bounded across long chat histories. + if (blobUrlRef.current) { + URL.revokeObjectURL(blobUrlRef.current); + blobUrlRef.current = null; + } + }; + }, [workspaceId, attachment.uri]); + + // Failure → render the existing file chip. Maintains the download + // affordance even if preview fails; the user never gets stuck. + if (state.kind === "error") { + return ; + } + + // Loading → small placeholder pill so the bubble doesn't reflow + // when the image lands. Sized to roughly the thumbnail's aspect + // ratio guess (a 240x180 box) so the layout is stable. + if (state.kind === "loading" || state.kind === "idle") { + return ( +
+ ); + } + + // Ready → inline thumbnail with click handler. The img has its + // own onError so a corrupt blob (server returned the right size + // but invalid bytes) falls through to the chip too. + return ( + <> + + setOpen(false)} + ariaLabel={`Preview of ${attachment.name}`} + > + {attachment.name} + + + ); +} + +// Internal helper — duplicated from uploads.ts (it's not exported +// there). Kept local so this component doesn't reach into private +// surface; if AttachmentVideo / AttachmentPDF in PR-2/PR-3 also need +// it, lift to an exported helper at that point (the third-caller +// rule). +function getTenantSlug(): string | null { + if (typeof window === "undefined") return null; + const host = window.location.hostname; + // Tenant subdomain shape: .moleculesai.app + const m = host.match(/^([^.]+)\.moleculesai\.app$/); + return m ? m[1] : null; +} diff --git a/canvas/src/components/tabs/chat/AttachmentLightbox.tsx b/canvas/src/components/tabs/chat/AttachmentLightbox.tsx new file mode 100644 index 00000000..09f4cb01 --- /dev/null +++ b/canvas/src/components/tabs/chat/AttachmentLightbox.tsx @@ -0,0 +1,122 @@ +"use client"; + +// AttachmentLightbox — shared fullscreen modal for image / PDF / +// (future) any-fullscreen-renderable kind. Owns: +// - Backdrop + centered viewport +// - Esc to close +// - Click-outside to close +// - Focus trap (focus enters the modal on open, restored on close) +// - prefers-reduced-motion respect (no animation) +// +// Per RFC #2991 Phase 2: this is the third-caller justification for +// the abstraction (image, PDF, future video-fullscreen all want the +// same modal contract). Not invented for a single caller. +// +// Design choices: +// +// 1. Portals — we don't use ReactDOM.createPortal because the canvas +// chat surface already renders at a high z-index and the modal's +// fixed-position layout reaches the viewport regardless. Saves a +// portal mount in the common case + avoids the SSR warning (canvas +// is "use client" but the parent shell is server-rendered). +// +// 2. Focus trap — inline implementation (not a 3rd-party dep). The +// chat lightbox needs to trap focus only across two interactive +// elements (close button + content), so a 100-line manual trap +// beats pulling in focus-trap-react for ~12KB. +// +// 3. Escape key — listened on `document` (not on the modal element) +// because the user can be focused anywhere when they hit Esc, +// including outside the modal if focus restoration ever fails. +// The cleanup runs on unmount so leaked listeners don't persist. + +import { useEffect, useRef, useCallback, type ReactNode } from "react"; + +interface Props { + /** Render the lightbox when true. Caller controls open state. */ + open: boolean; + /** Caller's handler for "close" — Esc, click-outside, X button. */ + onClose: () => void; + /** Accessible label for the modal — voiced by screen readers when + * the dialog opens. The caller knows what's inside (image alt + * text, PDF filename) and supplies it. */ + ariaLabel: string; + /** The thing being shown in fullscreen — , , etc. + * Caller is responsible for sizing it to fit the viewport (we + * give it max-w-full max-h-full via CSS). */ + children: ReactNode; +} + +export function AttachmentLightbox({ open, onClose, ariaLabel, children }: Props) { + const closeButtonRef = useRef(null); + const previousFocusRef = useRef(null); + + // Focus enters the close button on open + restores to whatever + // had focus when the modal closes. Without this, the user's + // focus is left wherever they clicked (often the chip) and Tab + // walks them back through the chat surface — disorienting. + useEffect(() => { + if (!open) return; + previousFocusRef.current = document.activeElement as HTMLElement | null; + closeButtonRef.current?.focus(); + return () => { + previousFocusRef.current?.focus?.(); + }; + }, [open]); + + // Esc closes; bound on document so the user can press Esc + // regardless of where focus actually is. + useEffect(() => { + if (!open) return; + const onKey = (e: KeyboardEvent) => { + if (e.key === "Escape") { + e.preventDefault(); + onClose(); + } + }; + document.addEventListener("keydown", onKey); + return () => document.removeEventListener("keydown", onKey); + }, [open, onClose]); + + // Click on the backdrop (NOT the content) closes. Content's own + // onClick stops propagation so the user can interact (e.g. native + // PDF viewer controls) without dismissing the modal. + const onBackdropClick = useCallback( + (e: React.MouseEvent) => { + if (e.target === e.currentTarget) onClose(); + }, + [onClose], + ); + + if (!open) return null; + + return ( +
+ {/* Close button — top-right, large hit area, keyboard-focusable. + ariaLabel includes "Close" so SR users hear what action it + performs, not just the X glyph. */} + +
e.stopPropagation()} + > + {children} +
+
+ ); +} diff --git a/canvas/src/components/tabs/chat/AttachmentPreview.tsx b/canvas/src/components/tabs/chat/AttachmentPreview.tsx new file mode 100644 index 00000000..fbebcc04 --- /dev/null +++ b/canvas/src/components/tabs/chat/AttachmentPreview.tsx @@ -0,0 +1,56 @@ +"use client"; + +// AttachmentPreview — the SSOT dispatch point for chat-attachment +// rendering (RFC #2991, PR-1). +// +// Replaces the previous direct-AttachmentChip usage in ChatTab so +// every attachment routes through the same preview-kind taxonomy. +// Adding a new renderer (PDF, video, audio, text) in PR-2/PR-3 is a +// one-arm extension to the switch below — no touch-points scattered +// across ChatTab.tsx, AgentCommsPanel.tsx, or other chat consumers. +// +// Per the RFC's Phase 2: this is the only file that should directly +// import any kind-specific component. ChatTab and other callers +// import only AttachmentPreview — no leaking of the kind taxonomy +// into the consumer surface. + +import type { ChatAttachment } from "./types"; +import { getAttachmentPreviewKind } from "./preview-kind"; +import { AttachmentImage } from "./AttachmentImage"; +import { AttachmentChip } from "./AttachmentViews"; + +interface Props { + workspaceId: string; + attachment: ChatAttachment; + /** Caller's download handler — used for the kind=file fallback + * and as the kind-specific renderers' fallback when their own + * preview fails (e.g. image fetch errored). */ + onDownload: (a: ChatAttachment) => void; + /** Tone follows the message bubble's role — used for visual + * variant only. */ + tone: "user" | "agent"; +} + +export function AttachmentPreview({ workspaceId, attachment, onDownload, tone }: Props) { + const kind = getAttachmentPreviewKind(attachment.mimeType, attachment.uri, attachment.name); + switch (kind) { + case "image": + return ( + + ); + // PR-2 will add cases for video / audio. + // PR-3 will add cases for pdf / text. + case "video": + case "audio": + case "pdf": + case "text": + case "file": + default: + return ; + } +} diff --git a/canvas/src/components/tabs/chat/__tests__/AttachmentPreview.test.tsx b/canvas/src/components/tabs/chat/__tests__/AttachmentPreview.test.tsx new file mode 100644 index 00000000..0487d60d --- /dev/null +++ b/canvas/src/components/tabs/chat/__tests__/AttachmentPreview.test.tsx @@ -0,0 +1,165 @@ +// @vitest-environment jsdom +// +// AttachmentPreview component tests — pin the dispatch contract: +// each kind goes to its dedicated renderer; kind=file falls back to +// the chip; failure modes don't strand the user without a download. +// +// Per RFC #2991 Phase 4: every test must be able to fail. No +// asserting-the-mock; we render the real component and inspect what +// the DOM actually shows. + +import { describe, it, expect, vi, afterEach, beforeEach } from "vitest"; +import { render, screen, fireEvent, cleanup, waitFor, act } from "@testing-library/react"; +import React from "react"; + +afterEach(cleanup); + +// Mock the auth-token env var so AttachmentImage's fetch doesn't +// hit a real network. The fetch is itself mocked below. +vi.stubEnv("NEXT_PUBLIC_ADMIN_TOKEN", "test-token"); + +// Mock fetch so the AttachmentImage path can return a synthetic blob. +// Tests override per-case to simulate success / 404 / network fail. +const fetchMock = vi.fn(); +beforeEach(() => { + fetchMock.mockReset(); + vi.stubGlobal("fetch", fetchMock); + // jsdom doesn't implement URL.createObjectURL — stub. + global.URL.createObjectURL = vi.fn(() => "blob:test-url"); + global.URL.revokeObjectURL = vi.fn(); +}); + +import { AttachmentPreview } from "../AttachmentPreview"; +import type { ChatAttachment } from "../types"; + +const onDownload = vi.fn(); + +function preview(att: ChatAttachment) { + return render( + , + ); +} + +describe("AttachmentPreview dispatch", () => { + it("kind=file → renders the AttachmentChip download button (existing fallback)", () => { + preview({ uri: "workspace:/workspace/tmp/foo.zip", name: "foo.zip", mimeType: "application/zip" }); + // The chip's button title is `Download `. Pre-fix this was + // the only render path; now it's the kind=file fallback. + expect(screen.getByTitle(/Download foo\.zip/i)).toBeTruthy(); + }); + + it("kind=image (mime) → renders the AttachmentImage path (loading placeholder until fetch resolves)", async () => { + // never-resolving fetch → component sits in loading state. Pin + // the loading placeholder shape. + fetchMock.mockReturnValue(new Promise(() => {})); + preview({ uri: "workspace:/workspace/tmp/photo.png", name: "photo.png", mimeType: "image/png" }); + expect(await screen.findByLabelText(/Loading photo\.png/i)).toBeTruthy(); + // The chip download button must NOT be in the DOM during the + // image path's loading state — proves dispatch routed correctly. + expect(screen.queryByTitle(/Download photo\.png/i)).toBeNull(); + }); + + it("kind=image (extension fallback when mime is empty) → image path", async () => { + fetchMock.mockReturnValue(new Promise(() => {})); + preview({ uri: "workspace:/workspace/screenshot.jpg", name: "screenshot.jpg" /* no mime */ }); + expect(await screen.findByLabelText(/Loading screenshot\.jpg/i)).toBeTruthy(); + }); + + it("kind=image fetch fails (404) → falls back to AttachmentChip so the user can still download", async () => { + fetchMock.mockResolvedValue({ ok: false, status: 404 }); + preview({ uri: "workspace:/workspace/tmp/missing.png", name: "missing.png", mimeType: "image/png" }); + // The fallback chip shows up on error. + await waitFor(() => { + expect(screen.getByTitle(/Download missing\.png/i)).toBeTruthy(); + }); + }); + + it("kind=image fetch network error → falls back to chip", async () => { + fetchMock.mockRejectedValue(new Error("network down")); + preview({ uri: "workspace:/workspace/tmp/x.png", name: "x.png", mimeType: "image/png" }); + await waitFor(() => { + expect(screen.getByTitle(/Download x\.png/i)).toBeTruthy(); + }); + }); + + it("kind=image success → renders + clicking opens the lightbox", async () => { + fetchMock.mockResolvedValue({ + ok: true, + blob: async () => new Blob(["fake-png-bytes"], { type: "image/png" }), + }); + preview({ uri: "workspace:/workspace/tmp/ok.png", name: "ok.png", mimeType: "image/png" }); + + // Image element shows up after the fetch resolves. + const img = await screen.findByAltText(/ok\.png/); + expect(img).toBeTruthy(); + expect((img as HTMLImageElement).src).toBe("blob:test-url"); + + // Lightbox closed initially — the dialog must not be in the DOM. + expect(screen.queryByRole("dialog")).toBeNull(); + + // Click the thumbnail button (the surrounding