diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index 8d7c696cd625..539827a0a5ce 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -576,6 +576,67 @@ describe("AssetAccess", () => { }); }).pipe(Effect.provide(testLayer)), ); + + it.effect("serves document attachments inline when a viewer requests it", () => + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const attachmentId = "thread-1-00000000-0000-4000-8000-000000000001-pdf"; + const attachmentPath = path.join(config.attachmentsDir, `${attachmentId}.pdf`); + yield* fileSystem.makeDirectory(config.attachmentsDir, { recursive: true }); + yield* fileSystem.writeFile(attachmentPath, new Uint8Array([1, 2, 3])); + + const result = yield* issueAssetUrl({ + resource: { + _tag: "attachment", + attachmentId, + fileName: "report.pdf", + mimeType: "application/pdf", + disposition: "inline", + }, + }); + const suffix = result.relativeUrl.slice(`${ASSET_ROUTE_PREFIX}/`.length); + const separatorIndex = suffix.indexOf("/"); + + expect( + yield* resolveAsset(suffix.slice(0, separatorIndex), suffix.slice(separatorIndex + 1)), + ).toEqual({ + kind: "file", + path: attachmentPath, + fileName: "report.pdf", + mimeType: "application/pdf", + }); + }).pipe(Effect.provide(testLayer)), + ); + + it.effect("keeps inline requests for other attachment types as downloads", () => + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const attachmentId = "thread-1-00000000-0000-4000-8000-000000000002-zip"; + const attachmentPath = path.join(config.attachmentsDir, `${attachmentId}.zip`); + yield* fileSystem.makeDirectory(config.attachmentsDir, { recursive: true }); + yield* fileSystem.writeFile(attachmentPath, new Uint8Array([1, 2, 3])); + + const result = yield* issueAssetUrl({ + resource: { + _tag: "attachment", + attachmentId, + fileName: "archive.zip", + mimeType: "text/html", + disposition: "inline", + }, + }); + const suffix = result.relativeUrl.slice(`${ASSET_ROUTE_PREFIX}/`.length); + const separatorIndex = suffix.indexOf("/"); + + expect( + yield* resolveAsset(suffix.slice(0, separatorIndex), suffix.slice(separatorIndex + 1)), + ).toMatchObject({ kind: "file", path: attachmentPath, download: true }); + }).pipe(Effect.provide(testLayer)), + ); it.effect("issues project favicon capabilities with a signed fallback", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; diff --git a/apps/server/src/assets/AssetAccess.ts b/apps/server/src/assets/AssetAccess.ts index ad7cb273cf07..36fa9130b683 100644 --- a/apps/server/src/assets/AssetAccess.ts +++ b/apps/server/src/assets/AssetAccess.ts @@ -51,6 +51,14 @@ const ASSET_TOKEN_TTL_MS = 60 * 60 * 1000; const PROJECT_FAVICON_TOKEN_BUCKET_MS = 30 * 60 * 1000; const PROJECT_FAVICON_VERSION_PREFIX = "v"; const INLINE_VIDEO_MIME_TYPE_PATTERN = /^video\/[\w!#$&^.+-]+$/i; +// Extensions a document viewer may request inline. The extension comes from +// the attachment id the server assigned, never from the client's mime type. +const INLINE_DOCUMENT_EXTENSIONS = new Set(["pdf", "html", "htm"]); +const INLINE_DOCUMENT_MIME_TYPES: Record = { + pdf: "application/pdf", + html: "text/html", + htm: "text/html", +}; const PREVIEW_ASSET_EXTENSIONS = new Set([ ...WORKSPACE_BROWSER_PREVIEW_EXTENSIONS, ...WORKSPACE_IMAGE_PREVIEW_EXTENSIONS, @@ -362,19 +370,31 @@ export const issueAssetUrl = Effect.fn("AssetAccess.issueAssetUrl")(function* (i } // Generic files carry their extension inside the attachment id (that // shape resolves the on-disk path); images do not. Videos and images - // render inline; other generic files download. - const isGenericFile = parseAttachmentFileExtension(input.resource.attachmentId) !== null; + // render inline. Other generic files download, unless a document viewer + // asked for inline and the stored extension is one a browser can show. + const extension = parseAttachmentFileExtension(input.resource.attachmentId); + const isGenericFile = extension !== null; const videoMimeType = input.resource.mimeType?.split(";", 1)[0]?.trim() ?? ""; const isVideo = INLINE_VIDEO_MIME_TYPE_PATTERN.test(videoMimeType); + const inlineDocumentMimeType = + input.resource.disposition === "inline" && + extension !== null && + INLINE_DOCUMENT_EXTENSIONS.has(extension) + ? INLINE_DOCUMENT_MIME_TYPES[extension] + : undefined; claims = { version: 1, kind: "attachment", attachmentId: input.resource.attachmentId, - ...(isGenericFile && !isVideo ? { download: true } : {}), - ...(input.resource.fileName !== undefined ? { fileName: input.resource.fileName } : {}), - ...(input.resource.mimeType !== undefined - ? { mimeType: isVideo ? videoMimeType : input.resource.mimeType } + ...(isGenericFile && !isVideo && inlineDocumentMimeType === undefined + ? { download: true } : {}), + ...(input.resource.fileName !== undefined ? { fileName: input.resource.fileName } : {}), + ...(inlineDocumentMimeType !== undefined + ? { mimeType: inlineDocumentMimeType } + : input.resource.mimeType !== undefined + ? { mimeType: isVideo ? videoMimeType : input.resource.mimeType } + : {}), expiresAt, }; fileName = input.resource.fileName ?? path.basename(attachmentPath); diff --git a/apps/server/src/http.test.ts b/apps/server/src/http.test.ts index 6b0940856659..0c253033ac52 100644 --- a/apps/server/src/http.test.ts +++ b/apps/server/src/http.test.ts @@ -324,6 +324,19 @@ describe("assetResponseHeaders", () => { "X-Content-Type-Options": "nosniff", }); }); + it("serves inline attachment documents with their declared mime type", () => { + expect( + assetResponseHeaders("/attachments/upload.bin", { mimeType: "application/pdf" }), + ).toMatchObject({ + "Content-Type": "application/pdf", + }); + expect( + assetResponseHeaders("/attachments/upload.bin", { mimeType: "text/html" }), + ).toMatchObject({ + "Content-Type": "text/html; charset=utf-8", + "Content-Security-Policy": "sandbox allow-scripts allow-forms allow-popups allow-modals", + }); + }); it("serves HTML assets as utf-8 inside a sandboxed origin", () => { for (const path of ["/workspace/page.html", "/workspace/PAGE.HTM", "/tmp/report.html"]) { expect(assetResponseHeaders(path)).toMatchObject({ diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index 4d5865335a39..3bcca884107c 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -64,6 +64,8 @@ const isSafeDownloadMimeType = (mimeType: string): boolean => !/(?:^text\/html$|\/xml(?:$|-)|\+xml$)/i.test(mimeType.trim().toLowerCase()); const isSafeInlineVideoMimeType = (mimeType: string): boolean => DOWNLOAD_MIME_TYPE_PATTERN.test(mimeType) && mimeType.toLowerCase().startsWith("video/"); +const isSafeInlineDocumentMimeType = (mimeType: string): boolean => + mimeType.toLowerCase() === "application/pdf" || mimeType.toLowerCase() === "text/html"; /** RFC 6266 disposition with an ASCII fallback name plus a UTF-8 `filename*`. */ export function downloadContentDisposition(fileName?: string): string { @@ -93,7 +95,7 @@ export function assetResponseHeaders( }, ): Record { const lowerPath = filePath.toLowerCase(); - const inlineVideoMimeType = options?.mimeType?.split(";", 1)[0]?.trim(); + const inlineMimeType = options?.mimeType?.split(";", 1)[0]?.trim(); return { "Cache-Control": "private, max-age=3600", "X-Content-Type-Options": "nosniff", @@ -106,14 +108,24 @@ export function assetResponseHeaders( ? options.mimeType : "application/octet-stream", } - : inlineVideoMimeType !== undefined && isSafeInlineVideoMimeType(inlineVideoMimeType) - ? { "Content-Type": inlineVideoMimeType } - : lowerPath.endsWith(".html") || lowerPath.endsWith(".htm") + : inlineMimeType !== undefined && isSafeInlineVideoMimeType(inlineMimeType) + ? { "Content-Type": inlineMimeType } + : inlineMimeType !== undefined && isSafeInlineDocumentMimeType(inlineMimeType) ? { - "Content-Type": "text/html; charset=utf-8", - "Content-Security-Policy": HTML_CONTENT_SECURITY_POLICY, + "Content-Type": + inlineMimeType.toLowerCase() === "text/html" + ? "text/html; charset=utf-8" + : "application/pdf", + ...(inlineMimeType.toLowerCase() === "text/html" + ? { "Content-Security-Policy": HTML_CONTENT_SECURITY_POLICY } + : {}), } - : {}), + : lowerPath.endsWith(".html") || lowerPath.endsWith(".htm") + ? { + "Content-Type": "text/html; charset=utf-8", + "Content-Security-Policy": HTML_CONTENT_SECURITY_POLICY, + } + : {}), ...(!options?.download && lowerPath.endsWith(".svg") ? { "Content-Security-Policy": SVG_CONTENT_SECURITY_POLICY } : {}), diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index cadb8b3028b3..a5335856be93 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -132,6 +132,7 @@ import { DEFAULT_THREAD_TERMINAL_ID, MAX_TERMINALS_PER_GROUP, type ChatMessage, + isBrowserPreviewAttachment, isImageAttachment, videoMimeType, type SessionPhase, @@ -2602,7 +2603,7 @@ function ChatViewContent(props: ChatViewProps) { }); }, []); const serverMessages = activeThread?.messages; - const openFileAttachment = useCallback( + const downloadFileAttachment = useCallback( async (attachment: ChatFileAttachment) => { const connection = readPreparedConnection(environmentId); if (!connection) { @@ -2654,6 +2655,16 @@ function ChatViewContent(props: ChatViewProps) { }, [createAttachmentAssetUrl, environmentId, routeThreadKey], ); + const openFileAttachment = useCallback( + (attachment: ChatFileAttachment) => { + if (isBrowserPreviewAttachment(attachment) && activeThreadRef) { + useRightPanelStore.getState().openAttachment(activeThreadRef, attachment); + return; + } + void downloadFileAttachment(attachment); + }, + [activeThreadRef, downloadFileAttachment], + ); const serverAttachmentIds = useMemo(() => { const attachmentIds = new Set(); for (const message of serverMessages ?? []) { @@ -7351,14 +7362,18 @@ function ChatViewContent(props: ChatViewProps) { /> ) : (renderedRightPanelSurface?.kind === "files" || renderedRightPanelSurface?.kind === "file") && - activeProject && - activeWorkspaceRoot ? ( + ((activeProject && activeWorkspaceRoot) || + (renderedRightPanelSurface.kind === "file" && renderedRightPanelSurface.attachment)) ? ( [] = []; - if (surface.kind === "file") { + if (surface.kind === "file" && surface.attachment === undefined) { items.push({ id: "copy-path", label: "Copy path" }); } const menuPreviewTabId = previewTabIdOf(surface, props.previewSessions); @@ -837,7 +837,9 @@ export function RightPanelTabs(props: RightPanelTabsProps) { const action = await api.contextMenu.show(items, { x: event.clientX, y: event.clientY }); switch (action) { case "copy-path": - if (surface.kind === "file") props.onCopyFilePath(surface.relativePath); + if (surface.kind === "file" && surface.attachment === undefined) { + props.onCopyFilePath(surface.relativePath); + } break; case "toggle-mute": { // menuOverlay repeats the disabled gate above: the desktop tab must diff --git a/apps/web/src/components/chat/MessagesTimeline.test.tsx b/apps/web/src/components/chat/MessagesTimeline.test.tsx index 1044a956bb66..3cd601ce09a3 100644 --- a/apps/web/src/components/chat/MessagesTimeline.test.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.test.tsx @@ -437,7 +437,7 @@ describe("MessagesTimeline", () => { expect(resolveTimelineMinimapInteractiveWidth(40, true)).toBe("22rem"); }); - it("renders generic attachments as download links instead of image previews", () => { + it("gives browser documents separate preview and download controls", () => { const entry = { ...buildUserTimelineEntry("Read the report."), message: { @@ -459,9 +459,9 @@ describe("MessagesTimeline", () => { , ); - expect(markup).toContain( - '', - ); + expect(markup).toContain('aria-label="Preview report.pdf"'); + expect(markup).toContain('aria-label="Download report.pdf"'); + expect(markup).not.toContain('download="report.pdf"'); expect(markup).not.toContain('alt="report.pdf"'); }); @@ -502,7 +502,7 @@ describe("MessagesTimeline", () => { expect(busyMarkup).not.toContain('disabled=""'); expect(busyMarkup).toContain(">Loading…"); }); - it("renders a file download button without creating its URL in advance", () => { + it("renders an ordinary file download button without creating its URL in advance", () => { const entry = { ...buildUserTimelineEntry("Read the report."), message: { @@ -511,8 +511,8 @@ describe("MessagesTimeline", () => { { type: "file" as const, id: "attachment-report-pdf", - name: "report.pdf", - mimeType: "application/pdf", + name: "archive.zip", + mimeType: "application/zip", sizeBytes: 42, }, ], @@ -524,9 +524,9 @@ describe("MessagesTimeline", () => { ); expect(markup).toContain( - ' + + ctx.onFileDownload(file)} + /> + } + > + + + Download {file.name} + + + ); + } + + const content = ( + <> + {fileIdentity} {file.downloadable === false ? null : ( )} ); - return file.previewUrl ? ( + return file.previewUrl && !opensInPreview ? ( ctx.onFileOpen(file)} className="flex min-w-0 cursor-pointer items-center gap-2 rounded-md py-1 text-left text-sm hover:underline focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring/70" > @@ -1257,7 +1300,11 @@ function UserTimelineRow({ row }: { row: Extract (
- + {attachment.name}
))} diff --git a/apps/web/src/components/files/FilePreviewPanel.test.ts b/apps/web/src/components/files/FilePreviewPanel.test.ts index 3b5295f180eb..5ef590847c4b 100644 --- a/apps/web/src/components/files/FilePreviewPanel.test.ts +++ b/apps/web/src/components/files/FilePreviewPanel.test.ts @@ -5,7 +5,11 @@ import { normalizeFileCommentRange, remapFileCommentAnnotations, } from "./fileCommentAnnotations"; -import { isMarkdownPreviewFile, setMarkdownTaskChecked } from "./filePreviewMode"; +import { + isMarkdownPreviewFile, + setMarkdownTaskChecked, + shouldShowFileExplorer, +} from "./filePreviewMode"; describe("file comment annotations", () => { it("normalizes and formats selected line ranges", () => { @@ -66,6 +70,42 @@ describe("isMarkdownPreviewFile", () => { }); }); +describe("shouldShowFileExplorer", () => { + it("hides the workspace tree for host files and attachments", () => { + expect( + shouldShowFileExplorer({ + relativePath: "/tmp/report.pdf", + explorerOpen: true, + attachmentOpen: false, + }), + ).toBe(false); + expect( + shouldShowFileExplorer({ + relativePath: "report.pdf", + explorerOpen: true, + attachmentOpen: true, + }), + ).toBe(false); + }); + + it("keeps the saved explorer preference for workspace files", () => { + expect( + shouldShowFileExplorer({ + relativePath: "docs/report.pdf", + explorerOpen: true, + attachmentOpen: false, + }), + ).toBe(true); + expect( + shouldShowFileExplorer({ + relativePath: "docs/report.pdf", + explorerOpen: false, + attachmentOpen: false, + }), + ).toBe(false); + }); +}); + describe("setMarkdownTaskChecked", () => { const markdown = "- [ ] First\n- [x] Second\n"; diff --git a/apps/web/src/components/files/FilePreviewPanel.tsx b/apps/web/src/components/files/FilePreviewPanel.tsx index 40c67bfce5e1..0ad18434d7e7 100644 --- a/apps/web/src/components/files/FilePreviewPanel.tsx +++ b/apps/web/src/components/files/FilePreviewPanel.tsx @@ -1,4 +1,5 @@ import type { + ChatFileAttachment, EditorId, EnvironmentId, ResolvedKeybindingsConfig, @@ -23,6 +24,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { isBrowserPreviewFile, openFileInPreview } from "~/browser/openFileInPreview"; import { useAssetUrlRefresh, useAssetUrlState } from "~/assets/assetUrls"; import { OpenInPicker } from "~/components/chat/OpenInPicker"; +import { PierreEntryIcon } from "~/components/chat/PierreEntryIcon"; import { MediaVideoPlayer } from "~/components/media/MediaVideoPlayer"; import { MediaActions, type MediaActionSource } from "~/components/media/MediaActions"; import { useRemoteOpenState } from "~/remoteOpen"; @@ -64,7 +66,11 @@ import { installFileEditorDismissal } from "./fileEditorDismissal"; import { resolveCenteredFileLineScrollTop } from "./fileLineReveal"; import { DiffCommentAnnotation } from "../diffs/DiffCommentAnnotation"; import { projectFileCacheKey, projectFileEditorCacheKey } from "./fileContentRevision"; -import { isMarkdownPreviewFile, setMarkdownTaskChecked } from "./filePreviewMode"; +import { + isMarkdownPreviewFile, + setMarkdownTaskChecked, + shouldShowFileExplorer, +} from "./filePreviewMode"; import { FileSaveCoordinator } from "./fileSaveCoordinator"; import { confirmProjectFileQueryData, @@ -78,6 +84,7 @@ interface FilePreviewPanelProps { cwd: string; projectName: string; relativePath: string | null; + attachment?: ChatFileAttachment; threadRef: ScopedThreadRef; composerDraftTarget: ScopedThreadRef | DraftId; keybindings: ResolvedKeybindingsConfig; @@ -200,6 +207,69 @@ function WorkspaceImagePreview(props: { const isPdfPreviewFile = (path: string): boolean => /\.pdf$/i.test(path.split(/[?#]/, 1)[0] ?? ""); +function BrowserDocumentFrame(props: { + readonly src: string; + readonly title: string; + readonly pdf: boolean; +}) { + const className = "min-h-0 flex-1 border-0 bg-white"; + // The built-in PDF viewer needs an unsandboxed frame; a PDF runs no scripts. + return props.pdf ? ( + // oxlint-disable-next-line react/iframe-missing-sandbox +