Harden VFS mutation handling

Author: Sg312

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

@@ -42,8 +42,14 @@ const mocks = vi.hoisted(() => ({
   deleteFileVfsItems: vi.fn(),
   renameTableVfs: vi.fn(),
   deleteTableVfs: vi.fn(),
+  transferTableVfs: vi.fn(),
+  createTableFolders: vi.fn(),
+  deleteTableFolders: vi.fn(),
   renameKnowledgeVfs: vi.fn(),
   deleteKnowledgeVfs: vi.fn(),
+  transferKnowledgeVfs: vi.fn(),
+  createKnowledgeFolders: vi.fn(),
+  deleteKnowledgeFolders: vi.fn(),
 }))
 
 vi.mock('@sim/db', () => ({ ...dbChainMock, ...schemaMock }))
@@ -163,6 +169,18 @@ vi.mock('@/lib/table/application/table-vfs', () => ({
     operation: tableOperations.deleteByVfsPath,
     execute: mocks.deleteTableVfs,
   },
+  transferTableVfsItems: {
+    operation: tableOperations.moveByVfsPath,
+    execute: mocks.transferTableVfs,
+  },
+  createTableVfsFolders: {
+    operation: tableOperations.createFolder,
+    execute: mocks.createTableFolders,
+  },
+  deleteTableVfsFolders: {
+    operation: tableOperations.deleteFolder,
+    execute: mocks.deleteTableFolders,
+  },
 }))
 
 vi.mock('@/lib/knowledge/application/knowledge-vfs', () => ({
@@ -174,6 +192,18 @@ vi.mock('@/lib/knowledge/application/knowledge-vfs', () => ({
     operation: knowledgeOperations.deleteByVfsPath,
     execute: mocks.deleteKnowledgeVfs,
   },
+  transferKnowledgeVfsItems: {
+    operation: knowledgeOperations.moveByVfsPath,
+    execute: mocks.transferKnowledgeVfs,
+  },
+  createKnowledgeVfsFolders: {
+    operation: knowledgeOperations.manageVfsFolders,
+    execute: mocks.createKnowledgeFolders,
+  },
+  deleteKnowledgeVfsFolders: {
+    operation: knowledgeOperations.manageVfsFolders,
+    execute: mocks.deleteKnowledgeFolders,
+  },
 }))
 
 vi.mock('@/lib/table/service', () => ({
@@ -776,15 +806,41 @@ describe('vfs mv/cp', () => {
       })
     })
 
-    it('rejects flat namespaces', async () => {
+    it('creates table folders through the table application operation', async () => {
+      mocks.createTableFolders.mockResolvedValue({
+        outcomes: [
+          { source: 'tables/CRM', kind: 'folder', resourceId: 'fld-1', targetSegments: ['CRM'] },
+        ],
+      })
+
       const result = await executeVfsMkdir({ paths: ['tables/CRM'] }, context)
-      expect(result.success).toBe(false)
+
+      expect(mocks.createTableFolders).toHaveBeenCalledWith(
+        expect.objectContaining({
+          input: {
+            workspaceId: 'ws-1',
+            paths: [{ source: 'tables/CRM', segments: ['CRM'] }],
+          },
+        })
+      )
+      expect(result.success).toBe(true)
       expect(result.output).toMatchObject({
-        results: [{ from: 'tables/CRM', error: expect.stringContaining('flat namespace') }],
+        results: [{ from: 'tables/CRM', to: 'tables/CRM', kind: 'table_folder', id: 'fld-1' }],
       })
       expect(mocks.ensureCopilotFileFolderPath).not.toHaveBeenCalled()
     })
 
+    it('rejects the reserved knowledgebases/connectors folder path', async () => {
+      const result = await executeVfsMkdir({ paths: ['knowledgebases/connectors/sub'] }, context)
+      expect(result.success).toBe(false)
+      expect(result.output).toMatchObject({
+        results: [
+          { from: 'knowledgebases/connectors/sub', error: expect.stringContaining('reserved') },
+        ],
+      })
+      expect(mocks.createKnowledgeFolders).not.toHaveBeenCalled()
+    })
+
     it('rejects creation inside a locked workflow folder', async () => {
       mocks.createWorkflowVfsFolders.mockResolvedValue({
         outcomes: [
@@ -804,30 +860,65 @@ describe('vfs mv/cp', () => {
     })
   })
 
-  describe('tables and knowledge bases (flat namespaces)', () => {
-    it('renames a table', async () => {
+  describe('tables and knowledge bases (foldered)', () => {
+    it('renames a table through the transfer application operation', async () => {
+      mocks.transferTableVfs.mockResolvedValue({
+        outcomes: [
+          {
+            source: 'tables/Leads',
+            kind: 'resource',
+            resourceId: 'tbl-1',
+            targetSegments: ['Customers'],
+          },
+        ],
+      })
+
       const result = await executeVfsMv(
         { sources: ['tables/Leads'], destination: 'tables/Customers' },
         context
       )
 
-      expect(mocks.renameTableVfs).toHaveBeenCalledWith(
+      expect(mocks.transferTableVfs).toHaveBeenCalledWith(
         expect.objectContaining({
-          input: { workspaceId: 'ws-1', sourceName: 'Leads', newName: 'Customers' },
+          input: {
+            workspaceId: 'ws-1',
+            sources: [{ source: 'tables/Leads', segments: ['Leads'] }],
+            destination: { segments: ['Customers'], trailingSlash: false },
+          },
         })
       )
       expect(result.success).toBe(true)
       expect(result.output).toMatchObject({ results: [{ to: 'tables/Customers', kind: 'table' }] })
     })
 
-    it('rejects nested table destinations as flat-namespace violations', async () => {
+    it('moves a table into a folder, folders auto-created server-side', async () => {
+      mocks.transferTableVfs.mockResolvedValue({
+        outcomes: [
+          {
+            source: 'tables/Leads',
+            kind: 'resource',
+            resourceId: 'tbl-1',
+            targetSegments: ['CRM', 'Leads'],
+          },
+        ],
+      })
+
       const result = await executeVfsMv(
-        { sources: ['tables/Leads'], destination: 'tables/CRM/Leads' },
+        { sources: ['tables/Leads'], destination: 'tables/CRM/' },
         context
       )
-      expect(result.success).toBe(false)
-      expect(result.error).toContain('flat namespace')
-      expect(mocks.renameTable).not.toHaveBeenCalled()
+
+      expect(mocks.transferTableVfs).toHaveBeenCalledWith(
+        expect.objectContaining({
+          input: expect.objectContaining({
+            destination: { segments: ['CRM'], trailingSlash: true },
+          }),
+        })
+      )
+      expect(result.success).toBe(true)
+      expect(result.output).toMatchObject({
+        results: [{ to: 'tables/CRM/Leads', kind: 'table' }],
+      })
     })
 
     it('rejects copying tables', async () => {
@@ -840,12 +931,23 @@ describe('vfs mv/cp', () => {
     })
 
     it('renames a knowledge base through trusted application operations', async () => {
+      mocks.transferKnowledgeVfs.mockResolvedValue({
+        outcomes: [
+          {
+            source: 'knowledgebases/Docs',
+            kind: 'resource',
+            resourceId: 'kb-1',
+            targetSegments: ['Product Docs'],
+          },
+        ],
+      })
+
       const result = await executeVfsMv(
         { sources: ['knowledgebases/Docs'], destination: 'knowledgebases/Product Docs' },
         context
       )
 
-      expect(mocks.renameKnowledgeVfs).toHaveBeenCalledWith(
+      expect(mocks.transferKnowledgeVfs).toHaveBeenCalledWith(
         expect.objectContaining({
           principal: expect.objectContaining({
             kind: 'delegated',
@@ -855,16 +957,16 @@ describe('vfs mv/cp', () => {
           }),
           input: {
             workspaceId: 'ws-1',
-            sourceName: 'Docs',
-            newName: 'Product Docs',
+            sources: [{ source: 'knowledgebases/Docs', segments: ['Docs'] }],
+            destination: { segments: ['Product Docs'], trailingSlash: false },
           },
         })
       )
       expect(result.success).toBe(true)
     })
 
     it('propagates knowledge application infrastructure failures', async () => {
-      mocks.renameKnowledgeVfs.mockRejectedValueOnce(new Error('knowledge database unavailable'))
+      mocks.transferKnowledgeVfs.mockRejectedValueOnce(new Error('knowledge database unavailable'))
 
       await expect(
         executeVfsMv(
@@ -875,7 +977,7 @@ describe('vfs mv/cp', () => {
     })
 
     it('preserves an actionable knowledge rename conflict', async () => {
-      mocks.renameKnowledgeVfs.mockRejectedValue(
+      mocks.transferKnowledgeVfs.mockRejectedValue(
         new OrchestrationError('conflict', 'A knowledge base named Product Docs already exists')
       )
 
@@ -912,11 +1014,71 @@ describe('vfs mv/cp', () => {
           input: {
             workspaceId: 'ws-1',
             sourceName: 'Docs',
+            sourceSegments: ['Docs'],
           },
         })
       )
     })
 
+    it('moves a whole table folder through the transfer operation', async () => {
+      mocks.transferTableVfs.mockResolvedValue({
+        outcomes: [
+          {
+            source: 'tables/CRM',
+            kind: 'folder',
+            resourceId: 'fld-1',
+            targetSegments: ['Archive', 'CRM'],
+          },
+        ],
+      })
+
+      const result = await executeVfsMv(
+        { sources: ['tables/CRM'], destination: 'tables/Archive/' },
+        context
+      )
+
+      expect(result.success).toBe(true)
+      expect(result.output).toMatchObject({
+        results: [{ to: 'tables/Archive/CRM', kind: 'table_folder' }],
+      })
+    })
+
+    it('rm retargets to the folder cascade when the path is a folder', async () => {
+      mocks.deleteTableVfs.mockRejectedValue(
+        new OrchestrationError('invalid', 'tables/CRM is a folder; this operation takes a table.')
+      )
+      mocks.deleteTableFolders.mockResolvedValue({
+        outcomes: [{ source: 'tables/CRM', kind: 'folder', resourceId: 'fld-1' }],
+      })
+
+      const result = await executeVfsRm({ paths: ['tables/CRM'] }, context)
+
+      expect(mocks.deleteTableFolders).toHaveBeenCalledWith(
+        expect.objectContaining({
+          input: { workspaceId: 'ws-1', paths: [{ source: 'tables/CRM', segments: ['CRM'] }] },
+        })
+      )
+      expect(result.success).toBe(true)
+      expect(result.output).toMatchObject({
+        results: [{ from: 'tables/CRM', kind: 'table_folder', id: 'fld-1' }],
+      })
+    })
+
+    it('deletes a nested knowledge base by its folder path', async () => {
+      const result = await executeVfsRm({ paths: ['knowledgebases/Legal/Contracts'] }, context)
+
+      expect(mocks.deleteKnowledgeVfs).toHaveBeenCalledWith(
+        expect.objectContaining({
+          input: {
+            workspaceId: 'ws-1',
+            sourceName: 'Contracts',
+            sourceSegments: ['Legal', 'Contracts'],
+          },
+        })
+      )
+      expect(result.success).toBe(true)
+    })
+
     it('preserves an actionable knowledge delete failure', async () => {
       mocks.deleteKnowledgeVfs.mockRejectedValue(
         new OrchestrationError('not_found', 'Knowledge base no longer exists')