improvement(platform): standardize perms, audit logging, lifecycle across admin, copilot, ui actions (#3858)

* improvement(platform): standardize perms, audit logging, lifecycle mgmt across admin, copilot, ui actions

* address comments

* improve error codes

* address bugbot comments

* fix test

Author: icecrasher321

apps/sim/app/api/chat/manage/[id]/route.test.ts

@@ -15,12 +15,12 @@ const {
   mockLimit,
   mockUpdate,
   mockSet,
-  mockDelete,
   mockCreateSuccessResponse,
   mockCreateErrorResponse,
   mockEncryptSecret,
   mockCheckChatAccess,
   mockDeployWorkflow,
+  mockPerformChatUndeploy,
   mockLogger,
 } = vi.hoisted(() => {
   const logger = {
@@ -40,12 +40,12 @@ const {
     mockLimit: vi.fn(),
     mockUpdate: vi.fn(),
     mockSet: vi.fn(),
-    mockDelete: vi.fn(),
     mockCreateSuccessResponse: vi.fn(),
     mockCreateErrorResponse: vi.fn(),
     mockEncryptSecret: vi.fn(),
     mockCheckChatAccess: vi.fn(),
     mockDeployWorkflow: vi.fn(),
+    mockPerformChatUndeploy: vi.fn(),
     mockLogger: logger,
   }
 })
@@ -66,7 +66,6 @@ vi.mock('@sim/db', () => ({
   db: {
     select: mockSelect,
     update: mockUpdate,
-    delete: mockDelete,
   },
 }))
 vi.mock('@sim/db/schema', () => ({
@@ -88,6 +87,9 @@ vi.mock('@/app/api/chat/utils', () => ({
 vi.mock('@/lib/workflows/persistence/utils', () => ({
   deployWorkflow: mockDeployWorkflow,
 }))
+vi.mock('@/lib/workflows/orchestration', () => ({
+  performChatUndeploy: mockPerformChatUndeploy,
+}))
 vi.mock('drizzle-orm', () => ({
   and: vi.fn((...conditions: unknown[]) => ({ type: 'and', conditions })),
   eq: vi.fn((field: unknown, value: unknown) => ({ field, value, type: 'eq' })),
@@ -106,7 +108,7 @@ describe('Chat Edit API Route', () => {
     mockWhere.mockReturnValue({ limit: mockLimit })
     mockUpdate.mockReturnValue({ set: mockSet })
     mockSet.mockReturnValue({ where: mockWhere })
-    mockDelete.mockReturnValue({ where: mockWhere })
+    mockPerformChatUndeploy.mockResolvedValue({ success: true })
 
     mockCreateSuccessResponse.mockImplementation((data) => {
       return new Response(JSON.stringify(data), {
@@ -428,7 +430,11 @@ describe('Chat Edit API Route', () => {
       const response = await DELETE(req, { params: Promise.resolve({ id: 'chat-123' }) })
 
       expect(response.status).toBe(200)
-      expect(mockDelete).toHaveBeenCalled()
+      expect(mockPerformChatUndeploy).toHaveBeenCalledWith({
+        chatId: 'chat-123',
+        userId: 'user-id',
+        workspaceId: 'workspace-123',
+      })
       const data = await response.json()
       expect(data.message).toBe('Chat deployment deleted successfully')
     })
@@ -451,7 +457,11 @@ describe('Chat Edit API Route', () => {
 
       expect(response.status).toBe(200)
       expect(mockCheckChatAccess).toHaveBeenCalledWith('chat-123', 'admin-user-id')
-      expect(mockDelete).toHaveBeenCalled()
+      expect(mockPerformChatUndeploy).toHaveBeenCalledWith({
+        chatId: 'chat-123',
+        userId: 'admin-user-id',
+        workspaceId: 'workspace-123',
+      })
     })
   })
 })

apps/sim/app/api/chat/manage/[id]/route.ts

@@ -9,6 +9,7 @@ import { getSession } from '@/lib/auth'
 import { isDev } from '@/lib/core/config/feature-flags'
 import { encryptSecret } from '@/lib/core/security/encryption'
 import { getEmailDomain } from '@/lib/core/utils/urls'
+import { performChatUndeploy } from '@/lib/workflows/orchestration'
 import { deployWorkflow } from '@/lib/workflows/persistence/utils'
 import { checkChatAccess } from '@/app/api/chat/utils'
 import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils'
@@ -270,33 +271,25 @@ export async function DELETE(
       return createErrorResponse('Unauthorized', 401)
     }
 
-    const {
-      hasAccess,
-      chat: chatRecord,
-      workspaceId: chatWorkspaceId,
-    } = await checkChatAccess(chatId, session.user.id)
+    const { hasAccess, workspaceId: chatWorkspaceId } = await checkChatAccess(
+      chatId,
+      session.user.id
+    )
 
     if (!hasAccess) {
       return createErrorResponse('Chat not found or access denied', 404)
     }
 
-    await db.delete(chat).where(eq(chat.id, chatId))
-
-    logger.info(`Chat "${chatId}" deleted successfully`)
-
-    recordAudit({
-      workspaceId: chatWorkspaceId || null,
-      actorId: session.user.id,
-      actorName: session.user.name,
-      actorEmail: session.user.email,
-      action: AuditAction.CHAT_DELETED,
-      resourceType: AuditResourceType.CHAT,
-      resourceId: chatId,
-      resourceName: chatRecord?.title || chatId,
-      description: `Deleted chat deployment "${chatRecord?.title || chatId}"`,
-      request: _request,
+    const result = await performChatUndeploy({
+      chatId,
+      userId: session.user.id,
+      workspaceId: chatWorkspaceId,
     })
 
+    if (!result.success) {
+      return createErrorResponse(result.error || 'Failed to delete chat', 500)
+    }
+
     return createSuccessResponse({
       message: 'Chat deployment deleted successfully',
     })

apps/sim/app/api/chat/route.test.ts

@@ -3,7 +3,7 @@
  *
  * @vitest-environment node
  */
-import { auditMock, createEnvMock } from '@sim/testing'
+import { createEnvMock } from '@sim/testing'
 import { NextRequest } from 'next/server'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
@@ -12,66 +12,51 @@ const {
   mockFrom,
   mockWhere,
   mockLimit,
-  mockInsert,
-  mockValues,
-  mockReturning,
   mockCreateSuccessResponse,
   mockCreateErrorResponse,
-  mockEncryptSecret,
   mockCheckWorkflowAccessForChatCreation,
-  mockDeployWorkflow,
+  mockPerformChatDeploy,
   mockGetSession,
-  mockUuidV4,
 } = vi.hoisted(() => ({
   mockSelect: vi.fn(),
   mockFrom: vi.fn(),
   mockWhere: vi.fn(),
   mockLimit: vi.fn(),
-  mockInsert: vi.fn(),
-  mockValues: vi.fn(),
-  mockReturning: vi.fn(),
   mockCreateSuccessResponse: vi.fn(),
   mockCreateErrorResponse: vi.fn(),
-  mockEncryptSecret: vi.fn(),
   mockCheckWorkflowAccessForChatCreation: vi.fn(),
-  mockDeployWorkflow: vi.fn(),
+  mockPerformChatDeploy: vi.fn(),
   mockGetSession: vi.fn(),
-  mockUuidV4: vi.fn(),
 }))
 
-vi.mock('@/lib/audit/log', () => auditMock)
-
 vi.mock('@sim/db', () => ({
   db: {
     select: mockSelect,
-    insert: mockInsert,
   },
 }))
 
 vi.mock('@sim/db/schema', () => ({
-  chat: { userId: 'userId', identifier: 'identifier' },
+  chat: { userId: 'userId', identifier: 'identifier', archivedAt: 'archivedAt' },
   workflow: { id: 'id', userId: 'userId', isDeployed: 'isDeployed' },
 }))
 
+vi.mock('drizzle-orm', () => ({
+  and: vi.fn((...conditions: unknown[]) => ({ type: 'and', conditions })),
+  eq: vi.fn((field: unknown, value: unknown) => ({ field, value, type: 'eq' })),
+  isNull: vi.fn((field: unknown) => ({ type: 'isNull', field })),
+}))
+
 vi.mock('@/app/api/workflows/utils', () => ({
   createSuccessResponse: mockCreateSuccessResponse,
   createErrorResponse: mockCreateErrorResponse,
 }))
 
-vi.mock('@/lib/core/security/encryption', () => ({
-  encryptSecret: mockEncryptSecret,
-}))
-
-vi.mock('uuid', () => ({
-  v4: mockUuidV4,
-}))
-
 vi.mock('@/app/api/chat/utils', () => ({
   checkWorkflowAccessForChatCreation: mockCheckWorkflowAccessForChatCreation,
 }))
 
-vi.mock('@/lib/workflows/persistence/utils', () => ({
-  deployWorkflow: mockDeployWorkflow,
+vi.mock('@/lib/workflows/orchestration', () => ({
+  performChatDeploy: mockPerformChatDeploy,
 }))
 
 vi.mock('@/lib/auth', () => ({
@@ -94,10 +79,6 @@ describe('Chat API Route', () => {
     mockSelect.mockReturnValue({ from: mockFrom })
     mockFrom.mockReturnValue({ where: mockWhere })
     mockWhere.mockReturnValue({ limit: mockLimit })
-    mockInsert.mockReturnValue({ values: mockValues })
-    mockValues.mockReturnValue({ returning: mockReturning })
-
-    mockUuidV4.mockReturnValue('test-uuid')
 
     mockCreateSuccessResponse.mockImplementation((data) => {
       return new Response(JSON.stringify(data), {
@@ -113,12 +94,10 @@ describe('Chat API Route', () => {
       })
     })
 
-    mockEncryptSecret.mockResolvedValue({ encrypted: 'encrypted-password' })
-
-    mockDeployWorkflow.mockResolvedValue({
+    mockPerformChatDeploy.mockResolvedValue({
       success: true,
-      version: 1,
-      deployedAt: new Date(),
+      chatId: 'test-uuid',
+      chatUrl: 'http://localhost:3000/chat/test-chat',
     })
   })
 
@@ -277,7 +256,6 @@ describe('Chat API Route', () => {
         hasAccess: true,
         workflow: { userId: 'user-id', workspaceId: null, isDeployed: true },
       })
-      mockReturning.mockResolvedValue([{ id: 'test-uuid' }])
 
       const req = new NextRequest('http://localhost:3000/api/chat', {
         method: 'POST',
@@ -287,6 +265,13 @@ describe('Chat API Route', () => {
 
       expect(response.status).toBe(200)
       expect(mockCheckWorkflowAccessForChatCreation).toHaveBeenCalledWith('workflow-123', 'user-id')
+      expect(mockPerformChatDeploy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          workflowId: 'workflow-123',
+          userId: 'user-id',
+          identifier: 'test-chat',
+        })
+      )
     })
 
     it('should allow chat deployment when user has workspace admin permission', async () => {
@@ -309,7 +294,6 @@ describe('Chat API Route', () => {
         hasAccess: true,
         workflow: { userId: 'other-user-id', workspaceId: 'workspace-123', isDeployed: true },
       })
-      mockReturning.mockResolvedValue([{ id: 'test-uuid' }])
 
       const req = new NextRequest('http://localhost:3000/api/chat', {
         method: 'POST',
@@ -319,6 +303,12 @@ describe('Chat API Route', () => {
 
       expect(response.status).toBe(200)
       expect(mockCheckWorkflowAccessForChatCreation).toHaveBeenCalledWith('workflow-123', 'user-id')
+      expect(mockPerformChatDeploy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          workflowId: 'workflow-123',
+          workspaceId: 'workspace-123',
+        })
+      )
     })
 
     it('should reject when workflow is in workspace but user lacks admin permission', async () => {
@@ -383,7 +373,7 @@ describe('Chat API Route', () => {
       expect(mockCheckWorkflowAccessForChatCreation).toHaveBeenCalledWith('workflow-123', 'user-id')
     })
 
-    it('should auto-deploy workflow if not already deployed', async () => {
+    it('should call performChatDeploy for undeployed workflow', async () => {
       mockGetSession.mockResolvedValue({
         user: { id: 'user-id', email: 'user@example.com' },
       })
@@ -403,7 +393,6 @@ describe('Chat API Route', () => {
         hasAccess: true,
         workflow: { userId: 'user-id', workspaceId: null, isDeployed: false },
       })
-      mockReturning.mockResolvedValue([{ id: 'test-uuid' }])
 
       const req = new NextRequest('http://localhost:3000/api/chat', {
         method: 'POST',
@@ -412,10 +401,12 @@ describe('Chat API Route', () => {
       const response = await POST(req)
 
       expect(response.status).toBe(200)
-      expect(mockDeployWorkflow).toHaveBeenCalledWith({
-        workflowId: 'workflow-123',
-        deployedBy: 'user-id',
-      })
+      expect(mockPerformChatDeploy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          workflowId: 'workflow-123',
+          userId: 'user-id',
+        })
+      )
     })
   })
 })