fix(frontend): keep the attachments that uploaded when one in the batch fails

frontend/src/features/chat/ChatPage.test.tsx

@@ -1155,6 +1155,66 @@ describe("ChatPage composer integration", () => {
     expect(screen.getByRole("button", { name: "Remove next.md" })).toBeInTheDocument();
   });
 
+  it("keeps the files that uploaded when a later one in the batch fails", async () => {
+    // Each staged file is already uploaded and already holding backend
+    // retention. Committing the batch only after the whole loop meant one bad
+    // file discarded every good one before it, orphaning those artifacts and
+    // making the user re-add the rest by hand.
+    const user = userEvent.setup();
+    const uploaded: ChatAttachment = {
+      attachment_id: "doc-good",
+      filename: "good.md",
+      mime_type: "text/markdown",
+      size: 4,
+      sha256: "a".repeat(64),
+      kind: "document",
+      expires_at: "2099-01-01T00:00:00Z",
+    };
+    const api = chatApi({
+      stageChatAttachment: vi.fn()
+        .mockResolvedValueOnce(uploaded)
+        .mockRejectedValueOnce(new Error("the second file could not be staged")),
+    });
+    renderChat(api);
+
+    const attachmentInput = await screen.findByLabelText("Attach images or documents");
+    await user.upload(attachmentInput, [
+      new File(["good"], "good.md", { type: "text/markdown" }),
+      new File(["bad"], "bad.md", { type: "text/markdown" }),
+    ]);
+
+    await waitFor(() => expect(api.stageChatAttachment).toHaveBeenCalledTimes(2));
+    expect(await screen.findByRole("button", { name: "Remove good.md" })).toBeInTheDocument();
+    expect(await screen.findByRole("alert")).toHaveTextContent("Files attached before it were kept.");
+  });
+
+  it("says so when more files are chosen than a message can carry", async () => {
+    // Silently dropping the overflow looked exactly like attaching it.
+    const user = userEvent.setup();
+    const api = chatApi({
+      stageChatAttachment: vi.fn(async (request: { filename: string }) => ({
+        attachment_id: `id-${request.filename}`,
+        filename: request.filename,
+        mime_type: "text/markdown",
+        size: 4,
+        sha256: "b".repeat(64),
+        kind: "document" as const,
+        expires_at: "2099-01-01T00:00:00Z",
+      })),
+    });
+    renderChat(api);
+
+    const attachmentInput = await screen.findByLabelText("Attach images or documents");
+    await user.upload(
+      attachmentInput,
+      Array.from({ length: 9 }, (_unused, index) =>
+        new File([`f${index}`], `file-${index}.md`, { type: "text/markdown" })),
+    );
+
+    await waitFor(() => expect(api.stageChatAttachment).toHaveBeenCalledTimes(8));
+    expect(await screen.findByRole("alert")).toHaveTextContent("Only 8 of 9 files were attached");
+  });
+
   it("explains the image capability mismatch before a generation request is made", async () => {
     const user = userEvent.setup();
     const attachment: ChatAttachment = {

frontend/src/features/chat/ChatPage.tsx

@@ -637,12 +637,36 @@ export function ChatPage({
     attachmentDraftTargetsRef.current.add(target);
     setAttachmentsBusy(true);
     setAttachmentError(null);
+    // Each staged file is already uploaded and already holding backend
+    // retention, so it belongs in the composer whether or not a later file in
+    // the same batch fails. Committing only after the whole loop meant one bad
+    // file discarded every good one before it -- leaving those artifacts
+    // orphaned on the backend and making the user re-add the rest by hand.
+    const staged: ChatAttachment[] = [];
+    const commitStaged = () => {
+      if (!staged.length) return;
+      // The generation request and attachment staging can finish in either
+      // order. Merge into the latest scoped draft instead of the render-time
+      // `attachments` snapshot, which may contain files that were submitted
+      // and cleared while these new files were still uploading.
+      const currentAttachments = attachmentDraftsRef.current[target.scope]
+        ?? readComposerAttachments(target.threadId);
+      const currentAttachmentIds = new Set(currentAttachments.map((attachment) => attachment.attachment_id));
+      const next = [
+        ...currentAttachments,
+        ...staged.filter((attachment) => !currentAttachmentIds.has(attachment.attachment_id)),
+      ];
+      const nextAttachments = { ...attachmentDraftsRef.current, [target.scope]: next };
+      attachmentDraftsRef.current = nextAttachments;
+      setAttachmentDrafts(nextAttachments);
+      writeComposerAttachments(target.threadId, next);
+    };
     try {
       const remaining = Math.max(0, MAX_CHAT_ATTACHMENTS - attachments.length);
-      if (!remaining) throw new Error("A message can include at most eight attachments.");
+      if (!remaining) throw new Error(`A message can include at most ${MAX_CHAT_ATTACHMENTS} attachments.`);
       let totalBytes = attachments.reduce((total, attachment) => total + attachment.size, 0);
-      const staged: ChatAttachment[] = [];
-      for (const file of files.slice(0, remaining)) {
+      const accepted = files.slice(0, remaining);
+      for (const file of accepted) {
         if (!file.size || file.size > MAX_CHAT_ATTACHMENT_BYTES) {
           throw new Error(`${file.name} is empty or larger than 10 MB.`);
         }
@@ -658,23 +682,17 @@ export function ChatPage({
         staged.push(attachment);
         totalBytes += attachment.size;
       }
-      // The generation request and attachment staging can finish in either
-      // order. Merge into the latest scoped draft instead of the render-time
-      // `attachments` snapshot, which may contain files that were submitted
-      // and cleared while these new files were still uploading.
-      const currentAttachments = attachmentDraftsRef.current[target.scope]
-        ?? readComposerAttachments(target.threadId);
-      const currentAttachmentIds = new Set(currentAttachments.map((attachment) => attachment.attachment_id));
-      const next = [
-        ...currentAttachments,
-        ...staged.filter((attachment) => !currentAttachmentIds.has(attachment.attachment_id)),
-      ];
-      const nextAttachments = { ...attachmentDraftsRef.current, [target.scope]: next };
-      attachmentDraftsRef.current = nextAttachments;
-      setAttachmentDrafts(nextAttachments);
-      writeComposerAttachments(target.threadId, next);
+      commitStaged();
+      if (accepted.length < files.length) {
+        // Dropping the overflow silently looked exactly like attaching it.
+        setAttachmentError(
+          `Only ${accepted.length} of ${files.length} files were attached; a message can include at most ${MAX_CHAT_ATTACHMENTS}.`,
+        );
+      }
     } catch (error) {
-      setAttachmentError(error instanceof ApiError ? error.detail : error instanceof Error ? error.message : "The attachment could not be uploaded.");
+      commitStaged();
+      const detail = error instanceof ApiError ? error.detail : error instanceof Error ? error.message : "The attachment could not be uploaded.";
+      setAttachmentError(staged.length ? `${detail} Files attached before it were kept.` : detail);
     } finally {
       attachmentDraftTargetsRef.current.delete(target);
       setAttachmentsBusy(false);