diff --git a/README.md b/README.md index b2abd8319e..8982c4be8c 100644 --- a/README.md +++ b/README.md @@ -2114,6 +2114,8 @@ When the agent asks for clarification, the Web UI shows the structured response Unsent Web UI composer drafts survive page reloads and switching between conversations within the same browser tab. Drafts are isolated by user, agent, and conversation, include the selected skill and explicit conversation references, and are cleared once a send is accepted. Project documents already attached to the thread also survive reloads; browser-local uploads and quoted message excerpts are not persisted. +If an attachment upload reports `success: false`, the Web UI stops the send even when the HTTP response is 200. The upload error is shown and the composer keeps its draft and attachments for correction or retry. Files already accepted by a partially successful upload remain on the server; this does not roll them back. + The Web UI composer supports `@` references at the cursor or through the `@` button. Search enabled skills, documents in the current project, and other conversations in a grouped picker; selected references appear inline alongside the draft text and can be removed with normal editing. Multiple skills can be selected in one message (up to 16 unique skills); selecting a checked skill again removes it, with each selection checked against the current user registry and agent allowlist. Project files are attached only after ingestion succeeds; conversation references require the `read_conversation` capability and retain its per-run limit. If capability discovery fails, saved conversation references remain intact and sending waits for a successful retry. Polishing preserves complete reference labels when instructions are reordered. The file group also offers local uploads. `/goal`, `/compact`, and legacy `/skill-name` input remain supported. After picking an inline reference, typing continues immediately after it. Moving the cursor keeps the position you chose, including when editing text beside a reference. diff --git a/frontend/src/AGENTS.md b/frontend/src/AGENTS.md index 0770083fae..55ccf9a61d 100644 --- a/frontend/src/AGENTS.md +++ b/frontend/src/AGENTS.md @@ -225,7 +225,8 @@ export on a page error or non-advancing cursor; demos keep their fixture path. Model picker, chat entry pages, branch tree, and browser-control ownership are detailed in `frontend/docs/conversation-ui-ownership.md`. -- `src/core/threads/hooks.ts` owns pre-submit upload state and thread submission. The optimistic human bubble (before and after upload) and the submitted message all build `additional_kwargs` through `buildHumanMessageAdditionalKwargs`, so quote/reference metadata and staged project files survive the upload; never replace them with only the uploaded files. +- `core/threads/hooks.ts` owns uploads/sends. Both optimistic and submitted humans use `buildHumanMessageAdditionalKwargs` to retain quote/reference metadata and staged project files. +- `core/uploads/api.ts` rejects `success: false` before send or composer clear. Keep the existing retry path; accepted files are not rolled back. - `src/components/workspace/chats/chat-box.tsx` owns the desktop right-panel layout, and **all three** right panels (artifacts, sidecar, browser) share one `ResizablePanelGroup` — do not fork a non-resizable branch per panel kind, which is how the artifacts divider silently lost its drag handle (#4465). Open/close is `collapse()` / `resize()` on the side panel's imperative handle, not conditional rendering, so the width can animate. Three constraints hold that together: the size transition is applied from the group as `[&>[data-panel]]:transition-[flex-grow]` because the sized flex item is the library's own `[data-panel]` element rather than the child `className` lands on; it is applied only while an open/close is in flight, so a drag is not interpolated frame by frame; and during the animation the panel content is held at its final width in `cqw` and clipped, because a reflowing message list re-runs its scroll-to-bottom (pinned by `tests/e2e/sidecar-chat.spec.ts`'s no-animated-scroll test) and a re-wrapping composer changes which responsive labels it shows. Because the panel is `collapsible`, the library can also collapse it to `0%` on its own when a drag crosses `minSize`, without going through the state that owns it. `onResize` records the last positive size while the pointer moves, but the owning `sidecar` / `browserView` / `artifactsOpen` state must only mirror a final `0%` layout from `onLayoutChanged`, after pointer release; closing on the first `0%` resize frame breaks a continuous drag that reaches the edge and then reverses before release. `ask_clarification` ToolMessages delimit runs despite hidden human replies; skill diff --git a/frontend/src/core/uploads/api.ts b/frontend/src/core/uploads/api.ts index 9115f98fc5..501cfe8c04 100644 --- a/frontend/src/core/uploads/api.ts +++ b/frontend/src/core/uploads/api.ts @@ -145,7 +145,15 @@ export async function uploadFiles( throw new Error(await readErrorDetail(response, "Upload failed")); } - return response.json(); + const result = (await response.json()) as UploadResponse; + if (result.success === false) { + const message = result.message.trim(); + throw new Error( + message ? `Upload incomplete: ${message}` : "Upload failed", + ); + } + + return result; } /** diff --git a/frontend/tests/unit/components/workspace/input-box-failed-upload.dom.test.tsx b/frontend/tests/unit/components/workspace/input-box-failed-upload.dom.test.tsx index e1a2d1df92..665cbe8d42 100644 --- a/frontend/tests/unit/components/workspace/input-box-failed-upload.dom.test.tsx +++ b/frontend/tests/unit/components/workspace/input-box-failed-upload.dom.test.tsx @@ -138,9 +138,11 @@ function renderChat() { } let uploadAttempts = 0; +let firstUploadResponse: () => Response; beforeEach(() => { uploadAttempts = 0; + firstUploadResponse = () => json({ detail: "disk full" }, 500); mocks.submit.mockClear(); // happy-dom's File is not a Node Blob, so the composer's preview URL needs // a stand-in. @@ -151,7 +153,7 @@ beforeEach(() => { if (/\/api\/threads\/[^/]+\/uploads$/.test(url)) { uploadAttempts += 1; return uploadAttempts === 1 - ? json({ detail: "disk full" }, 500) + ? firstUploadResponse() : json(UPLOAD_RESPONSE); } if (/\/runs(\?|$)/.test(url)) { @@ -168,7 +170,7 @@ afterEach(() => { window.sessionStorage.clear(); }); -it("retries a failed upload with the staged project attachment still attached", async () => { +async function expectUploadRetryPreservesDraftAndAttachments() { stageProjectAttachment("thread-1", ATTACHMENT); const { container } = renderChat(); await screen.findByTestId("project-attachment-chip"); @@ -191,6 +193,10 @@ it("retries a failed upload with the staged project attachment still attached", ); expect(mocks.submit).not.toHaveBeenCalled(); + expect(screen.getByRole("textbox").value).toBe( + "summarize it", + ); + expect(screen.getByText("notes.txt")).toBeTruthy(); expect(screen.getByTestId("project-attachment-chip")).toBeTruthy(); fireEvent.submit(container.querySelector("form")!); @@ -208,7 +214,23 @@ it("retries a failed upload with the staged project attachment still attached", ATTACHMENT.virtual_path, "/mnt/user-data/uploads/notes.txt", ]); -}); +} + +it.each(["HTTP error", "unsuccessful HTTP 200"])( + "retries an upload after %s with the draft and staged project attachment still attached", + async (failure) => { + if (failure === "unsuccessful HTTP 200") { + firstUploadResponse = () => + json({ + success: false, + files: [], + message: "Successfully uploaded 0 file(s); skipped 1 unsafe file(s)", + skipped_files: ["notes.txt"], + }); + } + await expectUploadRetryPreservesDraftAndAttachments(); + }, +); it("keeps the draft through a failed upload and a busy /goal, then runs the goal's objective once", async () => { // A /goal the Gateway refuses with 409 never reaches sendMessage, so like diff --git a/frontend/tests/unit/core/uploads/api.test.ts b/frontend/tests/unit/core/uploads/api.test.ts index 72ed3fdbb0..105d307e7d 100644 --- a/frontend/tests/unit/core/uploads/api.test.ts +++ b/frontend/tests/unit/core/uploads/api.test.ts @@ -40,6 +40,60 @@ beforeEach(() => { }); describe("uploads api", () => { + test("returns a successful upload response unchanged", async () => { + const result = { + success: true, + files: [], + message: "Successfully uploaded 0 file(s)", + skipped_files: [], + }; + mockedFetch.mockResolvedValueOnce(jsonResponse(200, result)); + + await expect(uploadFiles("thread-1", [])).resolves.toEqual(result); + }); + + test.each([ + ["all skipped", []], + [ + "partially uploaded", + [ + { + filename: "accepted.txt", + size: 5, + path: "/data/uploads/accepted.txt", + virtual_path: "/mnt/user-data/uploads/accepted.txt", + artifact_url: "/api/threads/thread-1/artifacts/accepted.txt", + }, + ], + ], + ])("rejects an unsuccessful HTTP 200 upload (%s)", async (_label, files) => { + const message = `Successfully uploaded ${files.length} file(s); skipped 1 unsafe file(s)`; + const error = await uploadError( + jsonResponse(200, { + success: false, + files, + message, + skipped_files: ["report.pdf"], + }), + ); + + expect(error.message).toContain("Upload incomplete"); + expect(error.message).toContain(message); + }); + + test("uses the upload fallback when an unsuccessful response has no message", async () => { + const error = await uploadError( + jsonResponse(200, { + success: false, + files: [], + message: "", + skipped_files: ["report.pdf"], + }), + ); + + expect(error.message).toBe("Upload failed"); + }); + test("preserves string error details", async () => { const error = await uploadError( jsonResponse(413, { detail: "File exceeds the upload limit" }),