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

[dovvnloading]

The problem

Attaching several files at once and having one fail discards every file that already uploaded.

Those files are not merely missing from the UI. Each had already been uploaded and had a real backend artifact holding retention, so they are orphaned until it expires — and the user has to re-add the rest by hand, with no indication which ones made it.

Root cause

addAttachments accumulated into a local staged array and merged it into the composer draft only after the loop finished:

const staged: ChatAttachment[] = [];
for (const file of files.slice(0, remaining)) {
  if (!file.size || file.size > MAX_CHAT_ATTACHMENT_BYTES) throw new Error(...);
  if (totalBytes + file.size > MAX_CHAT_ATTACHMENT_TOTAL_BYTES) throw new Error(...);
  staged.push(await api.stageChatAttachment(...));
}
// merge into the draft -- only reached if every file succeeded

Any throw inside the loop — an oversized file, the combined-size ceiling, or a staging request failing — jumps straight to the handler, and staged goes out of scope unused. Three separate conditions reach it, and the second and third can only trigger after earlier files have already uploaded.

The merge is now a small helper called on both paths, so a partial batch keeps what genuinely uploaded and the message says so instead of reporting a bare failure.

A second, quieter one in the same function

for (const file of files.slice(0, remaining)) {

Files beyond the remaining slots were dropped in silence — a message existed only for the fully-full case (!remaining). Choosing nine files with eight slots attached eight and looked exactly like attaching nine.

It now reports how many were taken. The hardcoded "eight" in the neighbouring message became MAX_CHAT_ATTACHMENTS while I was there, since the new message reads from the same constant and the two must not drift.

Verification

Two new tests, both failing against the unfixed code:

• two files where the second fails staging: the first still appears in the composer, and the notice says the earlier files were kept
• nine files with eight slots: exactly eight are staged and the notice reads Only 8 of 9 files were attached

Check	Result
npm test -- --run	268 passed, 31 files
npm run typecheck	clean
npm run lint	clean
python -m pytest -q	907 passed

Compatibility and rollback

Frontend only, confined to one function. No API contract, stored data, or migration. The all-succeed path behaves exactly as before. Reverting the commit restores the previous behaviour exactly.

Unrelated observation

While running the backend suite for this change I hit a rare pre-existing flake: test_code_execution.py::test_cancelling_an_approved_but_unleased_code_job_reaches_a_terminal_status failed once in roughly five full-suite runs, then passed on four consecutive full runs here and two on main. This change is frontend-only and cannot affect it, but a non-deterministic test on a cancellation path is worth a look on its own — flagging it rather than leaving it in the scrollback.

🤖 Generated with Claude Code