refactor: never delete chat uploads from workspace storage

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

chat-sdk/README.md

@@ -252,9 +252,10 @@ message lists them in `attachments`, as `{ input, s3, filename }` references. Th
 extension is corrected to the file's media type for PNG, JPEG and PDF, because the worker
 reads the type off the key.
 Files need message text to go with them. A failed upload rejects `sendMessage` before any
-run starts, and `stop()` during the upload aborts it; both leave the transcript as it was. The workspace needs object
-storage set up. With Enterprise advanced storage permissions, the user needs read, write
-and delete on `windmill_uploads/*`, which the default rules grant. The upload goes through
+run starts, and `stop()` during the upload aborts it; both leave the transcript as it was.
+The chat never deletes uploads, so files of a send that did not run stay in storage. The
+workspace needs object storage set up. With Enterprise advanced storage permissions, the
+user needs read and write on `windmill_uploads/*`, which the default rules grant. The upload goes through
 `job_helpers`, so a restricted token needs `job_helpers:write`; a sandboxed raw app cannot
 request that scope today, so attachments are not available there yet.
 

chat-sdk/src/api.ts

@@ -232,11 +232,6 @@ export class WindmillChatApi {
     return (await res.json()) as { file_key: string }
   }
 
-  /** Removes an object from the workspace's object storage. */
-  async deleteFile(fileKey: string): Promise<void> {
-    await this.#request('job_helpers/delete_s3_file', { method: 'DELETE', query: { file_key: fileKey } })
-  }
-
   async deleteConversation(conversationId: string): Promise<void> {
     await this.#request(`flow_conversations/delete/${encodeURIComponent(conversationId)}`, {
       method: 'DELETE'

chat-sdk/src/attachments.ts

@@ -65,11 +65,6 @@ function dataUrlToBlob(dataUrl: string, fallbackType: string): Blob {
   return new Blob([bytes], { type: mediaType })
 }
 
-/** Delete uploads no run will read. Best effort: a delete that fails leaves that object behind. */
-export async function discardUploads(api: WindmillChatApi, uploaded: UploadedAttachment[]): Promise<void> {
-  await Promise.all(uploaded.map((u) => api.deleteFile(u.s3).catch(() => {})))
-}
-
 /**
  * Put each attachment in the workspace's object storage and hand back what the agent reads.
  * The key's turn prefix and per-file index keep two files with the same name, in this turn or
@@ -82,9 +77,8 @@ export async function uploadAttachments(
   signal?: AbortSignal
 ): Promise<UploadedAttachment[]> {
   const prefix = `${CHAT_UPLOADS_PREFIX}/${turnId}`
-  // One failed upload aborts the rest, and whatever already landed is deleted: no run will
-  // read it, and a resend uploads under a fresh prefix. Best effort, so a delete that fails
-  // leaves that object behind rather than masking the upload error.
+  // One failed upload aborts the rest. Nothing already stored is deleted: the chat never
+  // removes objects from the workspace's storage, so a send that does not run leaves them.
   if (signal?.aborted) throw abortError()
   const batch = new AbortController()
   const abortBatch = () => batch.abort()
@@ -109,11 +103,11 @@ export async function uploadAttachments(
         }
       })
     )
-    const uploaded = results.flatMap((r) => (r.status === 'fulfilled' ? [r.value] : []))
     const reasons = results.flatMap((r) => (r.status === 'rejected' ? [r.reason] : []))
     // A stop that lands once every upload has answered still withdraws the batch.
-    if (reasons.length === 0 && !signal?.aborted) return uploaded
-    await discardUploads(api, uploaded)
+    if (reasons.length === 0 && !signal?.aborted) {
+      return results.flatMap((r) => (r.status === 'fulfilled' ? [r.value] : []))
+    }
     // The failure that started it, not the aborts it caused in the other uploads.
     throw reasons.find((reason) => !isAbortError(reason)) ?? reasons[0] ?? abortError()
   } finally {

chat-sdk/src/chat.ts

@@ -8,7 +8,7 @@ import {
 import { resolveConfig, type ResolvedConfig } from './config'
 import { followJob } from './follow'
 import { createLocalHistory, type LocalHistory } from './history'
-import { discardUploads, uploadAttachments } from './attachments'
+import { uploadAttachments } from './attachments'
 import type { AgentStreamEvent } from './stream'
 import type {
   Chat,
@@ -171,13 +171,9 @@ class ChatImpl implements Chat {
             messages: this.#state.messages.map((m) => (m.id === turn.userMessageId ? { ...m, attachments: carried } : m))
           })
         }
-        // A subscriber told of those attachments may stop the turn right here.
-        if (turn.controller.signal.aborted) {
-          await discardUploads(this.#api, uploaded)
-          throw abortError()
-        }
       }
-      // Nothing may start once stop() or a conversation switch has withdrawn the turn.
+      // Nothing may start once stop() or a conversation switch has withdrawn the turn, including
+      // a stop from a subscriber told of the attachments just above.
       if (turn.controller.signal.aborted) throw abortError()
       turn.started = true
       // Listed only once the run is asked for: a send that never runs (an upload that failed

chat-sdk/test/attachments.test.ts

@@ -259,7 +259,7 @@ describe('sendMessage with attachments', () => {
     expect((await reloaded.loadConversations()).map((c) => c.id)).not.toContain(opened)
   })
 
-  test('a failed upload deletes the files of the same batch that did land', async () => {
+  test('a failed upload aborts the rest of its batch and deletes nothing', async () => {
     let first: (r: Response) => void = () => {}
     const { fetch, calls } = fetchMock(
       (c) => {
@@ -270,10 +270,6 @@ describe('sendMessage with attachments', () => {
         setTimeout(() => first(json({ file_key: keys()[0] })), 5)
         return text('quota exceeded', 507)
       },
-      (c) =>
-        c.method === 'DELETE' && c.url.pathname === '/api/w/ws/job_helpers/delete_s3_file'
-          ? json('deleted')
-          : undefined,
       run,
       answer
     )
@@ -288,10 +284,7 @@ describe('sendMessage with attachments', () => {
         attachmentsInput: { name: 'files', multiple: true }
       })
     ).rejects.toThrow('quota exceeded')
-    const deletes = calls
-      .filter((c) => c.method === 'DELETE')
-      .map((c) => c.url.searchParams.get('file_key'))
-    expect(deletes).toEqual([keys()[0]])
+    expect(calls.filter((c) => c.method === 'DELETE')).toEqual([])
     expect(runs(calls)).toHaveLength(0)
   })
 
@@ -414,16 +407,8 @@ describe('sendMessage with attachments', () => {
     expect(call.url.searchParams.get('content_type')).toBe('application/pdf')
   })
 
-  test('stop() after the uploads land but before the run starts deletes them and runs nothing', async () => {
-    const { fetch, calls } = fetchMock(
-      upload,
-      (c) =>
-        c.method === 'DELETE' && c.url.pathname === '/api/w/ws/job_helpers/delete_s3_file'
-          ? json('deleted')
-          : undefined,
-      run,
-      answer
-    )
+  test('stop() after the uploads land but before the run starts runs nothing', async () => {
+    const { fetch, calls } = fetchMock(upload, run, answer)
     const chat = createChat(options(fetch))
     const sending = chat.sendMessage('read this', {
       attachments: [{ name: 'contract.pdf', data: pdf }],
@@ -434,23 +419,12 @@ describe('sendMessage with attachments', () => {
     await chat.stop()
     await expect(sending).rejects.toMatchObject({ name: 'AbortError' })
     expect(runs(calls)).toHaveLength(0)
-    const key = uploads(calls)[0].url.searchParams.get('file_key')
-    expect(
-      calls.filter((c) => c.method === 'DELETE').map((c) => c.url.searchParams.get('file_key'))
-    ).toEqual([key])
+    expect(calls.filter((c) => c.method === 'DELETE')).toEqual([])
     expect(chat.getState()).toMatchObject({ status: 'idle', messages: [], conversations: [] })
   })
 
-  test('a subscriber stopping when the attachments appear still discards the uploads', async () => {
-    const { fetch, calls } = fetchMock(
-      upload,
-      (c) =>
-        c.method === 'DELETE' && c.url.pathname === '/api/w/ws/job_helpers/delete_s3_file'
-          ? json('deleted')
-          : undefined,
-      run,
-      answer
-    )
+  test('a subscriber stopping when the attachments appear runs nothing', async () => {
+    const { fetch, calls } = fetchMock(upload, run, answer)
     const chat = createChat(options(fetch))
     chat.subscribe((s) => {
       if (s.messages.some((m) => m.attachments)) void chat.stop()
@@ -462,10 +436,7 @@ describe('sendMessage with attachments', () => {
       })
     ).rejects.toMatchObject({ name: 'AbortError' })
     expect(runs(calls)).toHaveLength(0)
-    const key = uploads(calls)[0].url.searchParams.get('file_key')
-    expect(
-      calls.filter((c) => c.method === 'DELETE').map((c) => c.url.searchParams.get('file_key'))
-    ).toEqual([key])
+    expect(calls.filter((c) => c.method === 'DELETE')).toEqual([])
     expect(chat.getState()).toMatchObject({ status: 'idle', messages: [], conversations: [] })
   })
 })