feat(dashboard): wire chat attachment uploads in ChatPage#3075
Conversation
Backend already supported file uploads end-to-end (POST
/api/agents/{id}/upload, attachments[] on /message and on the WS
"message" frame), but the dashboard had no UI to send attachments and
never rendered AgentSessionMessage.images, so the path was effectively
dead code from the user's perspective.
- Add AttachmentRef / AgentFileUploadResult types and uploadAgentFile()
to api.ts; expose via http/client.ts and useUploadAgentFile mutation.
- Thread attachments through sendMessage in ChatPage: the WebSocket
payload and HTTP fallback both now include attachments[]; an
optimistic image preview is attached to the outgoing user message.
- ChatInput gains a paperclip picker, drag-drop, paste-image, an
attachment chip strip with per-file upload status, and client-side
10MB / MIME pre-validation that mirrors the server allowlist.
- MessageBubble renders message.images for both freshly-sent uploads
and historical sessions: image MIMEs become inline thumbnails linking
to /api/uploads/{file_id}; non-image attachments render as a labeled
file chip so text/PDF entries aren't invisible in the transcript.
The buttons relied on padding alone (~44px), while the textarea has min-h-[44px] sm:min-h-[52px], so items-end pinned the buttons 8px below the textarea top edge. Give all four action buttons the same min-h to match, and use inline-flex centering so the icon stays vertically centered.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abc6024557
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| > | ||
| <img | ||
| src={src} | ||
| alt={img.filename ? decodeURIComponent(img.filename) : "attachment"} |
There was a problem hiding this comment.
Avoid decodeURIComponent on raw attachment names
The attachment renderer calls decodeURIComponent(img.filename) for display, but filenames in this flow include raw browser File.name values (optimistic user messages) and can legally contain %. decodeURIComponent throws URIError on names like 100% notes.png, which can break message rendering and destabilize the chat view for that send. Render the filename as-is (or only decode when you can guarantee percent-encoding) to prevent runtime crashes.
Useful? React with 👍 / 👎.
| // Mirrors `is_allowed_content_type` in routes/agents.rs. Kept liberal to | ||
| // avoid divergence — if the server's allowlist tightens, the upload will | ||
| // fail with a 400 and we surface that error per-attachment. | ||
| const ATTACHMENT_ACCEPT = "image/png,image/jpeg,image/webp,image/gif,audio/mpeg,audio/wav,audio/webm,audio/ogg,audio/mp4,text/plain,text/markdown,text/csv,application/pdf"; |
There was a problem hiding this comment.
Don't offer non-image attachment types the agent drops
ATTACHMENT_ACCEPT advertises audio/text/PDF uploads as supported, but the agent send pipeline resolves attachments via resolve_attachments in routes/agents.rs, which skips any attachment whose MIME is not image/*. That means users can upload and send non-image files from ChatPage and see chips, yet the model never receives those attachments. Limit accepted types to images (or defer non-image options until backend injection is implemented) to avoid silent loss of user-provided context.
Useful? React with 👍 / 👎.
Three fixes from the PR review:
1. Stale-closure in unmount cleanup — the previous `useEffect(()=>{...},[])`
captured `attachments` from first render (always empty), so navigating
away mid-upload leaked every blob URL it created. Mirror the state
into a ref and read from it in the cleanup.
2. Slash + attachments silent drop — `handleSubmit` cleared the chip
strip unconditionally even when the message was a slash command,
which `useChatMessages.sendMessage` handles via a side-channel that
never sees the attachments. Detect the slash prefix and skip both
payload construction and chip cleanup so the user's uploads survive
for the next real send.
3. `content_type` propagation — thread the field from `AgentSessionImage`
through history mapping. Backend currently only emits image entries
so the renderer's "missing → image" fallback is correct today, but
the field-drop would silently break non-image rendering if the
backend ever serializes PDFs/text into history.
Plus aria-labels on the paperclip + chip-remove buttons (titles only
weren't enough for screen readers) and a comment defending the
deliberately empty `invalidateQueries` call in `useUploadAgentFile`.
P1: Avoid decodeURIComponent on raw filenames (would throw URIError on literal '%' in names like '100% notes.png'). Switch the upload helper to ASCII-sanitize the X-Filename header instead of percent-encoding it, so the server-stored label is already display-ready and the bubble renderer can drop its decode pass. P2: Restrict ATTACHMENT_ACCEPT to image MIMEs and reject non-image drag/paste in enqueueFiles. resolve_attachments() in routes/agents.rs silently skips non-image content, so accepting audio/text/PDF here showed user-facing chips for context the agent never received.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 204d447e03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!content.trim() && !(attachments && attachments.length > 0)) return; | ||
| const trimmed = content.trim(); | ||
| const hasAttachments = !!(attachments && attachments.length > 0); |
There was a problem hiding this comment.
Bypass WS when sending attachments without text
This new guard allows attachment-only sends, but the subsequent WebSocket path still transmits content as the trimmed message string, which is empty in this case. The WS handler rejects empty content (crates/librefang-api/src/ws.rs returns Missing or empty 'content' field), so a user who sends only an image hits an immediate error instead of a normal turn; attachment-only sends should either require text or skip WS and go directly through the HTTP /message path.
Useful? React with 👍 / 👎.
Summary
POST /api/agents/{id}/upload,attachments[]on the HTTP/messagebody, andattachments[]on the WS{"type":"message"}frame (crates/librefang-api/src/ws.rs:707-725). The dashboard, however, had no UI to send attachments and didn't renderAgentSessionMessage.images, so the path was effectively dead code from the user's perspective.Changes
Data layer
AttachmentRef/AgentFileUploadResulttypes anduploadAgentFile(agentId, file)helper inapi.ts; exposed viahttp/client.tsand auseUploadAgentFilemutation inlib/mutations/agents.ts.SendAgentMessageOptionsgainsattachments?: AttachmentRef[];sendAgentMessage()forwards the field on the JSON body.ChatPage (
src/pages/ChatPage.tsx)sendMessage(content, attachments?)snapshots the attachments onto the optimistic user bubble, ships them on the WSmessageframe, and falls back to HTTP with the same attachments on WS-drop.ChatInputgains a paperclip picker, drag-drop, paste-image, an attachment chip strip with per-file upload status, and client-side 10MB / MIME pre-validation that mirrors the server allowlist.MessageBubblerendersmessage.imagesfor both freshly-sent uploads and historical sessions: image MIMEs become inline thumbnails linking to/api/uploads/{file_id}; non-image attachments render as a labeled file chip so text/PDF entries aren't invisible in the transcript.Test plan
pnpm typecheckintroduces 0 new errors (25 pre-existing onmain, same 25 on this branch).pnpm testintroduces 0 new failures (4 pre-existing failures onmaininTerminalPage.test.tsxandworkflows.test.tsx, same 4 here).pnpm buildsucceeds;ChatPagechunk grew ~4kB gzipped.attachments[].