From 92bac490d7849d2de7ba2eacb010dea44a639455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sun, 16 Aug 2026 15:08:24 +0000 Subject: [PATCH] attach a PDF and the agent reads it; the microphone button is gone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two requests, one file apart. ── The microphone ── Removed from the chat composer. The HOOK stays: `useAudioRecording` and `blobToWav` are also the file browser's DictateDialog, which is not what was asked to go. ── PDFs, by not extracting anything ── A PDF entry sat in the attach menu once with no `onSelect` at all — the menu offered it and did nothing, because nothing here could extract PDF text. It is back and real, and it still extracts nothing: the file is uploaded, left on disk, and the agent is given its PATH. Its own `Read` handles PDFs by page, better than any extraction this could have shipped, and it can go back for a section later without paying for it twice. The alternative was base64 in the message. That is megabytes in the turn, re-sent for the rest of the conversation, to produce worse text. ── What this uncovered ── `attachmentIds` has been on this websocket and in `types.ts` for a long time and NOTHING on the server read it. Images work because they travel separately as base64, so the ids were dead on the wire — and a PDF is exactly the case that needs them. They resolve to paths now, appended to the prompt as `Attached files (read from disk)`. Ids arrive in a websocket message and become a filesystem path, so they are checked rather than trusted: only the `.` shape uploads actually generate, and only in the two directories uploads actually write to. Verified — a valid id resolves, an unknown one yields nothing, and `../../../../etc/passwd` and `; rm -rf /` yield nothing. Both upload directories are searched because the first message of a chat carries ids written to `tmp/` (no session yet) and later ones carry ids under the session. PDFs may be 25 MB against an image's 5: nothing carries them into a prompt, so the ceiling is storage rather than context. tsgo clean, frontend builds, 914 pass / 7 fail unchanged. `[open]` For a MEMBER the file lands under `data//attachments/`, and whether their Linux user can read it depends on the confinement ACLs. Untested — the owner path is what was verified. Co-Authored-By: Claude Opus 5 --- src/servers/api/chat/attached-files.ts | 48 +++++++++++++++++++ src/servers/api/chat/websocket.ts | 22 ++++++++- src/servers/api/upload/upload.ts | 31 +++++++++--- .../Chat/EmbeddableChat/useEmbeddableChat.ts | 1 + .../src/apps/Chat/components/AttachButton.tsx | 35 ++++++++++++-- .../apps/Chat/components/AttachmentList.tsx | 10 +++- .../src/apps/Chat/components/InputArea.tsx | 33 ++----------- .../officerdev/src/apps/Chat/types.ts | 9 +++- .../src/apps/Chat/useAttachments.ts | 31 ++++++++++++ 9 files changed, 175 insertions(+), 45 deletions(-) create mode 100644 src/servers/api/chat/attached-files.ts diff --git a/src/servers/api/chat/attached-files.ts b/src/servers/api/chat/attached-files.ts new file mode 100644 index 00000000..166ea869 --- /dev/null +++ b/src/servers/api/chat/attached-files.ts @@ -0,0 +1,48 @@ +import { existsSync } from 'node:fs'; +import { join } from 'node:path'; +import { getAttachmentsDir, getTmpAttachmentsDir } from '../../data-path'; +import { getUserById } from 'officerdb'; + +/** + * Where an attached file ended up, so the agent can be told to read it. + * + * ── Why a path and not the bytes ── + * + * An image goes to the model as base64 inside the message. A PDF cannot: it would be megabytes of base64 + * in the turn, re-sent for the rest of the conversation, to produce text the agent's own tools extract + * better. Reading a document is something `Read` already does well — page ranges, and re-reading a + * section later without paying for it twice. + * + * So the file stays where `/upload` put it and the prompt names it. This is a machine where the agent + * runs as the person who attached it, so it can simply open it. + * + * ── The ids are checked, not trusted ── + * + * An id arrives in a websocket message, and it is used to build a path. `/upload` generates them as + * `.`, so anything that is not that shape did not come from us — and a `..` inside one would + * name a file outside the attachment directory entirely. Only the two directories uploads actually write + * to are searched, and only ids matching the shape uploads actually produce. + */ +const ATTACHMENT_ID = /^[a-f0-9-]{36}\.[A-Za-z0-9]{1,8}$/; + +export async function attachedFilePaths( + userId: number, + sessionId: string, + attachmentIds: string[] | undefined, +): Promise { + if (!attachmentIds?.length) return []; + + const user = await getUserById(userId).catch(() => null); + if (!user?.email) return []; + + const paths: string[] = []; + for (const id of attachmentIds) { + if (!ATTACHMENT_ID.test(id)) continue; + // Uploads before a session exists land in `tmp/`; everything after lands under the session. Both are + // checked because a first message carries ids from the first shape and later ones from the second. + const candidates = [join(getAttachmentsDir(user.email, sessionId), id), join(getTmpAttachmentsDir(user.email), id)]; + const found = candidates.find((path) => existsSync(path)); + if (found) paths.push(found); + } + return paths; +} diff --git a/src/servers/api/chat/websocket.ts b/src/servers/api/chat/websocket.ts index c5da6ef5..7b70990c 100644 --- a/src/servers/api/chat/websocket.ts +++ b/src/servers/api/chat/websocket.ts @@ -16,6 +16,7 @@ import { sendOpenCodeStreaming } from '@@/channels/send-opencode'; import * as sidecar from '@@/sidecar-registry'; import { join } from 'path'; import { getOwnerHomeDir, getEmailAccountsDir } from '../../../servers/data-path'; +import { attachedFilePaths } from './attached-files'; import { getUserSettings, getEmailAccounts, getChatEventsSince, appendChatEvent, getUserById } from 'officerdb'; import { resolveHomeDir } from '@@/user-home'; import { claudeLoginState } from '@@/os-user-claude'; @@ -360,10 +361,29 @@ async function handleChat( const sessionId = msg.sessionId || randomUUID(); // Prepend resume summary to the prompt if present - const prompt = msg.resumeSummary + const withSummary = msg.resumeSummary ? `Here is a summary of a previous conversation to continue from:\n\n${msg.resumeSummary}\n\n---\n\nUser's new message: ${msg.prompt}` : msg.prompt; + /** + * Attached files, named by PATH rather than carried in the message. + * + * `attachmentIds` has been on this wire and in `types.ts` since before today and NOTHING read it — + * images work because they travel separately as base64, so the ids were dead. A PDF cannot travel that + * way: it would be megabytes of base64 in every turn of the conversation, re-sent forever. + * + * So the file stays on disk and the agent is told where it is. Reading a document is something its own + * tools already do better than a prompt can — page ranges, extraction, re-reading a section later + * without paying for it again — and this is a machine where the agent runs as the person who uploaded + * the file. + */ + const attached = await attachedFilePaths(userId, sessionId, msg.attachmentIds); + const prompt = attached.length + ? `${withSummary}\n\nAttached ${attached.length === 1 ? 'file' : 'files'} (read from disk):\n${attached + .map((path) => `- ${path}`) + .join('\n')}` + : withSummary; + // Use provided model, or fall back to user default, or the system default. const model = msg.model || (await getUserDefaultModel(userId)) || DEFAULT_MODEL; diff --git a/src/servers/api/upload/upload.ts b/src/servers/api/upload/upload.ts index 6f8029dd..b7cb970f 100644 --- a/src/servers/api/upload/upload.ts +++ b/src/servers/api/upload/upload.ts @@ -4,6 +4,8 @@ import { join } from 'node:path'; import { getTmpAttachmentsDir, getAttachmentsDir } from '@@/data-path'; const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5 MB +/** PDFs never enter a prompt whole — the agent reads them from disk — so the ceiling is storage, not context. */ +const PDF_MAX_FILE_SIZE = 25 * 1024 * 1024; // 25 MB export const uploadRouter = createRouter(); @@ -18,12 +20,21 @@ uploadRouter.post('/', async (ctx) => { return ctx.json({ error: 'file is required' }, 400); } - if (!file.type.startsWith('image/')) { - return ctx.json({ error: 'Only image files are allowed' }, 400); + // Images and PDFs. The two are handled very differently downstream and it is worth knowing which is + // which here: an image travels to the model as base64 in the message, while a PDF is left on disk and + // the agent is told where it is, because reading a document is something its own tools already do well + // (page ranges, extraction) and a 10 MB base64 blob in a prompt is not. + const isImage = file.type.startsWith('image/'); + const isPdf = file.type === 'application/pdf' || file.name.toLowerCase().endsWith('.pdf'); + if (!isImage && !isPdf) { + return ctx.json({ error: 'Only images and PDFs are allowed' }, 400); } - if (file.size > MAX_FILE_SIZE) { - return ctx.json({ error: 'File exceeds 5 MB limit' }, 400); + // A PDF is allowed to be bigger: 5 MB is a generous photo and a small report. The agent reads it from + // disk in page ranges, so size costs storage rather than context. + const limit = isPdf ? PDF_MAX_FILE_SIZE : MAX_FILE_SIZE; + if (file.size > limit) { + return ctx.json({ error: `File exceeds ${Math.round(limit / 1024 / 1024)} MB limit` }, 400); } try { @@ -41,10 +52,16 @@ uploadRouter.post('/', async (ctx) => { await mkdir(saveDir, { recursive: true }); await Bun.write(join(saveDir, filename), buffer); - const base64 = buffer.toString('base64'); - const dataUrl = `data:${file.type};base64,${base64}`; + // Only an image gets a data URL. Building one for a PDF would mean carrying megabytes through the + // browser for a preview nothing renders. + const dataUrl = isImage ? `data:${file.type};base64,${buffer.toString('base64')}` : ''; - return ctx.json({ filename: file.name, dataUrl, attachmentId: filename }); + return ctx.json({ + filename: file.name, + dataUrl, + attachmentId: filename, + kind: isPdf ? 'pdf' : 'image', + }); } catch (err) { const message = err instanceof Error ? err.message : 'Upload failed'; return ctx.json({ error: message }, 500); diff --git a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts index f4d68e47..88242e4d 100644 --- a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts +++ b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts @@ -589,6 +589,7 @@ export function useEmbeddableChat(params: UseEmbeddableChatParams, onMessageComp attachments: attachmentManager.attachments, attachWebpage: attachmentManager.attachWebpage, attachImage: attachmentManager.attachImage, + attachFile: attachmentManager.attachFile, removeAttachment: attachmentManager.removeAttachment, availableModels, showJumpToBottom, diff --git a/src/workspaces/officerdev/src/apps/Chat/components/AttachButton.tsx b/src/workspaces/officerdev/src/apps/Chat/components/AttachButton.tsx index 568af2bc..a72ace19 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/AttachButton.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/AttachButton.tsx @@ -14,6 +14,7 @@ type AttachButtonProps = { * attachment that would be discarded before the model saw it. See `supportsImages` in InputArea. */ onAttachImage?: (file: File) => void; + onAttachFile?: (file: File) => void; onAttachWebpage: () => void; /** Text files are inlined into the composer rather than sent as an attachment — see below. */ onAttachText: (text: string) => void; @@ -29,8 +30,15 @@ const TEXT_ACCEPT = const NUL = String.fromCharCode(0); -export function AttachButton({ onAttachImage, onAttachWebpage, onAttachText, size = 'sm' }: AttachButtonProps) { +export function AttachButton({ + onAttachImage, + onAttachFile, + onAttachWebpage, + onAttachText, + size = 'sm', +}: AttachButtonProps) { const imageInputRef = useRef(null); + const pdfInputRef = useRef(null); const textInputRef = useRef(null); const sizeClasses = size === 'md' ? 'h-10 w-10' : 'h-7 w-7 md:h-9 md:w-9'; @@ -81,9 +89,17 @@ export function AttachButton({ onAttachImage, onAttachWebpage, onAttachText, siz Image )} - {/* "PDF" used to sit beside this one. Both were inert — no onSelect at all — so the menu offered - four things and did two. There is no PDF text extraction anywhere in the platform, client or - server, so that entry could not be made honest without building one first. */} + {/* PDF sat here once with no `onSelect` at all — the menu offered it and did nothing, because + nothing in the platform could extract PDF text. It is back and real, by not extracting + anything: the file is uploaded, left on disk, and the agent is given its PATH. Its own + `Read` handles PDFs by page, which is better than any extraction this could have shipped — + and nothing has to carry megabytes of base64 through every turn of the conversation. */} + {onAttachFile && ( + pdfInputRef.current?.click()}> + + PDF + + )} textInputRef.current?.click()}> Text File @@ -105,6 +121,17 @@ export function AttachButton({ onAttachImage, onAttachWebpage, onAttachText, siz ev.target.value = ''; }} /> + { + const file = ev.target.files?.[0]; + if (file) onAttachFile?.(file); + ev.target.value = ''; + }} + /> ) : a.type === 'image' ? ( + ) : a.type === 'file' ? ( + ) : ( )} - {a.type === 'image' ? a.filename : a.loading ? 'Loading...' : a.title} + {/* A file has no preview and no title — its name is all there is, and all that is needed: + the agent is told where it is, not handed its contents. */} + + {a.type === 'image' || a.type === 'file' ? a.filename : a.loading ? 'Loading...' : a.title} + diff --git a/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx b/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx index 61f43b42..626ed13f 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx @@ -1,9 +1,8 @@ import type { DragEvent } from 'react'; import { useRef, useState } from 'react'; -import { Loader2, Mic, Send, Square, ListPlus, X } from 'lucide-react'; +import { Loader2, Send, Square, ListPlus, X } from 'lucide-react'; import { Button } from '@/components/ui/button'; import type { QueuedPrompt, UseEmbeddableChatType } from '../EmbeddableChat/useEmbeddableChat'; -import { useAudioRecording } from '../useAudioRecording'; import { ModelSelector } from './ModelSelector'; import { BackgroundJobDialog } from './BackgroundJobDialog'; import { armBackgroundJob } from '../../../background-job-origin'; @@ -37,6 +36,7 @@ export const InputArea = ({ manager }: InputAreaProps) => { attachments, attachWebpage, attachImage, + attachFile, removeAttachment, appendToInput, thinkingLevel, @@ -48,7 +48,6 @@ export const InputArea = ({ manager }: InputAreaProps) => { sessionId, } = manager; - const { recording, transcribing, toggleRecording } = useAudioRecording(appendToInput); const [urlDialogOpen, setUrlDialogOpen] = useState(false); const [jobDialogOpen, setJobDialogOpen] = useState(false); // The panel's own directory, so a background job needs no path typed into it. @@ -95,10 +94,10 @@ export const InputArea = ({ manager }: InputAreaProps) => {
setUrlDialogOpen(true)} onAttachText={appendToInput} /> -