Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 57 additions & 14 deletions apps/sim/app/api/chat/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ import { type NextRequest, NextResponse } from 'next/server'
import { v4 as uuidv4 } from 'uuid'
import { env } from '@/lib/env'
import { createLogger } from '@/lib/logs/console-logger'
import { persistExecutionLogs } from '@/lib/logs/execution-logger'
import { EnhancedLoggingSession } from '@/lib/logs/enhanced-logging-session'
import { buildTraceSpans } from '@/lib/logs/trace-spans'
import { processStreamingBlockLogs } from '@/lib/tokenization'
import { decryptSecret } from '@/lib/utils'
import { db } from '@/db'
import { chat, environment as envTable, userStats, workflow } from '@/db/schema'
Expand Down Expand Up @@ -252,11 +253,14 @@ export async function executeWorkflowForChat(

const deployment = deploymentResult[0]
const workflowId = deployment.workflowId
const executionId = uuidv4()

// Set up enhanced logging for chat execution
const loggingSession = new EnhancedLoggingSession(workflowId, executionId, 'chat', requestId)

// Check for multi-output configuration in customizations
const customizations = (deployment.customizations || {}) as Record<string, any>
let outputBlockIds: string[] = []
let outputPaths: string[] = []

// Extract output configs from the new schema format
if (deployment.outputConfigs && Array.isArray(deployment.outputConfigs)) {
Expand All @@ -271,13 +275,11 @@ export async function executeWorkflowForChat(
})

outputBlockIds = deployment.outputConfigs.map((config) => config.blockId)
outputPaths = deployment.outputConfigs.map((config) => config.path || '')
} else {
// Use customizations as fallback
outputBlockIds = Array.isArray(customizations.outputBlockIds)
? customizations.outputBlockIds
: []
outputPaths = Array.isArray(customizations.outputPaths) ? customizations.outputPaths : []
}

// Fall back to customizations if we still have no outputs
Expand All @@ -287,7 +289,6 @@ export async function executeWorkflowForChat(
customizations.outputBlockIds.length > 0
) {
outputBlockIds = customizations.outputBlockIds
outputPaths = customizations.outputPaths || new Array(outputBlockIds.length).fill('')
}

logger.debug(`[${requestId}] Using ${outputBlockIds.length} output blocks for extraction`)
Expand Down Expand Up @@ -407,6 +408,13 @@ export async function executeWorkflowForChat(
{} as Record<string, Record<string, any>>
)

// Start enhanced logging session
await loggingSession.safeStart({
userId: deployment.userId,
workspaceId: '', // TODO: Get from workflow
variables: workflowVariables,
})

const stream = new ReadableStream({
async start(controller) {
const encoder = new TextEncoder()
Expand Down Expand Up @@ -458,16 +466,41 @@ export async function executeWorkflowForChat(
},
})

const result = await executor.execute(workflowId)
// Set up enhanced logging on the executor
loggingSession.setupExecutor(executor)

let result
try {
result = await executor.execute(workflowId)
} catch (error: any) {
logger.error(`[${requestId}] Chat workflow execution failed:`, error)
await loggingSession.safeCompleteWithError({
endedAt: new Date().toISOString(),
totalDurationMs: 0,
error: {
message: error.message || 'Chat workflow execution failed',
stackTrace: error.stack,
},
})
throw error
}

if (result && 'success' in result) {
result.logs?.forEach((log: BlockLog) => {
if (streamedContent.has(log.blockId)) {
if (log.output) {
log.output.content = streamedContent.get(log.blockId)
// Update streamed content and apply tokenization
if (result.logs) {
result.logs.forEach((log: BlockLog) => {
if (streamedContent.has(log.blockId)) {
const content = streamedContent.get(log.blockId)
if (log.output) {
log.output.content = content
}
}
}
})
})

// Process all logs for streaming tokenization
const processedCount = processStreamingBlockLogs(result.logs, streamedContent)
logger.info(`[CHAT-API] Processed ${processedCount} blocks for streaming tokenization`)
}

const { traceSpans, totalDuration } = buildTraceSpans(result)
const enrichedResult = { ...result, traceSpans, totalDuration }
Expand All @@ -481,8 +514,7 @@ export async function executeWorkflowForChat(
;(enrichedResult.metadata as any).conversationId = conversationId
}
const executionId = uuidv4()
await persistExecutionLogs(workflowId, executionId, enrichedResult, 'chat')
logger.debug(`Persisted logs for deployed chat: ${executionId}`)
logger.debug(`Generated execution ID for deployed chat: ${executionId}`)
Comment on lines 516 to +517
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: using duplicate executionId - already generated on line 256

Suggested change
const executionId = uuidv4()
await persistExecutionLogs(workflowId, executionId, enrichedResult, 'chat')
logger.debug(`Persisted logs for deployed chat: ${executionId}`)
logger.debug(`Generated execution ID for deployed chat: ${executionId}`)
logger.debug(`Using execution ID for deployed chat: ${executionId}`)


if (result.success) {
try {
Expand All @@ -506,6 +538,17 @@ export async function executeWorkflowForChat(
)
}

// Complete enhanced logging session (for both success and failure)
if (result && 'success' in result) {
const { traceSpans } = buildTraceSpans(result)
await loggingSession.safeComplete({
endedAt: new Date().toISOString(),
totalDurationMs: result.metadata?.duration || 0,
finalOutput: result.output,
traceSpans,
})
}

controller.close()
},
})
Expand Down
135 changes: 135 additions & 0 deletions apps/sim/app/api/folders/[id]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ describe('Individual Folder API Route', () => {
}

const { mockAuthenticatedUser, mockUnauthenticated } = mockAuth(TEST_USER)
const mockGetUserEntityPermissions = vi.fn()

function createFolderDbMock(options: FolderDbMockOptions = {}) {
const {
Expand Down Expand Up @@ -109,6 +110,12 @@ describe('Individual Folder API Route', () => {
vi.resetModules()
vi.clearAllMocks()
setupCommonApiMocks()

mockGetUserEntityPermissions.mockResolvedValue('admin')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Default to admin permissions could mask permission-related issues in other tests. Consider setting to null or lowest permission by default.

Suggested change
mockGetUserEntityPermissions.mockResolvedValue('admin')
mockGetUserEntityPermissions.mockResolvedValue(null) // Don't assume any permissions by default


vi.doMock('@/lib/permissions/utils', () => ({
getUserEntityPermissions: mockGetUserEntityPermissions,
}))
})

afterEach(() => {
Expand Down Expand Up @@ -181,6 +188,72 @@ describe('Individual Folder API Route', () => {
expect(data).toHaveProperty('error', 'Unauthorized')
})

it('should return 403 when user has only read permissions', async () => {
mockAuthenticatedUser()
mockGetUserEntityPermissions.mockResolvedValue('read') // Read-only permissions

const dbMock = createFolderDbMock()
vi.doMock('@/db', () => dbMock)

const req = createMockRequest('PUT', {
name: 'Updated Folder',
})
const params = Promise.resolve({ id: 'folder-1' })

const { PUT } = await import('./route')

const response = await PUT(req, { params })

expect(response.status).toBe(403)

const data = await response.json()
expect(data).toHaveProperty('error', 'Write access required to update folders')
})

it('should allow folder update for write permissions', async () => {
mockAuthenticatedUser()
mockGetUserEntityPermissions.mockResolvedValue('write') // Write permissions

const dbMock = createFolderDbMock()
vi.doMock('@/db', () => dbMock)

const req = createMockRequest('PUT', {
name: 'Updated Folder',
})
const params = Promise.resolve({ id: 'folder-1' })

const { PUT } = await import('./route')

const response = await PUT(req, { params })

expect(response.status).toBe(200)

const data = await response.json()
expect(data).toHaveProperty('folder')
})

it('should allow folder update for admin permissions', async () => {
mockAuthenticatedUser()
mockGetUserEntityPermissions.mockResolvedValue('admin') // Admin permissions

const dbMock = createFolderDbMock()
vi.doMock('@/db', () => dbMock)

const req = createMockRequest('PUT', {
name: 'Updated Folder',
})
const params = Promise.resolve({ id: 'folder-1' })

const { PUT } = await import('./route')

const response = await PUT(req, { params })

expect(response.status).toBe(200)

const data = await response.json()
expect(data).toHaveProperty('folder')
})

it('should return 400 when trying to set folder as its own parent', async () => {
mockAuthenticatedUser()

Expand Down Expand Up @@ -387,6 +460,68 @@ describe('Individual Folder API Route', () => {
expect(data).toHaveProperty('error', 'Unauthorized')
})

it('should return 403 when user has only read permissions for delete', async () => {
mockAuthenticatedUser()
mockGetUserEntityPermissions.mockResolvedValue('read') // Read-only permissions

const dbMock = createFolderDbMock()
vi.doMock('@/db', () => dbMock)

const req = createMockRequest('DELETE')
const params = Promise.resolve({ id: 'folder-1' })

const { DELETE } = await import('./route')

const response = await DELETE(req, { params })

expect(response.status).toBe(403)

const data = await response.json()
expect(data).toHaveProperty('error', 'Admin access required to delete folders')
})

it('should return 403 when user has only write permissions for delete', async () => {
mockAuthenticatedUser()
mockGetUserEntityPermissions.mockResolvedValue('write') // Write permissions (not enough for delete)

const dbMock = createFolderDbMock()
vi.doMock('@/db', () => dbMock)

const req = createMockRequest('DELETE')
const params = Promise.resolve({ id: 'folder-1' })

const { DELETE } = await import('./route')

const response = await DELETE(req, { params })

expect(response.status).toBe(403)

const data = await response.json()
expect(data).toHaveProperty('error', 'Admin access required to delete folders')
})
Comment on lines +483 to +501
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Tests could be parameterized to avoid duplication between read/write permission tests.

Suggested change
it('should return 403 when user has only write permissions for delete', async () => {
mockAuthenticatedUser()
mockGetUserEntityPermissions.mockResolvedValue('write') // Write permissions (not enough for delete)
const dbMock = createFolderDbMock()
vi.doMock('@/db', () => dbMock)
const req = createMockRequest('DELETE')
const params = Promise.resolve({ id: 'folder-1' })
const { DELETE } = await import('./route')
const response = await DELETE(req, { params })
expect(response.status).toBe(403)
const data = await response.json()
expect(data).toHaveProperty('error', 'Admin access required to delete folders')
})
describe('permission-based delete restrictions', () => {
const testCases = [
{ permission: 'read', description: 'read permissions' },
{ permission: 'write', description: 'write permissions' }
]
testCases.forEach(({ permission, description }) => {
it(`should return 403 when user has only ${description} for delete`, async () => {
mockAuthenticatedUser()
mockGetUserEntityPermissions.mockResolvedValue(permission)
const dbMock = createFolderDbMock()
vi.doMock('@/db', () => dbMock)
const req = createMockRequest('DELETE')
const params = Promise.resolve({ id: 'folder-1' })
const { DELETE } = await import('./route')
const response = await DELETE(req, { params })
expect(response.status).toBe(403)
expect(await response.json()).toHaveProperty('error', 'Admin access required to delete folders')
})
})
})


it('should allow folder deletion for admin permissions', async () => {
mockAuthenticatedUser()
mockGetUserEntityPermissions.mockResolvedValue('admin') // Admin permissions

const dbMock = createFolderDbMock({
folderLookupResult: mockFolder,
})
vi.doMock('@/db', () => dbMock)

const req = createMockRequest('DELETE')
const params = Promise.resolve({ id: 'folder-1' })

const { DELETE } = await import('./route')

const response = await DELETE(req, { params })

expect(response.status).toBe(200)

const data = await response.json()
expect(data).toHaveProperty('success', true)
})

it('should handle database errors during deletion', async () => {
mockAuthenticatedUser()

Expand Down
Loading