fix(knowledge): preserve upload retry and VFS errors

Author: TheodoreSpeaks

apps/sim/lib/copilot/tools/handlers/vfs-mutate.test.ts

@@ -172,6 +172,7 @@ vi.mock('@/lib/core/telemetry', () => ({
 }))
 
 import type { ExecutionContext } from '@/lib/copilot/request/types'
+import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { executeVfsCp, executeVfsMkdir, executeVfsMv, executeVfsRm } from './vfs-mutate'
 
 const context = {
@@ -649,6 +650,25 @@ describe('vfs mv/cp', () => {
       ).rejects.toThrow('knowledge database unavailable')
     })
 
+    it('preserves an actionable knowledge rename conflict', async () => {
+      mocks.listKnowledgeBases.mockResolvedValue({
+        knowledgeBases: [{ knowledgeBase: { id: 'kb-1', name: 'Docs' }, folderPath: '/' }],
+      })
+      mocks.updateKnowledgeBase.mockRejectedValue(
+        new OrchestrationError('conflict', 'A knowledge base named Product Docs already exists')
+      )
+
+      const result = await executeVfsMv(
+        { sources: ['knowledgebases/Docs'], destination: 'knowledgebases/Product Docs' },
+        context
+      )
+
+      expect(result).toMatchObject({
+        success: false,
+        error: 'A knowledge base named Product Docs already exists',
+      })
+    })
+
     it('rejects the reserved knowledgebases/connectors name', async () => {
       const result = await executeVfsMv(
         { sources: ['knowledgebases/Docs'], destination: 'knowledgebases/connectors' },
@@ -682,5 +702,29 @@ describe('vfs mv/cp', () => {
       )
       expect(mocks.knowledgeBaseDeleted).toHaveBeenCalledWith({ knowledgeBaseId: 'kb-1' })
     })
+
+    it('preserves an actionable knowledge delete failure', async () => {
+      mocks.listKnowledgeBases.mockResolvedValue({
+        knowledgeBases: [{ knowledgeBase: { id: 'kb-1', name: 'Docs' }, folderPath: '/' }],
+      })
+      mocks.deleteKnowledgeBase.mockRejectedValue(
+        new OrchestrationError('not_found', 'Knowledge base no longer exists')
+      )
+
+      const result = await executeVfsRm({ paths: ['knowledgebases/Docs'] }, context)
+
+      expect(result).toMatchObject({
+        success: false,
+        error: 'Knowledge base no longer exists',
+        output: {
+          results: [
+            expect.objectContaining({
+              from: 'knowledgebases/Docs',
+              error: 'Knowledge base no longer exists',
+            }),
+          ],
+        },
+      })
+    })
   })
 })

apps/sim/lib/copilot/tools/handlers/vfs-mutate.ts

@@ -9,6 +9,7 @@ import {
 } from '@/lib/copilot/application/execute-file-use-case'
 import {
   executeCopilotKnowledgeUseCase,
+  messageForCopilotKnowledgeError,
   resolveCopilotKnowledgePrincipal,
 } from '@/lib/copilot/application/execute-knowledge-use-case'
 import { messageForCopilotFileError } from '@/lib/copilot/auth/file-delegation'
@@ -89,6 +90,14 @@ class KnowledgeVfsInfrastructureError extends Error {
   }
 }
 
+function messageForKnowledgeVfsError(error: unknown, forbiddenMessage: string): string {
+  const classified = asOrchestrationError(error)
+  if (!classified || classified.code === 'internal') {
+    throw new KnowledgeVfsInfrastructureError(error)
+  }
+  return classified.code === 'forbidden' ? forbiddenMessage : messageForCopilotKnowledgeError(error)
+}
+
 /** Top-level VFS segment of a raw (possibly encoded) path. */
 function topLevelSegment(path: string): string {
   return path.trim().replace(/^\/+/, '').split('/')[0] ?? ''
@@ -861,11 +870,10 @@ async function renameFlatResource(
     })
     knowledgeBases = result.knowledgeBases
   } catch (error) {
-    const classified = asOrchestrationError(error)
-    if (classified && classified.code !== 'internal') {
-      return { success: false, error: 'Write access required to rename knowledge bases' }
+    return {
+      success: false,
+      error: messageForKnowledgeVfsError(error, 'Write access required to rename knowledge bases'),
     }
-    throw new KnowledgeVfsInfrastructureError(error)
   }
   const match = knowledgeBases
     .map(({ knowledgeBase }) => knowledgeBase)
@@ -882,14 +890,13 @@ async function renameFlatResource(
       source: 'agent',
     })
   } catch (error) {
-    const classified = asOrchestrationError(error)
-    if (classified && classified.code !== 'internal') {
-      return {
-        success: false,
-        error: `Write access required to rename knowledge base "${match.name}"`,
-      }
+    return {
+      success: false,
+      error: messageForKnowledgeVfsError(
+        error,
+        `Write access required to rename knowledge base "${match.name}"`
+      ),
     }
-    throw new KnowledgeVfsInfrastructureError(error)
   }
   logger.info('Renamed knowledge base via mv', { knowledgeBaseId: match.id, workspaceId })
   return buildResult(verb, [
@@ -1193,15 +1200,11 @@ async function removeKnowledgeBasePath(
     })
     knowledgeBases = result.knowledgeBases
   } catch (error) {
-    const classified = asOrchestrationError(error)
-    if (classified && classified.code !== 'internal') {
-      return {
-        from: path,
-        kind: 'knowledge_base',
-        error: 'Write access required to delete knowledge bases',
-      }
+    return {
+      from: path,
+      kind: 'knowledge_base',
+      error: messageForKnowledgeVfsError(error, 'Write access required to delete knowledge bases'),
     }
-    throw new KnowledgeVfsInfrastructureError(error)
   }
   const match = knowledgeBases
     .map(({ knowledgeBase }) => knowledgeBase)
@@ -1216,16 +1219,15 @@ async function removeKnowledgeBasePath(
       source: 'agent',
     })
   } catch (error) {
-    const classified = asOrchestrationError(error)
-    if (classified && classified.code !== 'internal') {
-      return {
-        from: path,
-        kind: 'knowledge_base',
-        id: match.id,
-        error: `Write access required to delete knowledge base "${match.name}"`,
-      }
+    return {
+      from: path,
+      kind: 'knowledge_base',
+      id: match.id,
+      error: messageForKnowledgeVfsError(
+        error,
+        `Write access required to delete knowledge base "${match.name}"`
+      ),
     }
-    throw new KnowledgeVfsInfrastructureError(error)
   }
   PlatformEvents.knowledgeBaseDeleted({ knowledgeBaseId: match.id })
   logger.info('Deleted knowledge base via rm', { knowledgeBaseId: match.id, workspaceId })

apps/sim/lib/knowledge/application/upload-sessions.test.ts

@@ -370,7 +370,7 @@ describe('knowledge-document upload application lifecycle', () => {
         }>
       }) => ({
         session: { ...params.session, status: 'completed' as const },
-        value: (await params.finalize(params.session)).value,
+        value: (await params.finalize({ ...params.session, error: null })).value,
         alreadyCompleted: true,
       })
     )
@@ -445,7 +445,7 @@ describe('knowledge-document upload application lifecycle', () => {
         }>
       }) => ({
         session: { ...params.session, status: 'completed' as const },
-        value: (await params.finalize(params.session)).value,
+        value: (await params.finalize({ ...params.session, error: null })).value,
         alreadyCompleted: true,
       })
     )

apps/sim/lib/knowledge/application/upload-sessions.ts

@@ -260,7 +260,7 @@ export const completeKnowledgeDocumentUpload = defineAuthorizedKnowledgeUseCase(
         }
         if (bound.status === 'bound') {
           if (
-            claimed.error === PROCESSING_DISPATCH_FAILURE_MESSAGE &&
+            session.error === PROCESSING_DISPATCH_FAILURE_MESSAGE &&
             bound.document.processingStatus === 'pending'
           ) {
             const billingAttribution = await resolveKnowledgeBillingAttribution(