fix(mothership): retain completed files after export failure

Author: Sg312

apps/sim/lib/mothership/agent-cli/saved-run-read.postgres.test.ts

@@ -132,8 +132,10 @@ import {
   resolveInputFiles,
 } from '@/lib/mothership/tools/handlers/function-execute'
 import { chatSandboxSessionKey } from '@/lib/mothership/tools/sandbox-session-key'
+import { writeCopilotWorkspaceFileByPath } from '@/lib/mothership/vfs/resource-writer'
 import { replaceWorkflowNormalizedState } from '@/lib/workflows/persistence/replace-normalized-state'
 import { fileOperations } from '@/lib/workspace-files/application/operations'
+import { readWorkspaceFileArtifact } from '@/lib/workspace-files/application/read-workspace-file-artifact'
 import { readWorkspaceFileText } from '@/lib/workspace-files/application/read-workspace-file-text'
 import {
   POST as addChatResourceRoute,
@@ -2222,7 +2224,13 @@ describe.skipIf(!process.env.MSHIP_TEST_DATABASE_URL)(
 
     it
       .skipIf(!process.env.MSHIP_LOCAL_COMPUTE_IMAGE || !process.env.MSHIP_WORKER_ROOT)
-      .each(['returned-value', 'sandbox-file', 'mixed-files', 'missing-table'] as const)(
+      .each([
+        'returned-value',
+        'sandbox-file',
+        'mixed-files',
+        'missing-table',
+        'partial-files',
+      ] as const)(
       'composes attachment computation, table replacement, durable export and fresh-chat re-import: %s',
       async (mode) => {
         fixture.permission = 'write'
@@ -2360,6 +2368,15 @@ describe.skipIf(!process.env.MSHIP_TEST_DATABASE_URL)(
           expect(prepared.storedAttachments).toHaveLength(1)
           await vi.mocked(executeShellInSandbox).withImplementation(compute, async () => {
             const callContext = context(chatId)
+            if (mode === 'partial-files') {
+              await writeCopilotWorkspaceFileByPath(callContext, {
+                workspaceId,
+                target: { path: `files/existing-${exportName}`, mode: 'create' },
+                buffer: Buffer.from('Existing report must survive'),
+                inferredMimeType: 'text/csv',
+                secretProvenance: { status: 'exact', entries: [] },
+              })
+            }
             const params = {
               language: 'shell',
               timeout: 30,
@@ -2376,16 +2393,17 @@ describe.skipIf(!process.env.MSHIP_TEST_DATABASE_URL)(
                   ...(mode === 'mixed-files'
                     ? [{ path: `files/raw-${exportName}`, sandboxPath: '/home/user/totals.csv' }]
                     : []),
+                  ...(mode === 'partial-files' ? [{ path: `files/existing-${exportName}` }] : []),
                 ],
               },
               outputTable: mode === 'missing-table' ? generateId() : tableId,
             }
             let result = await executeFunctionExecute(params, callContext)
             expect(result.success, JSON.stringify(result)).toBe(true)
             result = await maybeWriteOutputToFile('run_function', params, result, callContext)
-            expect(result.success, JSON.stringify(result)).toBe(true)
+            expect(result.success, JSON.stringify(result)).toBe(mode !== 'partial-files')
             result = await maybeWriteOutputToTable('run_function', params, result, callContext)
-            if (mode === 'missing-table') {
+            if (mode === 'missing-table' || mode === 'partial-files') {
               expect(result.success).toBe(false)
               expect(result.error).toContain('already written')
               expect(result.output).toMatchObject({
@@ -2414,7 +2432,7 @@ describe.skipIf(!process.env.MSHIP_TEST_DATABASE_URL)(
               .from(userTableRows)
               .where(eq(userTableRows.tableId, tableId))
             const expectedRows =
-              mode === 'missing-table'
+              mode === 'missing-table' || mode === 'partial-files'
                 ? []
                 : [
                     { col_account: 'alpha', col_total: 22 },
@@ -2443,6 +2461,13 @@ describe.skipIf(!process.env.MSHIP_TEST_DATABASE_URL)(
               sandboxSession: 'created',
             })
             expect(sessions.size).toBe(2)
+            if (mode === 'partial-files') {
+              const existing = await readWorkspaceFileArtifact.execute({
+                principal: { kind: 'personal_api_key', userId: 'run-reader', keyId: 'local-key' },
+                input: { workspaceId, reference: `files/existing-${exportName}`, maxBytes: 1024 },
+              })
+              expect(existing.buffer.toString('utf8')).toBe('Existing report must survive')
+            }
           })
         } finally {
           setEnvFlags({ isMothershipSandboxEnabled: previousWorkbenchEnabled })

apps/sim/lib/mothership/request/tools/files.test.ts

@@ -243,6 +243,54 @@ describe('maybeWriteOutputToFile', () => {
     expect(result.resources).toContainEqual(existingResource)
   })
 
+  it.each(['before-write', 'after-first-write', 'after-sandbox-export'] as const)(
+    'reports only committed files when export fails: %s',
+    async (failurePoint) => {
+      const receipt = { fileId: 'file-1', fileName: 'report.csv', vfsPath: 'files/report.csv' }
+      if (failurePoint === 'after-first-write') {
+        mockWriteWorkspaceFileByPath.mockResolvedValueOnce({
+          id: 'file-1',
+          name: 'report.csv',
+          vfsPath: 'files/report.csv',
+          mode: 'create',
+        })
+      }
+      mockWriteWorkspaceFileByPath.mockRejectedValueOnce(new Error('Destination already exists'))
+      const result = await maybeWriteOutputToFile(
+        RunFunction.id,
+        {
+          outputs: {
+            files: [
+              { path: 'files/report.csv' },
+              ...(failurePoint === 'after-first-write' ? [{ path: 'files/second.csv' }] : []),
+            ],
+          },
+        },
+        {
+          success: true,
+          output: {
+            result: [{ name: 'Ada' }],
+            stdout: '1 row',
+            ...(failurePoint === 'after-sandbox-export' ? { exported: { files: [receipt] } } : {}),
+          },
+        },
+        buildContext()
+      )
+      expect(result.success).toBe(false)
+      expect(result.error).toContain('Destination already exists')
+      if (failurePoint === 'before-write') {
+        expect(result.output).toBeUndefined()
+        expect(result.error).not.toContain('already written')
+      } else {
+        expect(result.error).toContain('already written')
+        expect(result.output).toMatchObject({ files: [expect.objectContaining(receipt)] })
+      }
+      expect(mockWriteWorkspaceFileByPath).toHaveBeenCalledTimes(
+        failurePoint === 'after-first-write' ? 2 : 1
+      )
+    }
+  )
+
   it('classifies large structured output from its serialized bytes instead of its object count', async () => {
     const registry = new ResolvedSecretTraceRegistry(
       [

apps/sim/lib/mothership/request/tools/files.ts

@@ -35,6 +35,19 @@ export function getOutputFileReceipts(output: unknown): Record<string, unknown>[
   return Array.isArray(files) ? files.filter(isRecordLike) : []
 }
 
+/** Completed writes remain usable even when a later output operation fails. */
+export function outputWriteFailure(
+  error: string,
+  files: Record<string, unknown>[]
+): ToolCallResult {
+  if (files.length === 0) return { success: false, error }
+  return {
+    success: false,
+    error: `${error}. The listed output files were already written.`,
+    output: { files },
+  }
+}
+
 export const OUTPUT_PATH_TOOLS: Set<string> = new Set([RunFunction.id, UserTable.id])
 
 export type OutputFormat = 'json' | 'csv' | 'txt' | 'md' | 'html'
@@ -379,6 +392,7 @@ export async function maybeWriteOutputToFile(
       [TraceAttr.WorkspaceId]: workspaceId,
     },
     async (span) => {
+      const committedFiles = [...previousFiles]
       try {
         const preparedByFormat = new Map<
           OutputFormat,
@@ -439,6 +453,13 @@ export async function maybeWriteOutputToFile(
             inferredMimeType: contentType,
             secretProvenance,
           })
+          committedFiles.push({
+            fileId: written.id,
+            fileName: written.name,
+            vfsPath: written.vfsPath,
+            size: buffer.length,
+            downloadUrl: written.downloadUrl,
+          })
           writtenFiles.push({
             ...written,
             bytes: buffer.length,
@@ -468,16 +489,6 @@ export async function maybeWriteOutputToFile(
           })),
         })
 
-        const files = [
-          ...previousFiles,
-          ...writtenFiles.map((file) => ({
-            fileId: file.id,
-            fileName: file.name,
-            vfsPath: file.vfsPath,
-            size: file.bytes,
-            downloadUrl: file.downloadUrl,
-          })),
-        ]
         return {
           success: true,
           output: {
@@ -489,7 +500,7 @@ export async function maybeWriteOutputToFile(
               writtenFiles.length === 1
                 ? `Output ${firstWritten.mode === 'overwrite' ? 'updated' : 'written'} at ${firstWritten.vfsPath} (${firstWritten.bytes} bytes)`
                 : `Output written to ${writtenFiles.length} files`,
-            files,
+            files: committedFiles,
             fileId: firstWritten.id,
             fileName: firstWritten.name,
             vfsPath: firstWritten.vfsPath,
@@ -521,10 +532,10 @@ export async function maybeWriteOutputToFile(
         span.addEvent(TraceEvent.CopilotOutputFileError, {
           [TraceAttr.ErrorMessage]: projectedMessage.slice(0, 500),
         })
-        return {
-          success: false,
-          error: `Failed to write output file: ${message}`,
-        }
+        return outputWriteFailure(
+          `Failed to write output file: ${projectedMessage}`,
+          committedFiles
+        )
       }
     }
   )

apps/sim/lib/mothership/request/tools/tables.test.ts

@@ -271,7 +271,7 @@ describe('automatic Copilot tool-output table persistence', () => {
     expect(result).toEqual({
       success: false,
       error:
-        'Failed to write to table: Table operation failed. The declared output files were already written.',
+        'Failed to write to table: Table operation failed. The listed output files were already written.',
       output: { files },
     })
   })

apps/sim/lib/mothership/request/tools/tables.ts

@@ -9,7 +9,7 @@ import { TraceAttr } from '@/lib/mothership/generated/trace-attributes-v1'
 import { TraceEvent } from '@/lib/mothership/generated/trace-events-v1'
 import { TraceSpan } from '@/lib/mothership/generated/trace-spans-v1'
 import { withCopilotSpan } from '@/lib/mothership/request/otel'
-import { getOutputFileReceipts } from '@/lib/mothership/request/tools/files'
+import { getOutputFileReceipts, outputWriteFailure } from '@/lib/mothership/request/tools/files'
 import { denyOutputWriteWithoutWritePermission } from '@/lib/mothership/request/tools/permissions'
 import { projectToolErrorMessageForCopilot } from '@/lib/mothership/request/tools/resolved-secret-result'
 import type { ExecutionContext, ToolCallResult } from '@/lib/mothership/request/types'
@@ -37,13 +37,7 @@ function printedStdout(rawOutput: unknown): string | undefined {
  * files so the caller sees what landed instead of re-running the code for it.
  */
 function outputTableFailure(error: string, rawOutput: unknown): ToolCallResult {
-  const files = getOutputFileReceipts(rawOutput)
-  if (files.length === 0) return { success: false, error }
-  return {
-    success: false,
-    error: `${error}. The declared output files were already written.`,
-    output: { files },
-  }
+  return outputWriteFailure(error, getOutputFileReceipts(rawOutput))
 }
 /**
  * Replaces a table's rows with wire rows keyed by column name. Translates the