Repository navigation
fix(frontend): send attached files from the expert side-panel chat and show the message at once (SECRT-2956 1/3) [1/3] - #15582
Conversation
The drawer's onSend ignores the files and workspace files ChatInput hands it, and draws nothing until the session exists. These tests send a local file in a new thread, a workspace file in an existing one, a file while the expert is still replying, and a file queued behind the kickoff, and expect the message on screen at once and file_ids on /stream. They fail until the fix lands. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…SendMessage The upload, placeholder and send steps lived inside the /copilot hook, so the expert side panel could not reuse them and sent text only. They now live in helpers/sendWithAttachments.ts as one OutgoingMessage type and a sendWithAttachments() both surfaces can call, with an optional waitUntilReady step between the uploads and the send. The settle waiters in useCopilotPage move to useSettleGate so the panel can wait on its own reply the same way. No behaviour change on /copilot. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…d show the message at once The side panel's onSend took only the text, so files and workspace files attached in it never reached the expert, and nothing was drawn until the session existed, which left the empty state up for seconds after Send. Every send path in the panel now goes through sendWithAttachments, so local files upload into the session and go out as file_ids with the message, in new and existing threads and behind the kickoff. The message shows from the first render after Send: held in the panel until a session exists, then as the session's upload placeholder. A follow-up with files sent while the expert is replying uploads at once and goes out as the next turn when the reply ends, because the pending buffer would store its files without their names. If the user starts a new task before it goes out, its text comes back to the composer. The composer now gets the expert's id and name, as on /copilot, so the workspace picker offers that expert's files. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…replies The composer closed the attach menu, paste and workspace mentions for the whole reply, and its Queue button only carried text, so a file could not be sent mid-reply even though the drawer now holds such a message and sends it as the next turn. ChatInput takes `canAttachWhileStreaming`. With it, attaching stays open during a reply, and Queue message (like Enter) sends a message that has attachments through `onSend`. Only the side panel sets it: /copilot still refuses to queue files. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…nd through a reconnect Two ways a message with files could go wrong after Send: - It was sent with whatever `sendMessage` the panel held when the uploads finished. After New task, that belongs to the new thread, so the old task's text and files were posted to it. A send now checks that its thread is still on screen; if not, the text goes back to the composer. - A follow-up held until the reply ended was released on `status` `ready`/`error` alone. The SDK sets `status` on React's sync lane, but the stream's reconnect and post-finish probe flags render one pass later, so the send went out while the stream was reconnecting. The stream dropped it and the bubble disappeared. The panel now waits for `isReconnecting` and `isFinishProbing` too, and confirms the turn is still over once those later updates have rendered (`useNextCommit`). Tests now attach through the visible menu and send with the Queue button and Enter. They also cover a workspace file picked mid-reply, an upload that outlives its thread, and a connection dropped while a file is held. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…anel closes, and over a new draft Three ways the expert side panel still loses or locks a message with files: - a file sent while the connection is being restored is cleared from the screen and never sent; - closing the panel while a file uploads leaves the thread's composer locked when it is opened again, with or without a reply in flight; - a message sent back to the composer after New task replaces what the user has typed in the new task. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…nel and a new draft Every message with files now goes out through one send that waits until the reply in flight has ended. Before, that wait only applied when the SDK said a reply was streaming. A message sent while the connection was being restored, or right after a kickoff reply, skipped it, and the stream then refused the send without telling its caller. The settle gate and the commit barrier return false once their host has unmounted. An upload that finished after the panel closed used to wait on them forever, so the thread's placeholder stayed and its composer stayed locked. A message the panel hands back goes into a new recovered-draft slot. The composer adds it to what the user has typed instead of replacing it. Guided prompts keep the replace behaviour of `initialPrompt`. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
WalkthroughCopilot and Expert Chat now share attachment-aware message handling. Expert Chat can hold attachment-bearing messages until the current reply ends, including during reconnection, and display pending sends. The changes also add upload validation, failed-draft recovery, and attachment-flow tests. ChangesAttachment-aware chat
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant ChatInput
participant useExpertChatDrawer
participant useExpertChatSend
participant sendWithAttachments
participant useCopilotStream
ChatInput->>useExpertChatDrawer: Submit outgoing message
useExpertChatDrawer->>useExpertChatSend: Send message with attachments
useExpertChatSend->>useExpertChatSend: Wait for reply settlement
useExpertChatSend->>sendWithAttachments: Upload and prepare attachments
sendWithAttachments->>useCopilotStream: Send message parts
Merge Risk: ⚪ Minimal · up to
|
|
/review |
|
🤖 Review of |
…956-expert-side-panel-chat-drops-attached-files-message-should-p1 Resolve the ChatInput.tsx conflict by keeping both props: this PR's canAttachWhileStreaming and dev's atReset. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🔴 Merge Conflicts DetectedThe following PRs have been tested and will have merge conflicts if merged after this PR. Consider coordinating with the authors.
🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 3 conflict(s), 0 medium risk, 5 low risk (out of 8 PRs with file overlap) Auto-generated on push. Ignores: |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #15582 +/- ##
========================================
Coverage 86.87% 86.87%
========================================
Files 4947 4951 +4
Lines 378328 378471 +143
Branches 33657 33689 +32
========================================
+ Hits 328656 328814 +158
+ Misses 44196 44179 -17
- Partials 5476 5478 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…st one The duplicate-send guard compared only text, so a held side-panel follow-up with a file and the same caption as the previous message was dropped as a resend. The guard now also compares the attachments. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…pped Stop ended the reply and the held message with files went out right after, while the backend discards buffered text follow-ups on cancel. Now Stop cancels it, puts its text back in the composer and says why. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
There was a problem hiding this comment.
✅ Verdict: Approved — No findings require changes before approval; this review counts as an approval.
Well-structured frontend refactor extracting the upload-then-send path into a shared helper and wiring attached files through the expert side-panel chat; no reachable defect found and the concurrency-heavy paths are covered by passing integration tests. Required CI is still pending and the branch currently conflicts with its base, which must be resolved before merge.
Risk level: low | Human review: not required | Duration: 369s | Reviewed: b9cf1507
Findings: 🔴 0 blockers | 🟠 2 should fix | 🟡 1 nice to have | 🔵 1 nit
Optional advice — does not block approval
Follow-ups
- 🟠
autogpt_platform/frontend/src/app/(platform)/team/components/ExpertChatDrawer/useExpertChatDrawer.ts:303Stabilize send/release so drawer effects fire on sessionId, not every render — This effect and the kickoff-dequeue effect at line 249 depend onsend, which useExpertChatSend recreates every render, so both re-run every render. They avoid duplicate sends only because pendingPromptRef/kickoffAttemptRef are nulled synchronously inside each effect; a future edit to that guard would turn the benign re-run into a duplicate send.
Suggestion: Stabilize send/release behind a ref (as useSendMessage.ts does with dispatchRef) so the effects trigger on sessionId change only. - 🟠
autogpt_platform/frontend/src/app/(platform)/team/components/ExpertChatDrawer/useExpertChatSend.ts:128Scope recoveredDraft so a dropped drawer message can't reappear in /copilot — recoverUnsentMessage/returnToComposer write to the global useCopilotUIStore. If the drawer unmounts before useChatInput consumes recoveredDraft, the text persists and is appended into the next-mounted consumer (e.g. /copilot). No data loss since it appends, and the window is narrow.
Suggestion: Scope the recovery slot per surface or clear recoveredDraft on drawer unmount.
Optional advice — does not block approval
🟡 Nice to Have
- 🔵
autogpt_platform/frontend/src/app/(platform)/team/components/ExpertChatDrawer/ExpertChatDrawer.tsx:190Keep composer usable while a mid-reply attachment is merely held — isUploadingFiles is bound to !!pendingSend, which stays true for the whole reply while a follow-up attachment is held, disabling the textarea the entire time rather than only during the upload. Explicitly deferred to PR 2.
Suggestion: Distinguish 'actively uploading' from 'held waiting for reply to end'; acceptable to defer with the PR 2 per-file-status work.
Optional advice — does not block approval
🔵 Nits
- 🔵
autogpt_platform/frontend/src/app/(platform)/team/components/ExpertChatDrawer/__tests__/attachments.test.tsx:254Reset useCopilotUIStore in attachments test beforeEach — beforeEach resets the stream store and localStorage but not useCopilotUIStore, which now holds mutable cross-test state (recoveredDraft, initialPrompt). No leak today, but a future test could become order-dependent.
Suggestion: Add useCopilotUIStore.setState({ recoveredDraft: null, initialPrompt: null }) to beforeEach.
GitHub CI on reviewed head: failure
Validation and specialist details
Specialist Reports
| Specialist | Status | Summary |
|---|---|---|
| security | ✅ PASS | Frontend-only chat composer refactor with no new security surface; workspace-file scoping is correctly tightened via expertId and file-size/count validation is preserved. |
| architect | ✅ PASS | Well-structured extraction that routes drawer attachment sends through the shared upload-then-send path with carefully-correct settle/commit timing and thread-identity guards; no blocking defects, one low maintainability note. |
| performance | ✅ PASS | Client-side upload/send coordination with no reachable performance, memory, or scalability defects; uploads are parallelized and all async-gate waiters are cleaned up on settle and unmount. |
| testing | ✅ PASS | Comprehensive, assertion-strong integration tests directly verify the attachment-send fix and every hold/recovery path; no blocking test defects found. |
| quality | ✅ PASS | Clean extraction of the upload-then-send path into shared helpers plus a well-tested fix that sends side-panel attachments and renders the user's message on the first frame; no concrete defects found. |
| product | ✅ PASS | Attachment sending from the expert side-panel is implemented correctly across all send states with the message shown on the first frame; copy rules respected and edge cases covered by tests, with one advisory composer-disabled UX limitation. |
| discussion | ✅ PASS | No open human-reviewer threads; required checks pending, but the branch has a merge conflict with its base that must be resolved. |
| ui-reviewer (local) | ✅ PASS | Frontend fix wiring attached files through the expert side-panel chat is sound and well-covered; targeted drawer/ChatInput/useSendMessage test suites pass (99 tests) and prop/store contracts verify, with only a low-severity cross-surface recoveredDraft advisory. |
| ui-reviewer (hosted) | ✅ PASS | The drawer now forwards attached files/workspace files to sendWithAttachments so the expert side panel uploads and sends file_ids; logic is sound and the new plus existing copilot integration suites pass (184 tests) with no regressions. |
EvidenceVerdict: PASS at
Requests the browser recorded: Also: 277/277 targeted tests across 15 files (drawer Notes: while a held follow-up waits for the reply to end, its bubble still says "Uploading 1 file…" after the upload has finished. PR 2 replaces that flag with a status for each file. The "Connection lost / Reconnecting…" toast in some shots comes from the stream's existing resume logic, which this PR does not change. The held files went out in every run. |
|
/review |
|
🤖 Review of |
There was a problem hiding this comment.
✅ Verdict: Approved — No findings require changes before approval; this review counts as an approval.
A disciplined frontend refactor that extracts shared upload-then-send/settle-gate logic and wires attachments through the expert side-panel chat with a flash-free held-message handoff. Every send path and the duplicate-suppression change were traced and are covered by strong, deterministic integration tests; no reachable defect found and CI is green on the reviewed head.
Risk level: low | Human review: not required | Duration: 480s | Reviewed: 2080f378
Findings: 🔴 0 blockers | 🟠 1 should fix | 🟡 2 nice to have | 🔵 0 nits
Optional advice — does not block approval
Follow-ups
- 🟠
autogpt_platform/frontend/src/app/(platform)/team/components/ExpertChatDrawer/ExpertChatDrawer.tsx:190Relax composer lock while a held follow-up waits on the reply — isUploadingFiles={!!pendingSend} disables the textarea and attach menu for the entire time a follow-up is held, including the full in-flight reply it waits on, not just the byte upload. Stop stays enabled so the user is not trapped.
Suggestion: In the planned PR 2, gate the textarea lock on actual upload-in-progress rather than on any pendingSend.
Optional advice — does not block approval
🟡 Nice to Have
- 🔵
autogpt_platform/frontend/src/app/(platform)/copilot/helpers/sendWithAttachments.ts:171Collapse the stacked upload-failure toasts — A single failed local upload can surface up to three stacked destructive toasts (per-file, aggregate, and send-failure). Inherited from the existing /copilot path, not introduced here.
Suggestion: Collapse the cascade into one message in a follow-up. - 🔵
autogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts:318Confirm duplicate-suppression asymmetry is intended — Resending the same caption with fewer/no attachments than the previous message is no longer suppressed. Defensible as a materially different message but a behavior change beyond the stated fix.
Suggestion: Confirm the asymmetry is intended; no code change required.
GitHub CI on reviewed head: success
Validation and specialist details
Specialist Reports
| Specialist | Status | Summary |
|---|---|---|
| security | ✅ PASS | Frontend chat attachment refactor with server-enforced file scope/limits and an explicit same-session guard against cross-thread send; no concrete security defects found. |
| architect | ✅ PASS | Well-structured refactor that extracts shared upload-then-send/settle-gate logic and wires it into the expert drawer with correct hold/release, kickoff, reconnect and Stop handling; no reachable defect found and coupling stays unidirectional. |
| performance | ✅ PASS | Frontend attachment-send extraction and drawer hold/settle logic are carefully guarded against the cross-thread and lifecycle races they introduce; no concrete performance or correctness defects found. |
| testing | ✅ PASS | The new MSW integration suite and targeted helper/ChatInput tests cover every fixed behavior with strong, deterministic, regression-catching assertions; no concrete test defect found. |
| quality | ✅ PASS | Well-structured refactor that correctly fixes attachment sending in the expert side panel with no concrete defects found; the shared /copilot path is behavior-preserving and copy rules are respected. |
| product | ✅ PASS | Attachment send paths in the expert side panel are correctly unified, the held-message handoff is flash-free, and error/Stop/reconnect edge cases are handled; no concrete blocker found. |
| discussion | ✅ PASS | CI is green on the current head and there are no open human-reviewer requests. |
| ui-reviewer (local) | ✅ PASS | Source review of the expert side-panel attachment send/hold logic found no reachable defect; every send path (new thread, existing thread, mid-reply, kickoff, stop, new-task, reconnect) and the duplicate-suppression change are coherent and match the added tests. |
| ui-reviewer (hosted) | ✅ PASS | Frontend-only fix that correctly uploads and sends side-panel chat attachments; targeted and full copilot test suites pass (3918+164) with no regression and no reachable defect found. |















































Part of SECRT-2956
https://linear.app/autogpt/issue/SECRT-2956
This is PR 1 of 3. It fixes the bug. PR 2 adds per-file upload status with Retry, and PR 3 adds the new attachment cards.
Why / What / How
Why. In the Team page's expert side panel, a file attached to a message never reached the expert. The composer showed the file and cleared it on Send, but the drawer's
onSend(message: string)ignored thefilesandworkspaceFilesarguments thatChatInputpasses. Nothing was uploaded and nofile_idswent to/stream. In a new thread the panel also stayed on the "What can I do for you?" empty state until session creation returned (about 3 s), then showed the message as text only.What. The side panel now uploads local files and workspace files and sends them with the message, in a new thread, in an existing thread, while the expert is still replying, and behind the expert's kickoff turn. The user's message and its files show on the first frame after Send, before any network call returns.
How. The upload-then-send code from
/copilot'suseSendMessagemoves into a plain module,sendWithAttachments, and both surfaces call it. On/copilotthe code moves without changes, and its tests pass unedited. The drawer keeps its own session and kickoff logic and holds the message locally until a session exists, so it never uses/copilot's global first-send slot. A follow-up with files sent mid-reply does not go through/messages/pending, because that buffer stores files without names and the transcript would drop them. The drawer holds it instead: the bubble shows and the upload starts right away, then the message goes out as the next turn once the reply has ended. Text-only follow-ups still use the buffer.Changes 🏗️
copilot/helpers/sendWithAttachments.ts(new):OutgoingMessage,validateAttachments,describePendingUploadandsendWithAttachments({ sessionId, message, sendMessage, waitUntilReady? }), moved out ofuseSendMessage.ts.waitUntilReadyruns after the uploads and before the send; returningfalsedrops the placeholder.copilot/useSendMessage.ts: calls the helper. No behaviour change on/copilot.copilot/useSettleGate.ts(new): the "wait until the reply has ended" gate, taken out ofuseCopilotPage.ts, which now uses it. After unmount it resolvesfalserather than hanging.team/components/ExpertChatDrawer/useExpertChatSend.ts(new): holds the user's message on screen until it can be sent, and sends every message with attachments through one gatedsend(). Before sending, it checks that the message's thread is still the one on screen and that the reply has fully ended, including any reconnect or finish probe.team/components/ExpertChatDrawer/useNextCommit.ts(new): waits one React commit, so the gate does not open on anerrorstatus the render before the stream marks itself as reconnecting.useExpertChatDrawer.ts:onSend(text, files?, workspaceFiles?)builds anOutgoingMessage. Every send path (first send after create, kickoff dequeue, the not-active fallback, a plain send) goes throughsend(). Thread resets clear the held message.ExpertChatDrawer.tsx: renders the transcript when there is a session or a pending message, keeps onboarding hidden while a message is pending, and passesisUploadingFiles,expertIdandexpertNametoChatInput. WithoutexpertId, the backend rejects workspace files outside the expert's scope.ChatInput: new opt-incanAttachWhileStreamingprop, set only by the drawer, so files can be attached and queued while the expert replies./copilot, the builder chat and memory chat keep today's behaviour.copilot/store.tsanduseChatInput.ts: arecoveredDraftslot. If a held message can't go out (for example after the user starts a new task during its upload), its text is added to the composer next to any new draft instead of replacing it, with a "Message not sent" toast.copilot/helpers.ts:getSendSuppressionReasonnow calls a send a duplicate only when both the text and the attachments match the last user message. Before, a new file sent with the same caption as the last message was dropped, in the drawer and on/copilottoo.useExpertChatSendcounts Stops, and a held send that sees a Stop after it was queued does not go out. Its text goes back to the composer with a "Message not sent" toast. This matches the backend, which throws away buffered follow-ups when a reply is cancelled.ExpertChatDrawer/__tests__/attachments.test.tsx(new, MSW). It covers a new thread, an existing thread with a workspace file, mid-reply hold, kickoff hold, a dropped connection, a send during a reconnect, closing the panel during an upload, and a new task during an upload. Plus aChatInputtest for queuing files during a reply.Known limits, as the plan expected:
Evidence 📸
Verdict: PASS at
2080f3787(round 5). This replaces the earlier evidence taken atb9cf15078. Since then the branch mergeddev, which changed the shared composer, and added the duplicate-send fix and the Stop fix, so every row was proven again on the running app (Docker deps, native backend,next dev). Synthetic users and data only, reduced motion on, 1440×1000 and 390×844. No model key was available, so a scripted provider answered the turns. There are no "before" shots because there was no dev URL to capture from./streamcarriesfile_ids(create → upload?session_id=→/stream)file_ids/messages/pendingcallfile_idsfile_ids/copilotnew chat still shows the "Uploading 1 file…" bubble/copilotexisting chat still sends a file withfile_ids2080f3787): Stop while a follow-up with a file is held. The bubble goes, the text is back in the composer with a "Message not sent" toast, and no/streamwith files goes outname (type, size), file_id=…).Requests the browser recorded:
Also: 277/277 targeted tests across 15 files (drawer
__tests__,ChatInput__tests__,useSendMessage,CopilotPage,helpers.test).Notes: while a held follow-up waits for the reply to end, its bubble still says "Uploading 1 file…" after the upload has finished. PR 2 replaces that flag with a status for each file. The "Connection lost / Reconnecting…" toast in some shots comes from the stream's existing resume logic, which this PR does not change. The held files went out in every run.
Agents and large language models used
Checklist 📋
For code changes:
2080f3787: 277/277 across 15 targeted files in the evidence step (drawer__tests__,ChatInput__tests__,useSendMessage,CopilotPage,helpers.test). Earlier, in review atb9cf15078, 308/308 across 25 files.attachments.test.tsxfailed before the fix and passes after it, and the two drawer tests added for the duplicate-caption and Stop fixes fail on the code before them.pnpm format,pnpm lintandpnpm typesare clean.next dev), 1440 and 390 wide: new thread with a local image. The bubble shows 50 ms after the click (52 ms at 390) while session create is held back, then the upload runs, then/streamwithfile_ids.file_ids./messages/pendingcall, and goes out as the next turn withfile_ids.file_ids./copilotstill sends files in new and existing chats, with the "Uploading 1 file…" bubble.name (type, size), file_id=…).pnpm test:unitand Playwright suites (CI).For configuration changes:
No configuration changes.
.env.defaultis updated or already compatible with my changesdocker-compose.ymlis updated or already compatible with my changes🤖 Generated with Claude Code
Open workspace in Conductor