Skip to content

feat(dashboard): wire chat attachment uploads in ChatPage#3075

Merged
houko merged 4 commits into
mainfrom
feat/chat-attachments
Apr 25, 2026
Merged

feat(dashboard): wire chat attachment uploads in ChatPage#3075
houko merged 4 commits into
mainfrom
feat/chat-attachments

Conversation

@houko

@houko houko commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Backend already supported file uploads end-to-endPOST /api/agents/{id}/upload, attachments[] on the HTTP /message body, and attachments[] 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 render AgentSessionMessage.images, so the path was effectively dead code from the user's perspective.
  • This PR connects the dashboard to that pipeline, threading attachments through both the WS and HTTP send paths and rendering uploaded files in the chat transcript (live and historical).

Changes

Data layer

  • New AttachmentRef / AgentFileUploadResult types and uploadAgentFile(agentId, file) helper in api.ts; exposed via http/client.ts and a useUploadAgentFile mutation in lib/mutations/agents.ts.
  • SendAgentMessageOptions gains attachments?: 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 WS message frame, and falls back to HTTP with the same attachments on WS-drop.
  • 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.

Test plan

  • pnpm typecheck introduces 0 new errors (25 pre-existing on main, same 25 on this branch).
  • pnpm test introduces 0 new failures (4 pre-existing failures on main in TerminalPage.test.tsx and workflows.test.tsx, same 4 here).
  • pnpm build succeeds; ChatPage chunk grew ~4kB gzipped.
  • Manual: pick / drag / paste an image; verify thumbnail in chip and in user bubble after send; verify the agent receives the image (vision-capable model).
  • Manual: upload a PDF / text file; verify file chip in user bubble; verify the LLM receives the text content.
  • Manual: open an old session that has historical images; verify they render in the transcript.
  • Manual: WS-drop mid-send → verify HTTP fallback also carries attachments[].

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.
@github-actions github-actions Bot added size/L 250-999 lines changed no-rust-required This task does not require Rust knowledge ready-for-review PR is ready for maintainer review labels Apr 25, 2026
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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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"}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

houko added 2 commits April 25, 2026 13:09
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.
@houko
houko merged commit c3b58a9 into main Apr 25, 2026
10 checks passed
@houko
houko deleted the feat/chat-attachments branch April 25, 2026 04:13
@github-actions github-actions Bot removed the ready-for-review PR is ready for maintainer review label Apr 25, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +416 to +418
if (!content.trim() && !(attachments && attachments.length > 0)) return;
const trimmed = content.trim();
const hasAttachments = !!(attachments && attachments.length > 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-rust-required This task does not require Rust knowledge size/L 250-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant