From 33fcc36330107cfa9f4d0cc5c12fc11bc48bf67b Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Tue, 11 Aug 2026 15:49:36 -0700 Subject: [PATCH 1/7] fix(api): restore migrated endpoint compatibility --- apps/docs/openapi-v2-workflows.json | 11 ++- apps/sim/app/api/credentials/route.test.ts | 21 +++++ apps/sim/app/api/credentials/route.ts | 17 +++- .../app/api/knowledge/[id]/documents/route.ts | 2 +- .../app/api/knowledge/migrated-routes.test.ts | 43 ++++++++++ .../api/mcp/serve/[serverId]/route.test.ts | 43 ++++++++++ .../sim/app/api/mcp/serve/[serverId]/route.ts | 2 +- apps/sim/app/api/mcp/servers/route.test.ts | 57 ++++++++++++- apps/sim/app/api/mcp/servers/route.ts | 5 +- apps/sim/app/api/table/utils.test.ts | 5 +- .../app/api/tools/file/manage/route.test.ts | 21 +++-- apps/sim/app/api/tools/file/manage/route.ts | 17 ++-- apps/sim/app/api/v1/tables/route.test.ts | 82 +++++++++++++++++++ apps/sim/app/api/v1/tables/route.ts | 12 ++- .../app/api/v2/files/folders/route.test.ts | 44 +++++++++- apps/sim/app/api/v2/files/folders/route.ts | 6 +- apps/sim/app/api/v2/files/move/route.test.ts | 4 +- apps/sim/app/api/v2/lib/response.ts | 12 +++ .../[tableId]/rows/[rowId]/route.test.ts | 13 +++ .../[id]/files/[fileId]/content/route.test.ts | 15 ++++ .../[id]/files/[fileId]/content/route.ts | 2 +- .../files/folders/[folderId]/route.test.ts | 18 +++- .../[id]/files/folders/route.test.ts | 14 ++++ .../workspaces/[id]/files/move/route.test.ts | 14 ++++ .../sim/lib/api/contracts/deployments.test.ts | 22 +++++ apps/sim/lib/api/contracts/deployments.ts | 2 +- apps/sim/lib/api/contracts/workflows.ts | 5 +- .../server/routes/internal-binary-route.ts | 2 + .../server/routes/internal-json-route.test.ts | 28 +++++++ .../api/server/routes/internal-json-route.ts | 2 + .../lib/api/server/routes/v2-binary-route.ts | 3 +- .../server/routes/v2-body-lifecycle-route.ts | 3 +- .../api/server/routes/v2-json-route.test.ts | 20 +++++ .../lib/api/server/routes/v2-json-route.ts | 2 + .../lib/core/utils/with-route-handler.test.ts | 62 ++++++++++++++ apps/sim/lib/core/utils/with-route-handler.ts | 45 ++++++---- apps/sim/lib/knowledge/documents/service.ts | 2 +- apps/sim/lib/mcp/middleware.ts | 2 + .../service-filter-threading.test.ts | 12 ++- apps/sim/lib/table/application/groups.test.ts | 25 ++++++ apps/sim/lib/table/application/groups.ts | 15 +++- apps/sim/lib/table/rows/errors.ts | 6 +- apps/sim/lib/table/rows/service.ts | 33 +++----- apps/sim/lib/uploads/archive.test.ts | 12 +-- apps/sim/lib/uploads/archive.ts | 8 +- .../workspace-file-folder-manager.ts | 14 ++-- .../executor/execution-status.test.ts | 51 ++++++++++++ .../workflows/executor/execution-status.ts | 13 ++- .../api/internal-error-policies.test.ts | 13 +++ .../api/internal-error-policies.ts | 7 ++ .../download-workspace-file-items.test.ts | 15 ++-- .../download-workspace-file-items.ts | 8 -- .../workspace-file-folders.test.ts | 38 +++++++++ .../application/workspace-file-folders.ts | 36 +++++++- apps/sim/tools/agiloft/utils.test.ts | 9 +- apps/sim/tools/agiloft/utils.ts | 2 +- bun.lock | 2 +- packages/ts-sdk/README.md | 6 ++ packages/ts-sdk/package.json | 2 +- packages/ts-sdk/src/index.test.ts | 27 ++++++ packages/ts-sdk/src/index.ts | 19 ++++- 61 files changed, 906 insertions(+), 147 deletions(-) create mode 100644 apps/sim/app/api/v1/tables/route.test.ts create mode 100644 apps/sim/lib/api/contracts/deployments.test.ts create mode 100644 apps/sim/lib/core/utils/with-route-handler.test.ts diff --git a/apps/docs/openapi-v2-workflows.json b/apps/docs/openapi-v2-workflows.json index 5df4c84e884..be73a87146c 100644 --- a/apps/docs/openapi-v2-workflows.json +++ b/apps/docs/openapi-v2-workflows.json @@ -4012,8 +4012,15 @@ "type": "object", "properties": { "contextId": { - "type": "string", - "description": "Resume context identifier for the earliest active pause point." + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "description": "Resume context identifier, or null while every pause point is mid-resume." }, "pausedAt": { "type": "string", diff --git a/apps/sim/app/api/credentials/route.test.ts b/apps/sim/app/api/credentials/route.test.ts index fb90c407a9c..0e5f1c21a51 100644 --- a/apps/sim/app/api/credentials/route.test.ts +++ b/apps/sim/app/api/credentials/route.test.ts @@ -143,6 +143,25 @@ describe('POST /api/credentials', () => { auditMetadata: { principalKind: 'tenant', principalId: 'acct_123' }, principal: { kind: 'tenant', id: 'acct_123' }, }) + queueTableRows(credential, []) + queueTableRows(credential, []) + queueTableRows(credential, [ + { + id: 'credential-1', + workspaceId: WORKSPACE_ID, + type: 'service_account', + displayName: 'Zoom account acct_123', + description: null, + providerId: 'zoom-service-account', + accountId: null, + envKey: null, + envOwnerUserId: null, + encryptedServiceAccountKey: 'encrypted-blob', + createdBy: 'user-1', + createdAt: new Date('2026-08-11T00:00:00.000Z'), + updatedAt: new Date('2026-08-11T00:00:00.000Z'), + }, + ]) const req = createMockRequest('POST', { workspaceId: WORKSPACE_ID, @@ -154,8 +173,10 @@ describe('POST /api/credentials', () => { }) const response = await POST(req) + const body = await response.json() expect(response.status).toBe(201) + expect(body.credential).not.toHaveProperty('encryptedServiceAccountKey') expect(mockVerifyAndBuildServiceAccountSecret).toHaveBeenCalledTimes(1) expect(mockVerifyAndBuildServiceAccountSecret).toHaveBeenCalledWith( 'zoom-service-account', diff --git a/apps/sim/app/api/credentials/route.ts b/apps/sim/app/api/credentials/route.ts index a4c05dce002..20b4a4bcac0 100644 --- a/apps/sim/app/api/credentials/route.ts +++ b/apps/sim/app/api/credentials/route.ts @@ -274,9 +274,18 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) } + if (!result.credential) { + throw new Error('Credential creation succeeded without a credential') + } + + const responseBody = createWorkspaceCredentialContract.response.schema.parse({ + credential: { + ...result.credential, + createdAt: result.credential.createdAt.toISOString(), + updatedAt: result.credential.updatedAt.toISOString(), + }, + }) + // An existing credential matched the source: an idempotent replay, not a create. - return NextResponse.json( - { credential: result.credential }, - { status: result.created ? 201 : 200 } - ) + return NextResponse.json(responseBody, { status: result.created ? 201 : 200 }) }) diff --git a/apps/sim/app/api/knowledge/[id]/documents/route.ts b/apps/sim/app/api/knowledge/[id]/documents/route.ts index 5f372b0956e..ae6bb49230b 100644 --- a/apps/sim/app/api/knowledge/[id]/documents/route.ts +++ b/apps/sim/app/api/knowledge/[id]/documents/route.ts @@ -86,7 +86,7 @@ export const POST = defineInternalJsonRoute({ rateLimit: internalRateLimits.none({ reason: 'Preserve existing internal document-create behavior', }), - errorPolicy: internalKnowledgeErrorPolicies.documents, + errorPolicy: internalKnowledgeErrorPolicies.uploads, mapInput: ({ params, body }, { principal, request }) => { const documents = body.bulk ? body.documents : [body] return { diff --git a/apps/sim/app/api/knowledge/migrated-routes.test.ts b/apps/sim/app/api/knowledge/migrated-routes.test.ts index c25aa75d757..8284b70b2e9 100644 --- a/apps/sim/app/api/knowledge/migrated-routes.test.ts +++ b/apps/sim/app/api/knowledge/migrated-routes.test.ts @@ -114,12 +114,15 @@ vi.mock('@/lib/core/telemetry', () => ({ vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mocks.capture })) +import { OrchestrationError } from '@/lib/core/orchestration/types' +import { KnowledgeUsageLimitExceededError } from '@/lib/knowledge/application/billing' import { GET as listConnectorDocuments, PATCH as updateConnectorDocuments, } from '@/app/api/knowledge/[id]/connectors/[connectorId]/documents/route' import { PUT as updateDocument } from '@/app/api/knowledge/[id]/documents/[documentId]/route' import { + PATCH as bulkDocuments, POST as createDocuments, GET as listDocuments, } from '@/app/api/knowledge/[id]/documents/route' @@ -360,6 +363,46 @@ describe('migrated internal Knowledge routes', () => { ) }) + it('preserves payment-required for document usage admission', async () => { + mocks.createDocuments.mockRejectedValueOnce( + new KnowledgeUsageLimitExceededError('Usage limit exceeded') + ) + + const response = await createDocuments( + createMockRequest('POST', { + bulk: false, + filename: document.filename, + fileUrl: document.fileUrl, + fileSize: document.fileSize, + mimeType: document.mimeType, + }), + { params: Promise.resolve({ id: 'knowledge-1' }) } + ) + + expect(response.status).toBe(402) + await expect(response.json()).resolves.toEqual({ error: 'Usage limit exceeded' }) + expect(mocks.capture).not.toHaveBeenCalled() + }) + + it('returns not found when a bulk document selection has no active matches', async () => { + mocks.bulkDocuments.mockRejectedValueOnce( + new OrchestrationError('not_found', 'No valid documents found to update') + ) + + const response = await bulkDocuments( + createMockRequest('PATCH', { + operation: 'disable', + documentIds: ['document-1'], + }), + { params: Promise.resolve({ id: 'knowledge-1' }) } + ) + + expect(response.status).toBe(404) + await expect(response.json()).resolves.toEqual({ + error: 'No valid documents found to update', + }) + }) + it('rejects oversized document-create arrays at the contract boundary', async () => { const response = await createDocuments( createMockRequest('POST', { diff --git a/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts b/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts index 8fcc79dd65a..68e4976a7ff 100644 --- a/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts +++ b/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts @@ -974,6 +974,49 @@ describe('MCP Serve Route', () => { expect(body.result.isError).toBe(false) }) + it('reports a human-in-the-loop pause as a successful tool result', async () => { + dbChainMockFns.limit + .mockResolvedValueOnce([ + { + id: 'server-1', + name: 'Public Server', + workspaceId: 'ws-1', + isPublic: true, + createdBy: 'owner-1', + }, + ]) + .mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }]) + .mockResolvedValueOnce([{ workspaceId: 'ws-1', deploymentVersionId: 'deployment-1' }]) + + mockExecuteWorkflowService.mockResolvedValueOnce({ + ok: true, + executionId: 'exec-paused', + workflowId: 'wf-1', + status: 'paused', + aborted: null, + output: { approvalRequired: true }, + error: null, + hasResponseBlock: false, + resolvedSecretTraceProvenance: createResolvedSecretTraceProvenance('owner-1'), + }) + + const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', { + method: 'POST', + body: JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { name: 'tool_a', arguments: { q: 'test' } }, + }), + }) + const response = await POST(req, { params: Promise.resolve({ serverId: 'server-1' }) }) + const body = await response.json() + + expect(response.status).toBe(200) + expect(body.result.isError).toBe(false) + expect(body.result.content[0].text).toContain('approvalRequired') + }) + it('serializes failed runs with the structured error and child executionId', async () => { dbChainMockFns.limit .mockResolvedValueOnce([ diff --git a/apps/sim/app/api/mcp/serve/[serverId]/route.ts b/apps/sim/app/api/mcp/serve/[serverId]/route.ts index 3f4aad2a743..b7fd9db0c55 100644 --- a/apps/sim/app/api/mcp/serve/[serverId]/route.ts +++ b/apps/sim/app/api/mcp/serve/[serverId]/route.ts @@ -938,7 +938,7 @@ async function handleToolsCall( ) } - const isError = serviceResult.status !== 'completed' + const isError = serviceResult.status === 'failed' || serviceResult.status === 'cancelled' const toolOutput = isError ? { success: false, diff --git a/apps/sim/app/api/mcp/servers/route.test.ts b/apps/sim/app/api/mcp/servers/route.test.ts index beb8187dd41..b45b1296e20 100644 --- a/apps/sim/app/api/mcp/servers/route.test.ts +++ b/apps/sim/app/api/mcp/servers/route.test.ts @@ -1,11 +1,13 @@ /** * @vitest-environment node */ -import { resetDbChainMock } from '@sim/testing' +import { mcpServers } from '@sim/db/schema' +import { queueTableRows, resetDbChainMock } from '@sim/testing' import type { NextRequest } from 'next/server' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' -const { mockPerformDeleteMcpServer } = vi.hoisted(() => ({ +const { mockCanWrite, mockPerformDeleteMcpServer } = vi.hoisted(() => ({ + mockCanWrite: vi.fn(), mockPerformDeleteMcpServer: vi.fn(), })) @@ -21,6 +23,7 @@ vi.mock('@/lib/mcp/middleware', () => ({ userName: string userEmail: string workspaceId: string + canWrite: boolean requestId: string } ) => Promise @@ -31,6 +34,7 @@ vi.mock('@/lib/mcp/middleware', () => ({ userName: 'Test User', userEmail: 'test@example.com', workspaceId: 'workspace-1', + canWrite: mockCanWrite(), requestId: 'request-1', }), })) @@ -40,7 +44,13 @@ vi.mock('@/lib/mcp/orchestration', () => ({ performDeleteMcpServer: mockPerformDeleteMcpServer, })) -import { DELETE } from '@/app/api/mcp/servers/route' +import { DELETE, GET } from '@/app/api/mcp/servers/route' + +function createListRequest() { + return new Request('http://localhost:3000/api/mcp/servers?workspaceId=workspace-1', { + method: 'GET', + }) as NextRequest +} function createDeleteRequest(serverId = 'server-1') { return new Request( @@ -87,3 +97,44 @@ describe('MCP servers DELETE route', () => { expect(body).toEqual({ success: false, error: 'Failed to delete MCP server' }) }) }) + +describe('MCP servers GET route', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mockCanWrite.mockReturnValue(false) + queueTableRows(mcpServers, [ + { + id: 'server-1', + workspaceId: 'workspace-1', + name: 'Private server', + headers: { Authorization: 'Bearer secret-token' }, + oauthClientSecret: 'oauth-secret', + }, + ]) + }) + + afterAll(() => { + resetDbChainMock() + }) + + it('does not expose authentication header values to read-only users', async () => { + const response = await GET(createListRequest()) + const body = await response.json() + + expect(response.status).toBe(200) + expect(body.data.servers[0]).not.toHaveProperty('headers') + expect(body.data.servers[0]).not.toHaveProperty('oauthClientSecret') + expect(body.data.servers[0].hasOauthClientSecret).toBe(true) + }) + + it('retains configured headers for users who can manage the server', async () => { + mockCanWrite.mockReturnValue(true) + + const response = await GET(createListRequest()) + const body = await response.json() + + expect(response.status).toBe(200) + expect(body.data.servers[0].headers).toEqual({ Authorization: 'Bearer secret-token' }) + }) +}) diff --git a/apps/sim/app/api/mcp/servers/route.ts b/apps/sim/app/api/mcp/servers/route.ts index ddf5e468a9f..a4d7aa3f401 100644 --- a/apps/sim/app/api/mcp/servers/route.ts +++ b/apps/sim/app/api/mcp/servers/route.ts @@ -27,7 +27,7 @@ export const dynamic = 'force-dynamic' * GET - List all registered MCP servers for the workspace */ export const GET = withRouteHandler( - withMcpAuth('read')(async (request: NextRequest, { userId, workspaceId, requestId }) => { + withMcpAuth('read')(async (request: NextRequest, { workspaceId, canWrite, requestId }) => { try { logger.info(`[${requestId}] Listing MCP servers for workspace ${workspaceId}`) @@ -36,8 +36,9 @@ export const GET = withRouteHandler( .from(mcpServers) .where(and(eq(mcpServers.workspaceId, workspaceId), isNull(mcpServers.deletedAt))) - const servers = rows.map(({ oauthClientSecret: _secret, ...rest }) => ({ + const servers = rows.map(({ oauthClientSecret: _secret, headers, ...rest }) => ({ ...rest, + ...(canWrite ? { headers } : {}), hasOauthClientSecret: !!_secret, })) diff --git a/apps/sim/app/api/table/utils.test.ts b/apps/sim/app/api/table/utils.test.ts index fa7b57ac6dd..c2be8d0aeb4 100644 --- a/apps/sim/app/api/table/utils.test.ts +++ b/apps/sim/app/api/table/utils.test.ts @@ -4,6 +4,7 @@ import { describe, expect, it } from 'vitest' import { OrchestrationError } from '@/lib/core/orchestration/types' import { TableRowLimitError } from '@/lib/table/billing' +import { TableRowNotFoundError } from '@/lib/table/rows/errors' import type { ColumnDefinition } from '@/lib/table/types' import { rootErrorMessage, rowWriteErrorResponse, tableFilterError } from '@/app/api/table/utils' @@ -49,9 +50,7 @@ describe('rowWriteErrorResponse', () => { }) it('answers the code the failure carries, not one derived from its wording', () => { - expect( - rowWriteErrorResponse(new OrchestrationError('not_found', 'Row not found'))?.status - ).toBe(404) + expect(rowWriteErrorResponse(new TableRowNotFoundError())?.status).toBe(404) // The phrase that used to force a 400 no longer decides anything. expect( rowWriteErrorResponse(new OrchestrationError('conflict', 'Row 3: must be unique'))?.status diff --git a/apps/sim/app/api/tools/file/manage/route.test.ts b/apps/sim/app/api/tools/file/manage/route.test.ts index 74e2cec1505..d295ab39b77 100644 --- a/apps/sim/app/api/tools/file/manage/route.test.ts +++ b/apps/sim/app/api/tools/file/manage/route.test.ts @@ -91,7 +91,7 @@ vi.mock('@/lib/uploads/contexts/workspace', () => ({ })) vi.mock('@/lib/workspace-files/application/workspace-file-folders', () => ({ - createWorkspaceFileFolderOperation: { + ensureWorkspaceFileFolderPathOperation: { execute: (...args: unknown[]) => mockEnsureWorkspaceFileFolderPath(...args), }, })) @@ -196,7 +196,11 @@ describe('POST /api/tools/file/manage content provenance', () => { billedAccountUserId: 'user-1', })) mockAssertToolFileAccess.mockResolvedValue(undefined) - mockEnsureWorkspaceFileFolderPath.mockResolvedValue({ folder: { id: 'folder-1' } }) + mockEnsureWorkspaceFileFolderPath.mockImplementation( + async ({ input }: { input: { pathSegments: string[] } }) => ({ + folderId: input.pathSegments.length === 0 ? null : 'folder-1', + }) + ) mockDownloadServableFileFromStorage.mockImplementation(async (file: { name: string }) => ({ buffer: Buffer.from(`content:${file.name}`), })) @@ -349,7 +353,7 @@ describe('POST /api/tools/file/manage content provenance', () => { { operation: 'write', workspaceId: 'workspace-1', - fileName: 'Reports/secret-value.txt', + fileName: 'Reports & Plans/2026/secret-value.txt', content: 'ordinary text', __privateSecretProvenance: { version: 1, @@ -375,7 +379,7 @@ describe('POST /api/tools/file/manage content provenance', () => { expect(mockEnsureWorkspaceFileFolderPath).toHaveBeenCalledWith( expect.objectContaining({ principal: expect.objectContaining({ kind: 'delegated', subjectUserId: 'user-1' }), - input: { workspaceId: 'workspace-1', path: 'Reports' }, + input: { workspaceId: 'workspace-1', pathSegments: ['Reports & Plans', '2026'] }, }) ) expect(mockUploadWorkspaceFile).toHaveBeenCalledWith( @@ -596,7 +600,7 @@ describe('POST /api/tools/file/manage content provenance', () => { it('extracts a secret-bearing archive with unknown output provenance', async () => { const zip = new JSZip() - zip.file('child.txt', 'secret-value') + zip.file('Reports/child.txt', 'secret-value') mockDownloadFileFromStorage.mockResolvedValue( Buffer.from(await zip.generateAsync({ type: 'uint8array' })) ) @@ -620,6 +624,11 @@ describe('POST /api/tools/file/manage content provenance', () => { expect(response.status).toBe(200) expect(mockDownloadFileFromStorage).toHaveBeenCalledTimes(1) + expect(mockEnsureWorkspaceFileFolderPath).toHaveBeenCalledWith( + expect.objectContaining({ + input: { workspaceId: 'workspace-1', pathSegments: ['Reports'] }, + }) + ) expect(mockUploadWorkspaceFile).toHaveBeenCalledWith( 'workspace-1', 'user-1', @@ -628,7 +637,7 @@ describe('POST /api/tools/file/manage content provenance', () => { 'text/plain', { exactName: true, - folderId: null, + folderId: 'folder-1', folderPath: undefined, secretProvenance: { status: 'unknown' }, } diff --git a/apps/sim/app/api/tools/file/manage/route.ts b/apps/sim/app/api/tools/file/manage/route.ts index 3df9ac4ffd3..b8ee6e11ea6 100644 --- a/apps/sim/app/api/tools/file/manage/route.ts +++ b/apps/sim/app/api/tools/file/manage/route.ts @@ -62,7 +62,7 @@ import { downloadWorkspaceFileRecord } from '@/lib/workspace-files/application/r import { resolveWorkspaceFileReference } from '@/lib/workspace-files/application/resolve-workspace-file-reference' import { updateWorkspaceFileShare } from '@/lib/workspace-files/application/share-workspace-file' import { updateWorkspaceFileContent } from '@/lib/workspace-files/application/update-workspace-file-content' -import { createWorkspaceFileFolderOperation } from '@/lib/workspace-files/application/workspace-file-folders' +import { ensureWorkspaceFileFolderPathOperation } from '@/lib/workspace-files/application/workspace-file-folders' import { MAX_WORKSPACE_FILE_CONTENT_BYTES } from '@/lib/workspace-files/orchestration' import { isWorkspaceAccessDeniedError } from '@/lib/workspaces/permissions/utils' import { assertToolFileAccess } from '@/app/api/files/authorization' @@ -711,16 +711,11 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } const { folderSegments, leafName } = splitWorkspaceFilePath(fileName) await admitCreateWorkspaceFile(principal, workspaceId) - const folderId = - folderSegments.length === 0 - ? null - : ( - await createWorkspaceFileFolderOperation.execute({ - principal, - input: { workspaceId, path: folderSegments.join('/') }, - request, - }) - ).folder.id + const { folderId } = await ensureWorkspaceFileFolderPathOperation.execute({ + principal, + input: { workspaceId, pathSegments: folderSegments }, + request, + }) const mimeType = contentType || getMimeTypeFromExtension(getFileExtension(leafName)) const result = await createWorkspaceFile.execute({ principal, diff --git a/apps/sim/app/api/v1/tables/route.test.ts b/apps/sim/app/api/v1/tables/route.test.ts new file mode 100644 index 00000000000..ded5f484e8d --- /dev/null +++ b/apps/sim/app/api/v1/tables/route.test.ts @@ -0,0 +1,82 @@ +/** + * @vitest-environment node + */ +import { createMockRequest } from '@sim/testing' +import { NextResponse } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mocks, MockTableConflictError } = vi.hoisted(() => { + class MockTableConflictError extends Error { + constructor(name: string) { + super(`A table named "${name}" already exists in this workspace`) + this.name = 'TableConflictError' + } + } + + return { + mocks: { + checkRateLimit: vi.fn(), + validateWorkspaceAccess: vi.fn(), + createTable: vi.fn(), + getWorkspaceTableLimits: vi.fn(), + orchestrationErrorResponse: vi.fn(), + }, + MockTableConflictError, + } +}) + +vi.mock('@/app/api/v1/middleware', () => ({ + checkRateLimit: mocks.checkRateLimit, + createRateLimitResponse: () => NextResponse.json({ error: 'Rate limited' }, { status: 429 }), + validateWorkspaceAccess: mocks.validateWorkspaceAccess, + v1ValidationErrorResponse: (error: { issues: unknown[] }) => + NextResponse.json({ error: 'Validation error', details: error.issues }, { status: 400 }), + v1ValidationErrorResponseFromError: vi.fn(() => null), +})) + +vi.mock('@/app/api/table/utils', () => ({ + normalizeColumn: (column: unknown) => column, + orchestrationErrorResponse: mocks.orchestrationErrorResponse, +})) + +vi.mock('@/lib/table', () => ({ + createTable: mocks.createTable, + getWorkspaceTableLimits: mocks.getWorkspaceTableLimits, + listTables: vi.fn(), + TableConflictError: MockTableConflictError, +})) + +vi.mock('@sim/audit', () => ({ + AuditAction: { TABLE_CREATED: 'table.created' }, + AuditResourceType: { TABLE: 'table' }, + recordAudit: vi.fn(), +})) + +import { POST } from '@/app/api/v1/tables/route' + +describe('POST /api/v1/tables', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.checkRateLimit.mockResolvedValue({ allowed: true, userId: 'user-1' }) + mocks.validateWorkspaceAccess.mockResolvedValue(null) + mocks.getWorkspaceTableLimits.mockResolvedValue({ maxTables: 10 }) + }) + + it('preserves the legacy 400 response for a duplicate table name', async () => { + mocks.createTable.mockRejectedValue(new MockTableConflictError('Reports')) + + const response = await POST( + createMockRequest('POST', { + workspaceId: 'workspace-1', + name: 'Reports', + schema: { columns: [{ name: 'Name', type: 'string' }] }, + }) + ) + + expect(response.status).toBe(400) + await expect(response.json()).resolves.toEqual({ + error: 'A table named "Reports" already exists in this workspace', + }) + expect(mocks.orchestrationErrorResponse).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/api/v1/tables/route.ts b/apps/sim/app/api/v1/tables/route.ts index 6213fd59053..e8a13eb9090 100644 --- a/apps/sim/app/api/v1/tables/route.ts +++ b/apps/sim/app/api/v1/tables/route.ts @@ -5,7 +5,13 @@ import { v1CreateTableContract, v1ListTablesContract } from '@/lib/api/contracts import { parseRequest } from '@/lib/api/server' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { createTable, getWorkspaceTableLimits, listTables, type TableSchema } from '@/lib/table' +import { + createTable, + getWorkspaceTableLimits, + listTables, + TableConflictError, + type TableSchema, +} from '@/lib/table' import { normalizeColumn, orchestrationErrorResponse } from '@/app/api/table/utils' import { checkRateLimit, @@ -171,6 +177,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const validationResponse = v1ValidationErrorResponseFromError(error) if (validationResponse) return validationResponse + if (error instanceof TableConflictError) { + return NextResponse.json({ error: error.message }, { status: 400 }) + } + const classified = orchestrationErrorResponse(error) if (classified) return classified diff --git a/apps/sim/app/api/v2/files/folders/route.test.ts b/apps/sim/app/api/v2/files/folders/route.test.ts index f255960ce04..f95b2e9753f 100644 --- a/apps/sim/app/api/v2/files/folders/route.test.ts +++ b/apps/sim/app/api/v2/files/folders/route.test.ts @@ -64,6 +64,7 @@ vi.mock('@/lib/workspace-files/application/workspace-file-folders', () => ({ }, })) +import { WorkspaceFileFolderConflictError } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' import { DELETE, GET, PATCH, POST } from '@/app/api/v2/files/folders/route' const WORKSPACE_ID = 'workspace-1' @@ -124,6 +125,15 @@ describe('/api/v2/files/folders', () => { }) it('lists folders through the shared operation and v2 presenter', async () => { + mocks.listFolders.mockResolvedValueOnce({ + folders: [ + { + ...folder, + name: 'Quarterly Reports & Plans', + path: 'Archive/Quarterly Reports & Plans', + }, + ], + }) const response = await GET( request('GET', `/api/v2/files/folders?workspaceId=${WORKSPACE_ID}`), context @@ -132,9 +142,9 @@ describe('/api/v2/files/folders', () => { expect(response.status).toBe(200) expect((await response.json()).data).toEqual([ { - name: 'Reports', - path: '/Reports', - parentPath: '/', + name: 'Quarterly Reports & Plans', + path: '/Archive/Quarterly%20Reports%20%26%20Plans', + parentPath: '/Archive', createdAt: '2026-01-01T00:00:00.000Z', updatedAt: '2026-01-01T00:00:00.000Z', }, @@ -173,6 +183,18 @@ describe('/api/v2/files/folders', () => { }) }) + it('returns conflict when a folder already exists at the requested path', async () => { + mocks.createFolder.mockRejectedValueOnce(new WorkspaceFileFolderConflictError('Reports')) + + const response = await POST( + request('POST', '/api/v2/files/folders', { workspaceId: WORKSPACE_ID, path: '/Reports' }), + context + ) + + expect(response.status).toBe(409) + expect((await response.json()).error.code).toBe('CONFLICT') + }) + it('relocates a folder through the shared operation', async () => { const response = await PATCH( request('PATCH', '/api/v2/files/folders', { @@ -195,6 +217,22 @@ describe('/api/v2/files/folders', () => { }) }) + it('returns conflict when a folder relocation collides', async () => { + mocks.updateFolder.mockRejectedValueOnce(new WorkspaceFileFolderConflictError('Reports')) + + const response = await PATCH( + request('PATCH', '/api/v2/files/folders', { + workspaceId: WORKSPACE_ID, + path: '/Reports', + destinationPath: '/Archive/Reports', + }), + context + ) + + expect(response.status).toBe(409) + expect((await response.json()).error.code).toBe('CONFLICT') + }) + it('deletes a folder and returns the v2 deletion result', async () => { const response = await DELETE( request( diff --git a/apps/sim/app/api/v2/files/folders/route.ts b/apps/sim/app/api/v2/files/folders/route.ts index 1bbcbe91fd4..52107d2e598 100644 --- a/apps/sim/app/api/v2/files/folders/route.ts +++ b/apps/sim/app/api/v2/files/folders/route.ts @@ -5,6 +5,7 @@ import { v2RelocateFileFolderContract, } from '@/lib/api/contracts/v2/files' import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes' +import { buildFolderPath, parentFolderPath } from '@/lib/folders/paths' import { v2FileErrorPolicies } from '@/lib/workspace-files/api' import { fileOperations } from '@/lib/workspace-files/application/operations' import { @@ -18,12 +19,11 @@ export const dynamic = 'force-dynamic' export const revalidate = 0 function toV2Folder(folder: { name: string; path: string; createdAt: Date; updatedAt: Date }) { - const path = folder.path.startsWith('/') ? folder.path : `/${folder.path}` - const parentPath = path.includes('/') ? path.slice(0, path.lastIndexOf('/')) || '/' : '/' + const path = folder.path.startsWith('/') ? folder.path : buildFolderPath(folder.path.split('/')) return { name: folder.name, path, - parentPath, + parentPath: parentFolderPath(path), createdAt: folder.createdAt.toISOString(), updatedAt: folder.updatedAt.toISOString(), } diff --git a/apps/sim/app/api/v2/files/move/route.test.ts b/apps/sim/app/api/v2/files/move/route.test.ts index d192bf356b1..a311ffcb614 100644 --- a/apps/sim/app/api/v2/files/move/route.test.ts +++ b/apps/sim/app/api/v2/files/move/route.test.ts @@ -46,6 +46,7 @@ vi.mock('@/lib/workspace-files/application/move-workspace-file-items', () => ({ }, })) +import { WorkspaceFileMoveConflictError } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' import { POST } from '@/app/api/v2/files/move/route' const WS = 'workspace-1' @@ -129,8 +130,7 @@ describe('POST /api/v2/files/move', () => { }) it('maps a conflict error to 409', async () => { - const { OrchestrationError } = await import('@/lib/core/orchestration/types') - mockExecute.mockRejectedValue(new OrchestrationError('conflict', 'Name collision')) + mockExecute.mockRejectedValue(new WorkspaceFileMoveConflictError('report.csv')) const res = await callMove({ workspaceId: WS, fileIds: ['wf_1'] }) expect(res.status).toBe(409) expect((await res.json()).error.code).toBe('CONFLICT') diff --git a/apps/sim/app/api/v2/lib/response.ts b/apps/sim/app/api/v2/lib/response.ts index 475f73cb5c5..41ec572b215 100644 --- a/apps/sim/app/api/v2/lib/response.ts +++ b/apps/sim/app/api/v2/lib/response.ts @@ -3,6 +3,7 @@ import type { ZodError } from 'zod' import { type CursorKey, INVALID_CURSOR_MESSAGE } from '@/lib/api/list-query' import { getValidationErrorMessage, serializeZodIssues } from '@/lib/api/server' import { asOrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types' +import type { HttpError } from '@/lib/core/utils/http-error' import type { RateLimitResult, WorkspaceAccessError } from '@/app/api/v1/middleware' /** @@ -44,6 +45,10 @@ const STATUS_BY_CODE: Record = { SERVICE_UNAVAILABLE: 503, } +const V2_CODE_BY_HTTP_STATUS: Partial> = Object.fromEntries( + Object.entries(STATUS_BY_CODE).map(([code, status]) => [status, code as V2ErrorCode]) +) + /** * Every v2 response is authed, per-caller data (ids/filters appear in query * strings) — keep it out of shared HTTP caches unconditionally. @@ -114,6 +119,13 @@ export function v2Error( ) } +/** Renders a trusted typed HTTP error without changing the v2 envelope. */ +export function v2HttpError(error: HttpError): NextResponse { + const code = V2_CODE_BY_HTTP_STATUS[error.statusCode] + if (!code) return v2Error('INTERNAL_ERROR', 'Internal server error') + return v2Error(code, error.message) +} + /** Render a contract `ZodError` as the v2 error envelope. */ export function v2ValidationError(error: ZodError): NextResponse { return v2Error('BAD_REQUEST', getValidationErrorMessage(error, 'Invalid request'), { diff --git a/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts b/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts index a6b0472aee3..3b732ef1bb6 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts @@ -41,6 +41,7 @@ vi.mock('@/lib/table/application/rows', () => ({ })) import { OrchestrationError } from '@/lib/core/orchestration/types' +import { TableRowNotFoundError } from '@/lib/table/rows/errors' import { DELETE, GET, PATCH } from '@/app/api/v2/tables/[tableId]/rows/[rowId]/route' const WORKSPACE_ID = 'workspace-1' @@ -136,6 +137,18 @@ describe('/api/v2/tables/[tableId]/rows/[rowId]', () => { }) }) + it('returns not found when the row disappears before update', async () => { + mocks.updateRow.mockRejectedValueOnce(new TableRowNotFoundError()) + + const response = await PATCH( + request('PATCH', { workspaceId: WORKSPACE_ID, data: { name: 'Ada' } }), + CONTEXT + ) + + expect(response.status).toBe(404) + expect((await response.json()).error.code).toBe('NOT_FOUND') + }) + it('returns the compatible authoritative single-delete envelope', async () => { const req = request('DELETE') const response = await DELETE(req, CONTEXT) diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.test.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.test.ts index 14104079f62..3cfcc4664c0 100644 --- a/apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.test.ts +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.test.ts @@ -19,6 +19,7 @@ vi.mock('@/lib/workspace-files/application/update-workspace-file-content', () => }, })) +import { StorageLimitExceededError } from '@/lib/billing/storage' import { OrchestrationError } from '@/lib/core/orchestration/types' import { PUT } from '@/app/api/workspaces/[id]/files/[fileId]/content/route' @@ -132,4 +133,18 @@ describe('PUT /api/workspaces/[id]/files/[fileId]/content', () => { expect(mocks.admit).toHaveBeenCalled() expect(mocks.updateContent).not.toHaveBeenCalled() }) + + it('preserves the legacy 402 response when storage quota is exhausted', async () => { + mocks.updateContent.mockRejectedValueOnce( + new StorageLimitExceededError('Storage limit exceeded') + ) + + const response = await PUT(createRequest({ content: 'hello' }), routeContext) + + expect(response.status).toBe(402) + await expect(response.json()).resolves.toEqual({ + success: false, + error: 'Storage limit exceeded', + }) + }) }) diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.ts index c588f4e04f5..933dadbcb3a 100644 --- a/apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.ts +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.ts @@ -22,7 +22,7 @@ export const PUT = defineInternalJsonRoute({ rateLimit: internalRateLimits.none({ reason: 'Preserve existing internal content-update behavior', }), - errorPolicy: internalFileErrorPolicies.default, + errorPolicy: internalFileErrorPolicies.content, parseOptions: { maxBodyBytes: MAX_WORKSPACE_FILE_INLINE_BODY_BYTES }, beforeParse: async ({ principal, params }) => { if (typeof params.fileId === 'string') { diff --git a/apps/sim/app/api/workspaces/[id]/files/folders/[folderId]/route.test.ts b/apps/sim/app/api/workspaces/[id]/files/folders/[folderId]/route.test.ts index da38e42cde5..019a6aacc55 100644 --- a/apps/sim/app/api/workspaces/[id]/files/folders/[folderId]/route.test.ts +++ b/apps/sim/app/api/workspaces/[id]/files/folders/[folderId]/route.test.ts @@ -29,7 +29,10 @@ vi.mock('@/lib/workspace-files/application/workspace-file-folders', () => ({ }, })) -import { WorkspaceFileItemsNotFoundError } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' +import { + WorkspaceFileFolderConflictError, + WorkspaceFileItemsNotFoundError, +} from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' import { POST as RESTORE } from '@/app/api/workspaces/[id]/files/folders/[folderId]/restore/route' import { DELETE, PATCH } from '@/app/api/workspaces/[id]/files/folders/[folderId]/route' @@ -102,6 +105,19 @@ describe('/api/workspaces/[id]/files/folders/[folderId]', () => { ) }) + it('returns conflict when a folder rename collides', async () => { + mocks.updateFolder.mockRejectedValueOnce(new WorkspaceFileFolderConflictError('Reports')) + + const response = await PATCH(request('PATCH', { name: 'Reports' }), context) + + expect(response.status).toBe(409) + await expect(response.json()).resolves.toEqual({ + success: false, + error: 'A folder named "Reports" already exists in this location', + }) + expect(mocks.captureServerEvent).not.toHaveBeenCalled() + }) + it('deletes a folder through the shared use case', async () => { const response = await DELETE(request('DELETE'), context) diff --git a/apps/sim/app/api/workspaces/[id]/files/folders/route.test.ts b/apps/sim/app/api/workspaces/[id]/files/folders/route.test.ts index e9645fe19ba..a0d229bc1c0 100644 --- a/apps/sim/app/api/workspaces/[id]/files/folders/route.test.ts +++ b/apps/sim/app/api/workspaces/[id]/files/folders/route.test.ts @@ -24,6 +24,7 @@ vi.mock('@/lib/workspace-files/application/workspace-file-folders', () => ({ }, })) +import { WorkspaceFileFolderConflictError } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' import { GET, POST } from '@/app/api/workspaces/[id]/files/folders/route' const WORKSPACE_ID = 'workspace-1' @@ -105,6 +106,19 @@ describe('/api/workspaces/[id]/files/folders', () => { ) }) + it('returns conflict when a sibling folder has the requested name', async () => { + mocks.createFolder.mockRejectedValueOnce(new WorkspaceFileFolderConflictError('Reports')) + + const response = await POST(request('POST', { name: 'Reports' }), context) + + expect(response.status).toBe(409) + await expect(response.json()).resolves.toEqual({ + success: false, + error: 'A folder named "Reports" already exists in this location', + }) + expect(mocks.captureServerEvent).not.toHaveBeenCalled() + }) + it('rejects an invalid folder name before the use case', async () => { const response = await POST(request('POST', { name: 'nested/name' }), context) diff --git a/apps/sim/app/api/workspaces/[id]/files/move/route.test.ts b/apps/sim/app/api/workspaces/[id]/files/move/route.test.ts index 46c81932a2c..f3b96fe2f26 100644 --- a/apps/sim/app/api/workspaces/[id]/files/move/route.test.ts +++ b/apps/sim/app/api/workspaces/[id]/files/move/route.test.ts @@ -19,6 +19,7 @@ vi.mock('@/lib/workspace-files/application/move-workspace-file-items', () => ({ }, })) +import { WorkspaceFileMoveConflictError } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' import { POST } from '@/app/api/workspaces/[id]/files/move/route' const WORKSPACE_ID = 'workspace-1' @@ -75,6 +76,19 @@ describe('/api/workspaces/[id]/files/move', () => { expect(mocks.execute).not.toHaveBeenCalled() }) + it('returns conflict when the destination already contains the file name', async () => { + mocks.execute.mockRejectedValueOnce(new WorkspaceFileMoveConflictError('report.csv')) + + const response = await POST(request({ fileIds: ['wf_1'], targetFolderId: null }), context) + + expect(response.status).toBe(409) + await expect(response.json()).resolves.toEqual({ + success: false, + error: 'A file named "report.csv" already exists in the destination folder', + }) + expect(mocks.captureServerEvent).not.toHaveBeenCalled() + }) + it('authenticates before parsing the selection', async () => { mocks.getSession.mockResolvedValueOnce(null) diff --git a/apps/sim/lib/api/contracts/deployments.test.ts b/apps/sim/lib/api/contracts/deployments.test.ts new file mode 100644 index 00000000000..9384fd65172 --- /dev/null +++ b/apps/sim/lib/api/contracts/deployments.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest' +import { deploymentVersionOrActiveParamsSchema } from '@/lib/api/contracts/deployments' + +describe('deployment version route params', () => { + it('coerces numeric path params from the server boundary', () => { + expect(deploymentVersionOrActiveParamsSchema.parse({ id: 'workflow-1', version: '1' })).toEqual( + { id: 'workflow-1', version: 1 } + ) + }) + + it('retains the active deployment alias', () => { + expect( + deploymentVersionOrActiveParamsSchema.parse({ id: 'workflow-1', version: 'active' }) + ).toEqual({ id: 'workflow-1', version: 'active' }) + }) + + it.each(['0', '-1', '1.5', 'not-a-version'])('rejects invalid path version %s', (version) => { + expect( + deploymentVersionOrActiveParamsSchema.safeParse({ id: 'workflow-1', version }).success + ).toBe(false) + }) +}) diff --git a/apps/sim/lib/api/contracts/deployments.ts b/apps/sim/lib/api/contracts/deployments.ts index 7b5d1a1ffcd..f3e165dbd1f 100644 --- a/apps/sim/lib/api/contracts/deployments.ts +++ b/apps/sim/lib/api/contracts/deployments.ts @@ -28,7 +28,7 @@ export const deploymentVersionParamsSchema = z.object({ export const deploymentVersionOrActiveParamsSchema = z.object({ id: z.string().min(1, 'Invalid workflow ID'), - version: z.union([z.number().int().positive(), z.literal('active')]), + version: z.union([z.coerce.number().int().positive(), z.literal('active')]), }) export const deploymentVersionRouteParamsSchema = z.object({ diff --git a/apps/sim/lib/api/contracts/workflows.ts b/apps/sim/lib/api/contracts/workflows.ts index 94d33d04d1f..07945a72336 100644 --- a/apps/sim/lib/api/contracts/workflows.ts +++ b/apps/sim/lib/api/contracts/workflows.ts @@ -592,7 +592,10 @@ const workflowExecutionStatusEnum = z.enum([ ]) export const workflowExecutionPausedDetailSchema = z.object({ - contextId: z.string().describe('Resume context identifier for the earliest active pause point.'), + contextId: z + .string() + .nullable() + .describe('Resume context identifier, or null while every pause point is mid-resume.'), pausedAt: z.string().describe('ISO 8601 timestamp when the execution entered the paused state.'), resumeAt: z .string() diff --git a/apps/sim/lib/api/server/routes/internal-binary-route.ts b/apps/sim/lib/api/server/routes/internal-binary-route.ts index 5e40e070dc2..33ca0a13574 100644 --- a/apps/sim/lib/api/server/routes/internal-binary-route.ts +++ b/apps/sim/lib/api/server/routes/internal-binary-route.ts @@ -111,6 +111,8 @@ export function defineInternalBinaryRoute< } }, { + typedErrorResponse: ({ error, status }) => + NextResponse.json({ error: error.message }, { status }), unhandledErrorResponse: () => NextResponse.json({ error: 'Internal server error' }, { status: 500 }), } diff --git a/apps/sim/lib/api/server/routes/internal-json-route.test.ts b/apps/sim/lib/api/server/routes/internal-json-route.test.ts index 9449c25acd0..c57b07879ed 100644 --- a/apps/sim/lib/api/server/routes/internal-json-route.test.ts +++ b/apps/sim/lib/api/server/routes/internal-json-route.test.ts @@ -12,6 +12,11 @@ import { internalRateLimits, } from '@/lib/api/server/routes/internal-json-route' import { OrchestrationError } from '@/lib/core/orchestration/types' +import { HttpError } from '@/lib/core/utils/http-error' + +class TestLockedError extends HttpError { + readonly statusCode = 423 +} const operation = { id: 'test.read' } as const const auth = { @@ -81,6 +86,29 @@ describe('defineInternalJsonRoute', () => { await expect(response.json()).resolves.toEqual({ error: 'Already exists' }) }) + it('preserves HttpError status through the internal envelope', async () => { + const handler = defineInternalJsonRoute({ + contract, + auth, + operation, + rateLimit: internalRateLimits.none({ reason: 'Unit test' }), + errorPolicy: internalPlainOrchestrationErrorPolicy, + mapInput: () => undefined, + useCase: { + operation, + async execute(): Promise<{ value: string }> { + throw new TestLockedError('Table imports are locked') + }, + }, + }) + + const response = await handler(new NextRequest('http://localhost/api/test/internal-json-route')) + + expect(response.status).toBe(423) + await expect(response.json()).resolves.toEqual({ error: 'Table imports are locked' }) + expect(response.headers.get('x-request-id')).toBeTruthy() + }) + it('rejects invalid error statuses immediately', () => { expect(() => internalErrorResponse(200, { error: 'Invalid' })).toThrow( 'Internal error responses require a 4xx or 5xx status' diff --git a/apps/sim/lib/api/server/routes/internal-json-route.ts b/apps/sim/lib/api/server/routes/internal-json-route.ts index caa0e3429c1..aff46bae3dc 100644 --- a/apps/sim/lib/api/server/routes/internal-json-route.ts +++ b/apps/sim/lib/api/server/routes/internal-json-route.ts @@ -358,6 +358,8 @@ export function defineInternalJsonRoute< } }, { + typedErrorResponse: ({ error, status }) => + NextResponse.json({ error: error.message }, { status }), unhandledErrorResponse: () => createJsonErrorResponse( options.errorPolicy.unhandled?.() ?? diff --git a/apps/sim/lib/api/server/routes/v2-binary-route.ts b/apps/sim/lib/api/server/routes/v2-binary-route.ts index eaf1a76ef60..294fd3ca77d 100644 --- a/apps/sim/lib/api/server/routes/v2-binary-route.ts +++ b/apps/sim/lib/api/server/routes/v2-binary-route.ts @@ -16,7 +16,7 @@ import { import { parseRequest } from '@/lib/api/server/validation' import type { ApplicationOperation } from '@/lib/core/application' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { v2Error, v2ValidationError } from '@/app/api/v2/lib/response' +import { v2Error, v2HttpError, v2ValidationError } from '@/app/api/v2/lib/response' interface V2BinaryRouteOptions< C extends BinaryApiRouteContract, @@ -86,6 +86,7 @@ export function defineV2BinaryRoute< } }, { + typedErrorResponse: ({ error }) => v2HttpError(error), unhandledErrorResponse: ({ error }) => error instanceof V2RouteInfrastructureError ? v2Error('SERVICE_UNAVAILABLE', 'Service temporarily unavailable') diff --git a/apps/sim/lib/api/server/routes/v2-body-lifecycle-route.ts b/apps/sim/lib/api/server/routes/v2-body-lifecycle-route.ts index 5a6d5fd3ee8..996194751ca 100644 --- a/apps/sim/lib/api/server/routes/v2-body-lifecycle-route.ts +++ b/apps/sim/lib/api/server/routes/v2-body-lifecycle-route.ts @@ -21,7 +21,7 @@ import { } from '@/lib/api/server/validation' import type { ApplicationOperation, OperationUseCase } from '@/lib/core/application' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { v2Error, v2ValidationError } from '@/app/api/v2/lib/response' +import { v2Error, v2HttpError, v2ValidationError } from '@/app/api/v2/lib/response' interface V2BodyLifecycleAdmission< O extends ApplicationOperation, @@ -168,6 +168,7 @@ export function defineV2BodyLifecycleRoute< } }, { + typedErrorResponse: ({ error }) => v2HttpError(error), unhandledErrorResponse: ({ error }) => error instanceof V2RouteInfrastructureError ? v2Error('SERVICE_UNAVAILABLE', 'Service temporarily unavailable') diff --git a/apps/sim/lib/api/server/routes/v2-json-route.test.ts b/apps/sim/lib/api/server/routes/v2-json-route.test.ts index ea4f0ba6710..43cc24f26ca 100644 --- a/apps/sim/lib/api/server/routes/v2-json-route.test.ts +++ b/apps/sim/lib/api/server/routes/v2-json-route.test.ts @@ -16,6 +16,11 @@ import { defineRouteContract } from '@/lib/api/contracts' import type { ParsedRequest } from '@/lib/api/server/validation' import type { OperationUseCase } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' +import { HttpError } from '@/lib/core/utils/http-error' + +class TestLockedError extends HttpError { + readonly statusCode = 423 +} vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => v2ApiKeyAuthModuleMock) vi.mock('@/lib/core/rate-limiter', () => v2RateLimiterModuleMock) @@ -357,6 +362,21 @@ describe('defineV2JsonRoute', () => { expect(response.headers.get('X-RateLimit-Remaining')).toBe('99') }) + it('preserves HttpError status through the v2 envelope', async () => { + const response = await createHandler({ + execute: async () => { + throw new TestLockedError('Resource is locked') + }, + })(request()) + + expect(response.status).toBe(423) + await expect(response.json()).resolves.toEqual({ + error: { code: 'LOCKED', message: 'Resource is locked' }, + }) + expect(response.headers.get('cache-control')).toBe('private, no-store') + expect(response.headers.get('X-RateLimit-Remaining')).toBe('99') + }) + it('validates the presented response before onSuccess', async () => { const onSuccess = vi.fn() const response = await createHandler({ diff --git a/apps/sim/lib/api/server/routes/v2-json-route.ts b/apps/sim/lib/api/server/routes/v2-json-route.ts index 0f0e4210226..109338c0cd7 100644 --- a/apps/sim/lib/api/server/routes/v2-json-route.ts +++ b/apps/sim/lib/api/server/routes/v2-json-route.ts @@ -22,6 +22,7 @@ import { v2ApiGateError } from '@/app/api/v2/lib/gate' import { v2CaughtOrchestrationError, v2Error, + v2HttpError, v2RateLimitError, v2ValidationError, } from '@/app/api/v2/lib/response' @@ -275,6 +276,7 @@ export function defineV2JsonRoute< } }, { + typedErrorResponse: ({ error }) => v2HttpError(error), unhandledErrorResponse: ({ error }) => error instanceof V2RouteInfrastructureError ? v2Error('SERVICE_UNAVAILABLE', 'Service temporarily unavailable') diff --git a/apps/sim/lib/core/utils/with-route-handler.test.ts b/apps/sim/lib/core/utils/with-route-handler.test.ts new file mode 100644 index 00000000000..c69b4f38fdc --- /dev/null +++ b/apps/sim/lib/core/utils/with-route-handler.test.ts @@ -0,0 +1,62 @@ +/** + * @vitest-environment node + */ + +import { NextRequest, NextResponse } from 'next/server' +import { describe, expect, it, vi } from 'vitest' +import { HttpError } from '@/lib/core/utils/http-error' +import { withRouteHandler } from '@/lib/core/utils/with-route-handler' + +class TestHttpError extends HttpError { + constructor( + message: string, + readonly statusCode: number + ) { + super(message) + } +} + +describe('withRouteHandler', () => { + it('lets a route family render a typed error before its generic fallback', async () => { + const unhandledErrorResponse = vi.fn(() => + NextResponse.json({ family: 'generic' }, { status: 500 }) + ) + const handler = withRouteHandler( + async () => { + throw new TestHttpError('Locked', 423) + }, + { + typedErrorResponse: ({ error, status }) => + NextResponse.json({ family: 'typed', error: error.message }, { status }), + unhandledErrorResponse, + } + ) + + const response = await handler(new NextRequest('http://localhost/api/test'), undefined) + + expect(response.status).toBe(423) + await expect(response.json()).resolves.toEqual({ family: 'typed', error: 'Locked' }) + expect(unhandledErrorResponse).not.toHaveBeenCalled() + expect(response.headers.get('x-request-id')).toBeTruthy() + }) + + it.each([Number.NaN, 399, 429.5, 600])( + 'does not expose an invalid typed status %s', + async (statusCode) => { + const handler = withRouteHandler( + async () => { + throw new TestHttpError('Do not expose', statusCode) + }, + { + typedErrorResponse: ({ status }) => NextResponse.json({ family: 'typed' }, { status }), + unhandledErrorResponse: () => NextResponse.json({ family: 'generic' }, { status: 500 }), + } + ) + + const response = await handler(new NextRequest('http://localhost/api/test'), undefined) + + expect(response.status).toBe(500) + await expect(response.json()).resolves.toEqual({ family: 'generic' }) + } + ) +}) diff --git a/apps/sim/lib/core/utils/with-route-handler.ts b/apps/sim/lib/core/utils/with-route-handler.ts index 32ccc7fc788..6313437e6a4 100644 --- a/apps/sim/lib/core/utils/with-route-handler.ts +++ b/apps/sim/lib/core/utils/with-route-handler.ts @@ -18,7 +18,14 @@ interface RouteHandlerErrorContext { requestId: string } +interface RouteHandlerTypedErrorContext { + error: HttpError + requestId: string + status: number +} + interface RouteHandlerOptions { + typedErrorResponse?: (context: RouteHandlerTypedErrorContext) => NextResponse | Response unhandledErrorResponse?: (context: RouteHandlerErrorContext) => NextResponse | Response } @@ -38,11 +45,11 @@ interface RouteHandlerOptions { * safe to expose to clients (no stack traces, secrets, file paths, ORM * internals). */ -function readTypedErrorStatus(error: unknown): number | undefined { +function readTypedError(error: unknown): RouteHandlerTypedErrorContext['error'] | undefined { if (!(error instanceof HttpError)) return undefined const status = error.statusCode - if (status < 400 || status >= 600) return undefined - return status + if (!Number.isInteger(status) || status < 400 || status >= 600) return undefined + return error } /** @@ -94,28 +101,30 @@ export function withRouteHandler( } catch (error) { const duration = Date.now() - startTime const message = getErrorMessage(error, 'Unknown error') - if (options.unhandledErrorResponse) { - logger.error('Unhandled route error', { duration, error: message }) - response = options.unhandledErrorResponse({ error, requestId }) - applyResponseHeaders(response, request, requestId) - return response - } - - const typedStatus = readTypedErrorStatus(error) - if (typedStatus !== undefined) { + const typedError = readTypedError(error) + if (typedError) { + const typedStatus = typedError.statusCode if (typedStatus >= 500) { logger.error('Unhandled route error', { duration, status: typedStatus, error: message }) } else { logger.warn('Typed route error', { duration, status: typedStatus, error: message }) } - response = NextResponse.json({ error: message, requestId }, { status: typedStatus }) - } else { + response = options.typedErrorResponse + ? options.typedErrorResponse({ error: typedError, requestId, status: typedStatus }) + : NextResponse.json({ error: message, requestId }, { status: typedStatus }) + applyResponseHeaders(response, request, requestId) + return response + } + + if (options.unhandledErrorResponse) { logger.error('Unhandled route error', { duration, error: message }) - response = NextResponse.json( - { error: 'Internal server error', requestId }, - { status: 500 } - ) + response = options.unhandledErrorResponse({ error, requestId }) + applyResponseHeaders(response, request, requestId) + return response } + + logger.error('Unhandled route error', { duration, error: message }) + response = NextResponse.json({ error: 'Internal server error', requestId }, { status: 500 }) applyResponseHeaders(response, request, requestId) return response } diff --git a/apps/sim/lib/knowledge/documents/service.ts b/apps/sim/lib/knowledge/documents/service.ts index 35895a65689..5a896d8c9a2 100644 --- a/apps/sim/lib/knowledge/documents/service.ts +++ b/apps/sim/lib/knowledge/documents/service.ts @@ -2182,7 +2182,7 @@ export async function bulkDocumentOperation( ) if (documentsToUpdate.length === 0) { - throw new Error('No valid documents found to update') + throw new OrchestrationError('not_found', 'No valid documents found to update') } if (documentsToUpdate.length !== documentIds.length) { diff --git a/apps/sim/lib/mcp/middleware.ts b/apps/sim/lib/mcp/middleware.ts index 0018aa3ebc4..2eedf3ae416 100644 --- a/apps/sim/lib/mcp/middleware.ts +++ b/apps/sim/lib/mcp/middleware.ts @@ -24,6 +24,7 @@ export interface McpAuthContext { userEmail?: string | null authType?: AuthTypeValue workspaceId: string + canWrite: boolean requestId: string } @@ -196,6 +197,7 @@ async function validateMcpAuth( userEmail: auth.userEmail, authType: auth.authType, workspaceId, + canWrite: permissionSatisfies(userPermissions as PermissionType, 'write'), requestId, }, } diff --git a/apps/sim/lib/table/__tests__/service-filter-threading.test.ts b/apps/sim/lib/table/__tests__/service-filter-threading.test.ts index 2730adf8431..c0e3432a822 100644 --- a/apps/sim/lib/table/__tests__/service-filter-threading.test.ts +++ b/apps/sim/lib/table/__tests__/service-filter-threading.test.ts @@ -181,10 +181,10 @@ describe('bulk update/delete limited-subset ordering', () => { expect(dbChainMockFns.limit).toHaveBeenCalledWith(5) }) - it('orders and caps an updateRowsByFilter without an explicit limit', async () => { + it('updates every filter match without an implicit limit', async () => { await updateRowsByFilter(TABLE, { filter: { score: { $gt: 0 } }, data: { name: 'x' } }, 'req-1') - expect(dbChainMockFns.orderBy).toHaveBeenCalled() - expect(dbChainMockFns.limit).toHaveBeenCalledWith(TABLE_LIMITS.MAX_BULK_OPERATION_SIZE + 1) + expect(dbChainMockFns.orderBy).not.toHaveBeenCalled() + expect(dbChainMockFns.limit).not.toHaveBeenCalled() }) it('orders the match query when deleteRowsByFilter has a limit', async () => { @@ -192,6 +192,12 @@ describe('bulk update/delete limited-subset ordering', () => { expect(dbChainMockFns.orderBy).toHaveBeenCalled() expect(dbChainMockFns.limit).toHaveBeenCalledWith(3) }) + + it('deletes every filter match without an implicit limit', async () => { + await deleteRowsByFilter(TABLE, { filter: { score: { $gt: 0 } } }, 'req-1') + expect(dbChainMockFns.orderBy).not.toHaveBeenCalled() + expect(dbChainMockFns.limit).not.toHaveBeenCalled() + }) }) describe('queryRows byte budget', () => { diff --git a/apps/sim/lib/table/application/groups.test.ts b/apps/sim/lib/table/application/groups.test.ts index 228467271b3..ec6ddc3a0cc 100644 --- a/apps/sim/lib/table/application/groups.test.ts +++ b/apps/sim/lib/table/application/groups.test.ts @@ -316,6 +316,31 @@ describe('workflow and enrichment Table application commands', () => { expect(mocks.audit).not.toHaveBeenCalled() }) + it('preserves an existing output coordinate that is no longer pickable', async () => { + mocks.loadWorkflowOutputs.mockResolvedValueOnce({ + ...resolvedWorkflow, + outputs: resolvedWorkflow.outputs.filter((output) => output.blockId !== 'block-1'), + }) + + await updateTableGroupUseCase.execute({ + principal, + input: { + tableId: table.id, + workspaceId: table.workspaceId, + groupId: group.id, + name: 'Renamed group', + outputs: group.outputs, + }, + }) + + expect(mocks.resolveWorkflowContext).not.toHaveBeenCalled() + expect(mocks.loadWorkflowOutputs).not.toHaveBeenCalled() + expect(mocks.updateGroup).toHaveBeenCalledWith( + expect.objectContaining({ outputs: group.outputs, name: 'Renamed group' }), + 'request-1' + ) + }) + it('rejects an invalid output before constructing or mutating the group', async () => { await expect( createWorkflowTableGroup.execute({ diff --git a/apps/sim/lib/table/application/groups.ts b/apps/sim/lib/table/application/groups.ts index 143a4a4213a..60bd4ca5129 100644 --- a/apps/sim/lib/table/application/groups.ts +++ b/apps/sim/lib/table/application/groups.ts @@ -548,9 +548,18 @@ export const updateTableGroupUseCase = defineAuthorizedTableUseCase({ const previousGroup = (context.table.schema.workflowGroups ?? []).find( (group) => group.id === input.groupId ) + const previousOutputKeys = new Set( + previousGroup?.outputs.map((output) => `${output.blockId}::${output.path}`) ?? [] + ) + const workflowChanged = + input.workflowId !== undefined && input.workflowId !== previousGroup?.workflowId + const outputCoordinatesToValidate = + input.outputs?.filter( + (output) => workflowChanged || !previousOutputKeys.has(`${output.blockId}::${output.path}`) + ) ?? [] const workflowMetadataRequired = input.workflowId !== undefined || - input.outputs !== undefined || + outputCoordinatesToValidate.length > 0 || (input.mappingUpdates?.length ?? 0) > 0 const targetWorkflowId = input.workflowId ?? previousGroup?.workflowId let resolvedWorkflow: ResolveWorkflowOutputsResult | undefined @@ -562,8 +571,8 @@ export const updateTableGroupUseCase = defineAuthorizedTableUseCase({ targetWorkflowId, context.workspaceId ) - if (input.outputs && input.outputs.length > 0) { - validateRequestedOutputs(input.outputs, resolvedWorkflow, targetWorkflowId) + if (outputCoordinatesToValidate.length > 0) { + validateRequestedOutputs(outputCoordinatesToValidate, resolvedWorkflow, targetWorkflowId) } } const actorUserId = attributedUserId(principal, context.billedAccountUserId) diff --git a/apps/sim/lib/table/rows/errors.ts b/apps/sim/lib/table/rows/errors.ts index 897dde9a0e9..480a42db148 100644 --- a/apps/sim/lib/table/rows/errors.ts +++ b/apps/sim/lib/table/rows/errors.ts @@ -1,7 +1,9 @@ +import { OrchestrationError } from '@/lib/core/orchestration/types' + /** Raised when a row disappears before an operation can mutate it. */ -export class TableRowNotFoundError extends Error { +export class TableRowNotFoundError extends OrchestrationError { constructor() { - super('Row not found') + super('not_found', 'Row not found') this.name = 'TableRowNotFoundError' } } diff --git a/apps/sim/lib/table/rows/service.ts b/apps/sim/lib/table/rows/service.ts index 7fd91263ac5..9f1a9042472 100644 --- a/apps/sim/lib/table/rows/service.ts +++ b/apps/sim/lib/table/rows/service.ts @@ -1823,25 +1823,18 @@ export async function updateRowsByFilter( // A limit selects a SUBSET, so impose the default `(order_key, id)` order — // without it Postgres returns planner-arbitrary rows and "update the first N" // is nondeterministic. Sort is irrelevant (and skipped) when every match is updated. - // Tenant-bounded: the jsonb filter is unestimatable and otherwise sends the planner to a - // whole-shared-relation seq scan (14.4s measured on a 1M-row table). const matchingRows = await withSeqscanOff(async (trx) => { const base = trx .select({ id: userTableRows.id, data: userTableRows.data }) .from(userTableRows) .where(and(baseConditions, filterClause)) - return base - .orderBy(buildRowOrderBySql(undefined, tableName, table.schema.columns)) - .limit(data.limit ?? TABLE_LIMITS.MAX_BULK_OPERATION_SIZE + 1) + return data.limit === undefined + ? base + : base + .orderBy(buildRowOrderBySql(undefined, tableName, table.schema.columns)) + .limit(data.limit) }) - if (matchingRows.length > TABLE_LIMITS.MAX_BULK_OPERATION_SIZE) { - throw new OrchestrationError( - 'validation', - `Cannot update more than ${TABLE_LIMITS.MAX_BULK_OPERATION_SIZE} rows per operation` - ) - } - if (matchingRows.length === 0) { return { affectedCount: 0, affectedRowIds: [] } } @@ -2262,24 +2255,18 @@ export async function deleteRowsByFilter( // A limit deletes a SUBSET, so order deterministically by `(order_key, id)` — // see updateRowsByFilter. Unbounded deletes affect every match, so order is moot. - // Tenant-bounded for the same reason as updateRowsByFilter — see withSeqscanOff. const matchingRows = await withSeqscanOff(async (trx) => { const base = trx .select({ id: userTableRows.id, position: userTableRows.position }) .from(userTableRows) .where(and(baseConditions, filterClause)) - return base - .orderBy(buildRowOrderBySql(undefined, tableName, table.schema.columns)) - .limit(data.limit ?? TABLE_LIMITS.MAX_BULK_OPERATION_SIZE + 1) + return data.limit === undefined + ? base + : base + .orderBy(buildRowOrderBySql(undefined, tableName, table.schema.columns)) + .limit(data.limit) }) - if (matchingRows.length > TABLE_LIMITS.MAX_BULK_OPERATION_SIZE) { - throw new OrchestrationError( - 'validation', - `Cannot delete more than ${TABLE_LIMITS.MAX_BULK_OPERATION_SIZE} rows per operation` - ) - } - if (matchingRows.length === 0) { return { affectedCount: 0, affectedRowIds: [] } } diff --git a/apps/sim/lib/uploads/archive.test.ts b/apps/sim/lib/uploads/archive.test.ts index c4180dbba8e..4d76283c963 100644 --- a/apps/sim/lib/uploads/archive.test.ts +++ b/apps/sim/lib/uploads/archive.test.ts @@ -11,7 +11,7 @@ const { mockEnsureFolder, mockUpload, mockDelete } = vi.hoisted(() => ({ mockDelete: vi.fn(), })) vi.mock('@/lib/workspace-files/application/workspace-file-folders', () => ({ - createWorkspaceFileFolderOperation: { + ensureWorkspaceFileFolderPathOperation: { execute: mockEnsureFolder, }, })) @@ -85,7 +85,7 @@ function craftCentralDirectory(records: number, extraPerRecord: number): Buffer beforeEach(() => { vi.clearAllMocks() - mockEnsureFolder.mockResolvedValue({ folder: { id: 'folder_1' } }) + mockEnsureFolder.mockResolvedValue({ folderId: 'folder_1' }) mockDelete.mockResolvedValue(undefined) mockUpload.mockImplementation( async ({ input }: { input: { content: Buffer; name: string } }) => ({ @@ -103,7 +103,7 @@ beforeEach(() => { describe('decompressArchiveBufferToWorkspaceFiles', () => { it('extracts entries as workspace files under the root folder', async () => { - const buffer = await buildZip({ 'report.txt': 'hi', 'data/sheet.csv': 'a,b' }) + const buffer = await buildZip({ 'report.txt': 'hi', 'data & sheets/sheet.csv': 'a,b' }) const result = await decompressArchiveBufferToWorkspaceFiles(buffer, { workspaceId: 'ws', @@ -118,10 +118,12 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => { expect(leafNames).toEqual(['report.txt', 'sheet.csv']) // Entries are rooted under the archive's folder; nested paths are preserved. expect(mockEnsureFolder).toHaveBeenCalledWith( - expect.objectContaining({ input: { workspaceId: 'ws', path: 'bundle' } }) + expect.objectContaining({ input: { workspaceId: 'ws', pathSegments: ['bundle'] } }) ) expect(mockEnsureFolder).toHaveBeenCalledWith( - expect.objectContaining({ input: { workspaceId: 'ws', path: 'bundle/data' } }) + expect.objectContaining({ + input: { workspaceId: 'ws', pathSegments: ['bundle', 'data & sheets'] }, + }) ) }) diff --git a/apps/sim/lib/uploads/archive.ts b/apps/sim/lib/uploads/archive.ts index e019a22956d..8eb2ea82d37 100644 --- a/apps/sim/lib/uploads/archive.ts +++ b/apps/sim/lib/uploads/archive.ts @@ -7,7 +7,7 @@ import type { WorkspaceFileSecretProvenance } from '@/lib/uploads/contexts/works import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils' import { createWorkspaceFileFromBuffer } from '@/lib/workspace-files/application/create-workspace-file' import { deleteWorkspaceFileOperation } from '@/lib/workspace-files/application/delete-workspace-file' -import { createWorkspaceFileFolderOperation } from '@/lib/workspace-files/application/workspace-file-folders' +import { ensureWorkspaceFileFolderPathOperation } from '@/lib/workspace-files/application/workspace-file-folders' import type { UserFile } from '@/executor/types' /** @@ -365,11 +365,11 @@ export async function decompressArchiveBufferToWorkspaceFiles( if (folderSegments.length === 0) { folderId = null } else { - const result = await createWorkspaceFileFolderOperation.execute({ + const result = await ensureWorkspaceFileFolderPathOperation.execute({ principal, - input: { workspaceId, path: folderSegments.join('/') }, + input: { workspaceId, pathSegments: folderSegments }, }) - folderId = result.folder.id + folderId = result.folderId } folderIdCache.set(folderKey, folderId) } diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts index 7b3645a9161..03f11ee7710 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts @@ -32,19 +32,17 @@ const isFileFolder = eq(folderTable.resourceType, FILE_FOLDER_RESOURCE_TYPE) export type WorkspaceFileFolderScope = 'active' | 'archived' | 'all' -export class WorkspaceFileFolderConflictError extends Error { - readonly code = 'FOLDER_CONFLICT' as const - +export class WorkspaceFileFolderConflictError extends OrchestrationError { constructor(name: string) { - super(`A folder named "${name}" already exists in this location`) + super('conflict', `A folder named "${name}" already exists in this location`) + this.name = 'WorkspaceFileFolderConflictError' } } -export class WorkspaceFileMoveConflictError extends Error { - readonly code = 'FILE_MOVE_CONFLICT' as const - +export class WorkspaceFileMoveConflictError extends OrchestrationError { constructor(name: string) { - super(`A file named "${name}" already exists in the destination folder`) + super('conflict', `A file named "${name}" already exists in the destination folder`) + this.name = 'WorkspaceFileMoveConflictError' } } diff --git a/apps/sim/lib/workflows/executor/execution-status.test.ts b/apps/sim/lib/workflows/executor/execution-status.test.ts index b5ab71f2e72..632a758ab83 100644 --- a/apps/sim/lib/workflows/executor/execution-status.test.ts +++ b/apps/sim/lib/workflows/executor/execution-status.test.ts @@ -325,4 +325,55 @@ describe('getWorkflowExecutionStatus queue projection', () => { blockedOnBlockId: 'approval-block', }) }) + + it('returns null pause coordinates while every pause point is mid-resume', async () => { + queueTableRows(schemaMock.workflowExecutionLogs, [ + { + executionId: 'execution-1', + workflowId: 'workflow-1', + workspaceId: 'workspace-1', + status: 'paused', + level: 'info', + trigger: 'api', + startedAt: new Date('2026-08-05T12:00:00.000Z'), + endedAt: null, + totalDurationMs: null, + executionData: null, + costTotal: null, + }, + ]) + queueTableRows(schemaMock.resumeQueue, []) + queueTableRows(schemaMock.pausedExecutions, [ + { + id: 'paused-execution-1', + status: 'partially_resumed', + pausePoints: { + 'context-1': { + contextId: 'context-1', + blockId: 'approval-block', + response: null, + registeredAt: '2026-08-05T12:00:01.000Z', + resumeStatus: 'resuming', + snapshotReady: true, + pauseKind: 'human', + }, + }, + metadata: {}, + resumedCount: 0, + pausedAt: new Date('2026-08-05T12:00:01.000Z'), + nextResumeAt: null, + }, + ]) + + const status = await getWorkflowExecutionStatus(input) + + expect(status?.status).toBe('paused') + expect(status?.paused).toMatchObject({ + contextId: null, + resumeAt: null, + pauseKind: null, + blockedOnBlockId: null, + pausePointCount: 1, + }) + }) }) diff --git a/apps/sim/lib/workflows/executor/execution-status.ts b/apps/sim/lib/workflows/executor/execution-status.ts index c3352dd014d..363b57c1efa 100644 --- a/apps/sim/lib/workflows/executor/execution-status.ts +++ b/apps/sim/lib/workflows/executor/execution-status.ts @@ -242,18 +242,15 @@ export async function getWorkflowExecutionStatus( if (isCurrentlyPaused && pausedRow) { const points = normalizePausePoints(pausedRow.pausePoints) const earliest = pickEarliestPausePoint(points) - if (!earliest) { - throw new Error('Paused execution has no active resume context') - } const automaticResumeWaiting = getAutomaticResumeWaitingMetadata(pausedRow.metadata) paused = { - contextId: earliest.contextId, + contextId: earliest?.contextId ?? null, pausedAt: pausedRow.pausedAt.toISOString(), - resumeAt: pausedRow.nextResumeAt?.toISOString() ?? earliest.resumeAt ?? null, - pauseKind: earliest.pauseKind, - blockedOnBlockId: earliest.blockId ?? null, + resumeAt: pausedRow.nextResumeAt?.toISOString() ?? earliest?.resumeAt ?? null, + pauseKind: earliest?.pauseKind ?? null, + blockedOnBlockId: earliest?.blockId ?? null, automaticResumeWaitingReason: - automaticResumeWaiting?.reason ?? earliest.automaticResumeWaitingReason ?? null, + automaticResumeWaiting?.reason ?? earliest?.automaticResumeWaitingReason ?? null, pausedExecutionId: pausedRow.id, pausePointCount: points.length, resumedCount: pausedRow.resumedCount, diff --git a/apps/sim/lib/workspace-files/api/internal-error-policies.test.ts b/apps/sim/lib/workspace-files/api/internal-error-policies.test.ts index 641514f0ef7..ed962991516 100644 --- a/apps/sim/lib/workspace-files/api/internal-error-policies.test.ts +++ b/apps/sim/lib/workspace-files/api/internal-error-policies.test.ts @@ -2,6 +2,7 @@ * @vitest-environment node */ import { describe, expect, it } from 'vitest' +import { StorageLimitExceededError } from '@/lib/billing/storage' import { OrchestrationError } from '@/lib/core/orchestration/types' import { internalFileErrorPolicies } from '@/lib/workspace-files/api/internal-error-policies' import { @@ -32,4 +33,16 @@ describe('internal file error policies', () => { headers: undefined, }) }) + + it('preserves the legacy payment-required status for storage quota failures', () => { + expect( + internalFileErrorPolicies.content.project( + new StorageLimitExceededError('Storage limit exceeded') + ) + ).toEqual({ + status: 402, + body: { success: false, error: 'Storage limit exceeded' }, + headers: undefined, + }) + }) }) diff --git a/apps/sim/lib/workspace-files/api/internal-error-policies.ts b/apps/sim/lib/workspace-files/api/internal-error-policies.ts index c5814cde15a..2dad228d3c8 100644 --- a/apps/sim/lib/workspace-files/api/internal-error-policies.ts +++ b/apps/sim/lib/workspace-files/api/internal-error-policies.ts @@ -6,6 +6,7 @@ import { internalOrchestrationErrorPolicy, internalPlainOrchestrationErrorPolicy, } from '@/lib/api/server/routes' +import { StorageLimitExceededError } from '@/lib/billing/storage' import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types' import { CompiledCheckTooLargeError, @@ -30,6 +31,11 @@ const compiledCheck = extendInternalErrorPolicy(internalPlainOrchestrationErrorP return null }) +const content = extendInternalErrorPolicy(internalOrchestrationErrorPolicy, (error) => { + if (!(error instanceof StorageLimitExceededError)) return null + return internalErrorResponse(402, { success: false, error: error.message }) +}) + const downloadUrl: InternalErrorPolicy = { project(error) { const typed = internalOrchestrationErrorPolicy.project(error) @@ -79,6 +85,7 @@ const inline: InternalErrorPolicy = { export const internalFileErrorPolicies = { default: internalOrchestrationErrorPolicy, plain: internalPlainOrchestrationErrorPolicy, + content, style, compiledCheck, downloadUrl, diff --git a/apps/sim/lib/workspace-files/application/download-workspace-file-items.test.ts b/apps/sim/lib/workspace-files/application/download-workspace-file-items.test.ts index ae370bc4038..fd017ba1b02 100644 --- a/apps/sim/lib/workspace-files/application/download-workspace-file-items.test.ts +++ b/apps/sim/lib/workspace-files/application/download-workspace-file-items.test.ts @@ -188,13 +188,12 @@ describe('downloadWorkspaceFileItems', () => { expect(mockRecordAudit).not.toHaveBeenCalled() }) - it('rejects unknown selected IDs rather than exposing another workspace selection', async () => { - mockListFiles.mockResolvedValue([]) - await expect( - downloadWorkspaceFileItems.execute({ - principal, - input: { workspaceId: 'ws-1', fileIds: ['other-workspace-file'], folderIds: [] }, - }) - ).rejects.toEqual(expect.objectContaining({ code: 'not_found' })) + it('ignores stale selected IDs when another selected file still resolves', async () => { + const result = await downloadWorkspaceFileItems.execute({ + principal, + input: { workspaceId: 'ws-1', fileIds: ['f1', 'stale-file'], folderIds: [] }, + }) + + expect(result.filesToZip.map((item) => item.id)).toEqual(['f1']) }) }) diff --git a/apps/sim/lib/workspace-files/application/download-workspace-file-items.ts b/apps/sim/lib/workspace-files/application/download-workspace-file-items.ts index 98611e62a0c..afbe5e026ef 100644 --- a/apps/sim/lib/workspace-files/application/download-workspace-file-items.ts +++ b/apps/sim/lib/workspace-files/application/download-workspace-file-items.ts @@ -84,14 +84,6 @@ async function executeDownloadWorkspaceFileItems({ listWorkspaceFileFolders(context.workspaceId), ]) const folderPaths = buildWorkspaceFileFolderPathMap(folders) - const knownFileIds = new Set(files.map((file) => file.id)) - const knownFolderIds = new Set(folders.map((folder) => folder.id)) - if ( - fileIds.some((fileId) => !knownFileIds.has(fileId)) || - folderIds.some((folderId) => !knownFolderIds.has(folderId)) - ) { - throw new OrchestrationError('not_found', 'File selection not found') - } const selectedFolderIds = collectDescendantFolderIds(folderIds, folders) const requestedFileIds = new Set(fileIds) const filesToZip = files.filter( diff --git a/apps/sim/lib/workspace-files/application/workspace-file-folders.test.ts b/apps/sim/lib/workspace-files/application/workspace-file-folders.test.ts index 0608deaedbe..2f550327008 100644 --- a/apps/sim/lib/workspace-files/application/workspace-file-folders.test.ts +++ b/apps/sim/lib/workspace-files/application/workspace-file-folders.test.ts @@ -10,6 +10,8 @@ const { mockAssertItems, mockArchive, mockCreate, + mockEnsure, + mockList, mockRelocate, mockRestore, mockAudit, @@ -21,6 +23,8 @@ const { mockAssertItems: vi.fn(), mockArchive: vi.fn(), mockCreate: vi.fn(), + mockEnsure: vi.fn(), + mockList: vi.fn(), mockRelocate: vi.fn(), mockRestore: vi.fn(), mockAudit: vi.fn(), @@ -31,6 +35,8 @@ vi.mock('@/lib/uploads/contexts/workspace', () => ({ assertWorkspaceFileItemsBelongToWorkspace: mockAssertItems, bulkArchiveWorkspaceFileItems: mockArchive, createWorkspaceFileFolderAtPath: mockCreate, + ensureWorkspaceFileFolderPath: mockEnsure, + listWorkspaceFileFolders: mockList, loadWorkspaceFileOperationContext: mockLoadContext, relocateWorkspaceFileFolderByPath: mockRelocate, restoreWorkspaceFileFolder: mockRestore, @@ -55,6 +61,8 @@ vi.mock('@/lib/realtime/notify', () => ({ notifyWorkspaceFilesChanged: mockNotif import { createWorkspaceFileFolderOperation, deleteWorkspaceFileFolderOperation, + ensureWorkspaceFileFolderPathOperation, + listWorkspaceFileFoldersOperation, restoreWorkspaceFileFolderOperation, updateWorkspaceFileFolderOperation, } from '@/lib/workspace-files/application/workspace-file-folders' @@ -113,6 +121,36 @@ describe('workspace file folder operations', () => { expect(mockNotify).toHaveBeenCalledOnce() }) + it('matches a canonical encoded parent path against decoded stored folder paths', async () => { + mockList.mockResolvedValue([ + { ...folder, id: 'child-1', name: 'Q1', path: 'Reports & Plans/Q1' }, + { ...folder, id: 'other-1', name: 'Other', path: 'Archive/Other' }, + ]) + + const result = await listWorkspaceFileFoldersOperation.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { workspaceId: 'ws-1', parentPath: '/Reports%20%26%20Plans' }, + }) + + expect(result.folders.map((item) => item.id)).toEqual(['child-1']) + }) + + it('ensures an entire decoded folder chain for a file write', async () => { + mockEnsure.mockResolvedValue('nested-folder') + + const result = await ensureWorkspaceFileFolderPathOperation.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { workspaceId: 'ws-1', pathSegments: ['Reports', '2026'] }, + }) + + expect(result.folderId).toBe('nested-folder') + expect(mockEnsure).toHaveBeenCalledWith({ + workspaceId: 'ws-1', + userId: 'user-1', + pathSegments: ['Reports', '2026'], + }) + }) + it('relocates a canonical path folder without invoking legacy orchestration', async () => { mockRelocate.mockResolvedValue({ folder, path: '/Archive/Reports' }) const result = await updateWorkspaceFileFolderOperation.execute({ diff --git a/apps/sim/lib/workspace-files/application/workspace-file-folders.ts b/apps/sim/lib/workspace-files/application/workspace-file-folders.ts index c8fb8f9d2ff..28cdc478b4e 100644 --- a/apps/sim/lib/workspace-files/application/workspace-file-folders.ts +++ b/apps/sim/lib/workspace-files/application/workspace-file-folders.ts @@ -2,6 +2,7 @@ import { AuditAction, AuditResourceType } from '@sim/audit' import { resolvePrincipalAttribution } from '@sim/auth/principal' import { createLogger } from '@sim/logger' import { OrchestrationError } from '@/lib/core/orchestration/types' +import { parseFolderPath } from '@/lib/folders/paths' import { notifyWorkspaceFilesChanged } from '@/lib/realtime/notify' import { assertWorkspaceFileItemsBelongToWorkspace, @@ -9,6 +10,7 @@ import { createWorkspaceFileFolder, createWorkspaceFileFolderAtPath, deleteWorkspaceFileFolderByPath, + ensureWorkspaceFileFolderPath, listWorkspaceFileFolders, loadWorkspaceFileOperationContext, relocateWorkspaceFileFolderByPath, @@ -46,6 +48,15 @@ export interface CreateWorkspaceFileFolderResult { folder: WorkspaceFileFolderRecord } +export interface EnsureWorkspaceFileFolderPathInput { + workspaceId: string + pathSegments: string[] +} + +export interface EnsureWorkspaceFileFolderPathResult { + folderId: string | null +} + export interface UpdateWorkspaceFileFolderInput { workspaceId: string folderId?: string @@ -98,7 +109,7 @@ async function executeListWorkspaceFileFolders(args: { scope: args.input.scope, }) if (args.input.parentPath !== undefined) { - const parentPath = args.input.parentPath === '/' ? '' : args.input.parentPath.replace(/^\//, '') + const parentPath = parseFolderPath(args.input.parentPath).join('/') folders = folders.filter((folder) => { const parent = folder.path.includes('/') ? folder.path.slice(0, folder.path.lastIndexOf('/')) @@ -148,6 +159,22 @@ async function executeCreateWorkspaceFileFolder(args: { return { folder } } +async function executeEnsureWorkspaceFileFolderPath(args: { + principal: Parameters[0] + input: EnsureWorkspaceFileFolderPathInput + context: FolderOperationContext +}): Promise { + const attribution = resolvePrincipalAttribution(args.principal, { + workspaceBillingOwnerUserId: args.context.billedAccountUserId, + }) + const folderId = await ensureWorkspaceFileFolderPath({ + workspaceId: args.context.workspaceId, + userId: attribution.attributedUserId, + pathSegments: args.input.pathSegments, + }) + return { folderId } +} + async function executeUpdateWorkspaceFileFolder(args: { input: UpdateWorkspaceFileFolderInput context: FolderOperationContext @@ -240,6 +267,13 @@ export const createWorkspaceFileFolderOperation = defineAuthorizedWorkspaceFileU }, }) +export const ensureWorkspaceFileFolderPathOperation = defineAuthorizedWorkspaceFileUseCase({ + operation: fileOperations.create, + resolveContext: (args: { input: EnsureWorkspaceFileFolderPathInput }) => + resolveFolderContext(args), + execute: executeEnsureWorkspaceFileFolderPath, +}) + export const updateWorkspaceFileFolderOperation = defineAuthorizedWorkspaceFileUseCase({ operation: fileOperations.updateFolder, resolveContext: (args: { input: UpdateWorkspaceFileFolderInput }) => resolveFolderContext(args), diff --git a/apps/sim/tools/agiloft/utils.test.ts b/apps/sim/tools/agiloft/utils.test.ts index bfffee607da..6cbe1d62fce 100644 --- a/apps/sim/tools/agiloft/utils.test.ts +++ b/apps/sim/tools/agiloft/utils.test.ts @@ -81,9 +81,6 @@ describe('CRUD endpoints', () => { it('does not ask for the .json variant on operations whose JSON shape is undocumented', () => { expect(buildSearchRecordsUrl(BASE, { ...baseParams, query: "a='b'" })).not.toContain('.json') - expect( - buildLockRecordUrl(BASE, { ...baseParams, recordId: '18', lockAction: 'check' }) - ).not.toContain('.json') }) }) @@ -150,6 +147,12 @@ describe('buildSearchRecordsUrl', () => { }) describe('buildLockRecordUrl', () => { + it('requests the JSON variant consumed by the lock route', () => { + expect( + buildLockRecordUrl(BASE, { ...baseParams, recordId: '18', lockAction: 'check' }) + ).toContain('/EWLock/.json?') + }) + it('adds force only on unlock, where EWLock accepts it', () => { const unlock = buildLockRecordUrl(BASE, { ...baseParams, diff --git a/apps/sim/tools/agiloft/utils.ts b/apps/sim/tools/agiloft/utils.ts index 8f9d084cde9..4d26964e110 100644 --- a/apps/sim/tools/agiloft/utils.ts +++ b/apps/sim/tools/agiloft/utils.ts @@ -162,7 +162,7 @@ export function buildAttachmentInfoUrl(base: string, params: AgiloftAttachmentIn */ export function buildLockRecordUrl(base: string, params: AgiloftLockRecordParams): string { const id = encodeURIComponent(params.recordId.trim()) - let url = `${base}/ewws/EWLock?${buildEwBaseQuery(params)}&id=${id}` + let url = `${base}/ewws/EWLock/.json?${buildEwBaseQuery(params)}&id=${id}` if (params.lockAction === 'unlock' && params.force) { url += '&force=true' } diff --git a/bun.lock b/bun.lock index f951e9d18e3..bc17d95213d 100644 --- a/bun.lock +++ b/bun.lock @@ -611,7 +611,7 @@ }, "packages/ts-sdk": { "name": "simstudio-ts-sdk", - "version": "0.1.2", + "version": "0.1.3", "devDependencies": { "@sim/tsconfig": "workspace:*", "@types/node": "24.2.1", diff --git a/packages/ts-sdk/README.md b/packages/ts-sdk/README.md index ac01b7c3965..10c9ddd008b 100644 --- a/packages/ts-sdk/README.md +++ b/packages/ts-sdk/README.md @@ -80,6 +80,8 @@ const result = await client.executeWorkflow('workflow-id', { message: 'Hello' }, **Returns:** `Promise` +Synchronous executions that finish with `status: 'failed'` reject with `SimStudioError`. + ##### getWorkflowStatus(workflowId) Get the status of a workflow (deployment status, etc.). @@ -232,12 +234,16 @@ client.setBaseUrl('https://my-custom-domain.com'); ```typescript interface WorkflowExecutionResult { success: boolean; + executionId?: string; output?: any; error?: string; logs?: any[]; metadata?: { duration?: number; + executionId?: string; runId?: string; + startTime?: string; + endTime?: string; [key: string]: any; }; traceSpans?: any[]; diff --git a/packages/ts-sdk/package.json b/packages/ts-sdk/package.json index 1974aeba580..21e38369e4e 100644 --- a/packages/ts-sdk/package.json +++ b/packages/ts-sdk/package.json @@ -1,6 +1,6 @@ { "name": "simstudio-ts-sdk", - "version": "0.1.2", + "version": "0.1.3", "description": "Sim SDK - Execute workflows programmatically", "type": "module", "exports": { diff --git a/packages/ts-sdk/src/index.test.ts b/packages/ts-sdk/src/index.test.ts index 267ad113635..b0e4eec3bf8 100644 --- a/packages/ts-sdk/src/index.test.ts +++ b/packages/ts-sdk/src/index.test.ts @@ -12,6 +12,8 @@ function v2ExecutionResponse(output: unknown = {}) { status: 'completed', output, error: null, + startedAt: '2026-08-11T12:00:00.000Z', + endedAt: '2026-08-11T12:00:00.010Z', durationMs: 10, }, } @@ -164,10 +166,35 @@ describe('SimStudioClient', () => { ) expect(result).toHaveProperty('success', true) + expect(result).toHaveProperty('executionId', 'execution-123') expect(result).toHaveProperty('output') + expect(result).toHaveProperty('metadata.executionId', 'execution-123') + expect(result).toHaveProperty('metadata.startTime', '2026-08-11T12:00:00.000Z') + expect(result).toHaveProperty('metadata.endTime', '2026-08-11T12:00:00.010Z') expect(result).not.toHaveProperty('jobId') }) + it('throws when a sync workflow run completes with failed status', async () => { + const failed = v2ExecutionResponse({ partial: true }) + failed.data.status = 'failed' + failed.data.error = { + code: 'BLOCK_EXECUTION_FAILED', + message: 'Invalid credentials', + } + vi.mocked(mockFetch).mockResolvedValue({ + ok: true, + status: 200, + json: vi.fn().mockResolvedValue(failed), + headers: { get: vi.fn().mockReturnValue(null) }, + } as any) + + await expect(client.executeWorkflow('workflow-id', {})).rejects.toMatchObject({ + name: 'SimStudioError', + code: 'BLOCK_EXECUTION_FAILED', + message: 'Invalid credentials', + }) + }) + it('should not set X-Execution-Mode header when async is undefined', async () => { const mockResponse = { ok: true, diff --git a/packages/ts-sdk/src/index.ts b/packages/ts-sdk/src/index.ts index c2bdb8f1c62..8d2b5706a28 100644 --- a/packages/ts-sdk/src/index.ts +++ b/packages/ts-sdk/src/index.ts @@ -17,12 +17,16 @@ export interface LargeValueRef { export interface WorkflowExecutionResult { success: boolean + executionId?: string output?: any error?: string logs?: any[] metadata?: { duration?: number + executionId?: string runId?: string + startTime?: string + endTime?: string [key: string]: any } traceSpans?: any[] @@ -346,6 +350,8 @@ export class SimStudioClient { status?: 'completed' | 'failed' | 'paused' | 'cancelled' output?: unknown error?: WorkflowExecutionError | null + startedAt?: string + endedAt?: string durationMs?: number } } @@ -366,13 +372,24 @@ export class SimStudioClient { } } + if (result.data.status === 'failed') { + throw new SimStudioError( + result.data.error?.message || 'Workflow execution failed', + result.data.error?.code || 'EXECUTION_FAILED' + ) + } + return { - success: result.data.status !== 'failed', + success: result.data.status === 'completed' || result.data.status === 'paused', + executionId: result.data.runId, output: result.data.output, error: result.data.error?.message, metadata: { duration: result.data.durationMs, + executionId: result.data.runId, runId: result.data.runId, + startTime: result.data.startedAt, + endTime: result.data.endedAt, }, totalDuration: result.data.durationMs, } From 5bfa4d01f115928e9800ee960d7fdd0cd497d267 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Tue, 11 Aug 2026 16:22:06 -0700 Subject: [PATCH 2/7] fix(api): close remaining migration regressions --- .../[documentId]/chunks/[chunkId]/route.ts | 6 +- .../documents/[documentId]/chunks/route.ts | 6 +- .../app/api/tools/file/manage/route.test.ts | 34 ++ apps/sim/app/api/tools/file/manage/route.ts | 3 +- .../app/api/v2/tables/[tableId]/route.test.ts | 7 + apps/sim/app/api/v2/tables/route.test.ts | 7 + apps/sim/app/api/v2/tables/utils.ts | 43 +- .../tools/server/knowledge/knowledge-base.ts | 3 + .../guardrails/validate_hallucination.test.ts | 35 +- .../lib/guardrails/validate_hallucination.ts | 22 +- apps/sim/lib/knowledge/api/internal-route.ts | 8 +- .../knowledge/application/authorization.ts | 15 +- .../authorized-knowledge-use-case.ts | 138 +++++- apps/sim/lib/knowledge/application/billing.ts | 31 +- apps/sim/lib/knowledge/application/chunks.ts | 6 +- .../knowledge/application/connectors.test.ts | 1 + .../lib/knowledge/application/connectors.ts | 41 +- .../knowledge/application/contexts.test.ts | 57 +++ .../sim/lib/knowledge/application/contexts.ts | 60 ++- .../knowledge/application/documents.test.ts | 43 ++ .../lib/knowledge/application/documents.ts | 81 ++-- .../lib/knowledge/application/search.test.ts | 52 +++ apps/sim/lib/knowledge/application/search.ts | 71 ++- .../lib/knowledge/application/tags.test.ts | 1 + apps/sim/lib/knowledge/application/tags.ts | 12 +- .../service-filter-threading.test.ts | 12 +- apps/sim/lib/table/rows/service.ts | 423 ++++++++++++------ apps/sim/tools/agiloft/utils.test.ts | 9 +- apps/sim/tools/agiloft/utils.ts | 2 +- packages/ts-sdk/src/index.test.ts | 2 +- 30 files changed, 941 insertions(+), 290 deletions(-) diff --git a/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/[chunkId]/route.ts b/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/[chunkId]/route.ts index 92e8ef049e2..a5d4fdb84d4 100644 --- a/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/[chunkId]/route.ts +++ b/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/[chunkId]/route.ts @@ -31,7 +31,7 @@ function resolveContentProvenance( request: NextRequest, principal: Principal, payload: unknown, - workspaceId: string, + workspaceId: string | undefined, includeContent: boolean ) { const resolved = resolveKnowledgeWriteSecretProvenance({ @@ -39,7 +39,7 @@ function resolveContentProvenance( payload, authType: internalKnowledgeAuthType(principal), userId: internalKnowledgeActorUserId(principal), - workspaceId, + ...(workspaceId ? { workspaceId } : {}), selectionKeys: includeContent ? ['chunk-content'] : [], }) if (!resolved.success) { @@ -93,7 +93,7 @@ export const PUT = defineInternalJsonRoute({ chunkId: params.chunkId, content: body.content, enabled: body.enabled, - resolveContentProvenance: ({ workspaceId }: { workspaceId: string }) => + resolveContentProvenance: ({ workspaceId }: { workspaceId?: string }) => resolveContentProvenance(request, principal, body, workspaceId, body.content !== undefined), }), useCase: updateKnowledgeChunk, diff --git a/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.ts b/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.ts index 44afacfe847..c7461356308 100644 --- a/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.ts +++ b/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.ts @@ -32,7 +32,7 @@ function resolveContentProvenance( request: NextRequest, principal: Principal, payload: unknown, - workspaceId: string, + workspaceId: string | undefined, includeContent: boolean ) { const resolved = resolveKnowledgeWriteSecretProvenance({ @@ -40,7 +40,7 @@ function resolveContentProvenance( payload, authType: internalKnowledgeAuthType(principal), userId: internalKnowledgeActorUserId(principal), - workspaceId, + ...(workspaceId ? { workspaceId } : {}), selectionKeys: includeContent ? ['chunk-content'] : [], }) if (!resolved.success) { @@ -95,7 +95,7 @@ export const POST = defineInternalJsonRoute({ documentId: params.documentId, content: body.content, enabled: body.enabled, - resolveContentProvenance: ({ workspaceId }: { workspaceId: string }) => + resolveContentProvenance: ({ workspaceId }: { workspaceId?: string }) => resolveContentProvenance(request, principal, body, workspaceId, true), }), useCase: createKnowledgeChunk, diff --git a/apps/sim/app/api/tools/file/manage/route.test.ts b/apps/sim/app/api/tools/file/manage/route.test.ts index d295ab39b77..7fa6d78b4ac 100644 --- a/apps/sim/app/api/tools/file/manage/route.test.ts +++ b/apps/sim/app/api/tools/file/manage/route.test.ts @@ -15,6 +15,7 @@ const { mockGetBoundWorkspaceFileSecretProvenance, mockLoadActiveWorkspaceContext, mockLoadActiveWorkspaceFileContext, + mockMoveWorkspaceFileItems, mockResolveEffectiveWorkspacePermission, mockGetFileMetadataByKey, mockGetWorkspaceFile, @@ -31,6 +32,7 @@ const { mockGetBoundWorkspaceFileSecretProvenance: vi.fn(), mockLoadActiveWorkspaceContext: vi.fn(), mockLoadActiveWorkspaceFileContext: vi.fn(), + mockMoveWorkspaceFileItems: vi.fn(), mockResolveEffectiveWorkspacePermission: vi.fn(), mockGetFileMetadataByKey: vi.fn(), mockGetWorkspaceFile: vi.fn(), @@ -96,6 +98,12 @@ vi.mock('@/lib/workspace-files/application/workspace-file-folders', () => ({ }, })) +vi.mock('@/lib/workspace-files/application/move-workspace-file-items', () => ({ + moveWorkspaceFileItemsOperation: { + execute: (...args: unknown[]) => mockMoveWorkspaceFileItems(...args), + }, +})) + vi.mock('@/lib/core/config/redis', () => ({ acquireLock: vi.fn(async () => true), releaseLock: vi.fn(async () => undefined), @@ -206,6 +214,7 @@ describe('POST /api/tools/file/manage content provenance', () => { })) mockFetchWorkspaceFileBuffer.mockResolvedValue(Buffer.from('before')) mockUpdateWorkspaceFileContent.mockResolvedValue({ file: workspaceFile('file-1') }) + mockMoveWorkspaceFileItems.mockResolvedValue({ moved: 1 }) mockUploadWorkspaceFile.mockResolvedValue({ id: 'new-file', name: 'new.txt', @@ -422,6 +431,31 @@ describe('POST /api/tools/file/manage content provenance', () => { ) }) + it.each([ + ['Reports & Plans/2026', '/Reports%20%26%20Plans/2026'], + ['', '/'], + ])('moves files to the canonical folder path for %j', async (targetFolder, expectedPath) => { + const response = await POST( + createMockRequest('POST', { + operation: 'move', + workspaceId: 'workspace-1', + fileId: 'file-1', + targetFolder, + }) + ) + + expect(response.status).toBe(200) + expect(mockMoveWorkspaceFileItems).toHaveBeenCalledWith( + expect.objectContaining({ + input: { + workspaceId: 'workspace-1', + fileIds: ['file-1'], + targetFolderPath: expectedPath, + }, + }) + ) + }) + it('persists an authenticated file write with unavailable lineage as unknown', async () => { const response = await POST( createMockRequest( diff --git a/apps/sim/app/api/tools/file/manage/route.ts b/apps/sim/app/api/tools/file/manage/route.ts index b8ee6e11ea6..7c73861446e 100644 --- a/apps/sim/app/api/tools/file/manage/route.ts +++ b/apps/sim/app/api/tools/file/manage/route.ts @@ -26,6 +26,7 @@ import { requestsPrivateToolMetadata, } from '@/lib/execution/private-tool-metadata' import { isSupportedFileType, parseBuffer } from '@/lib/file-parsers' +import { buildFolderPath } from '@/lib/folders/paths' import { getSharesForResources, ShareValidationError } from '@/lib/public-shares/share-manager' import { ArchiveError, @@ -766,7 +767,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { input: { workspaceId, fileIds: [fileId], - targetFolderPath: pathSegments.join('/'), + targetFolderPath: buildFolderPath(pathSegments), }, request, }) diff --git a/apps/sim/app/api/v2/tables/[tableId]/route.test.ts b/apps/sim/app/api/v2/tables/[tableId]/route.test.ts index f17703b74ea..2dc9cf380a6 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/route.test.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/route.test.ts @@ -15,6 +15,7 @@ const mocks = vi.hoisted(() => ({ remove: vi.fn(), capture: vi.fn(), getUserEmailsByIds: vi.fn(), + getMaxRowsPerTable: vi.fn(), })) vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => ({ @@ -39,6 +40,9 @@ vi.mock('@/lib/users/queries', () => ({ getUserEmailsByIds: mocks.getUserEmailsByIds, requireResolvedUserEmail: (emails: Map, userId: string) => emails.get(userId)!, })) +vi.mock('@/lib/table/billing', () => ({ + getMaxRowsPerTable: mocks.getMaxRowsPerTable, +})) import { OrchestrationError } from '@/lib/core/orchestration/types' import { DELETE, GET, PATCH } from '@/app/api/v2/tables/[tableId]/route' @@ -107,6 +111,7 @@ describe('/api/v2/tables/[tableId]', () => { mocks.operationRate.mockResolvedValue(rate) mocks.gate.mockResolvedValue(null) mocks.getUserEmailsByIds.mockResolvedValue(new Map([['owner-1', 'owner@example.com']])) + mocks.getMaxRowsPerTable.mockResolvedValue(5000) mocks.read.mockResolvedValue({ table, folderPath: '/' }) mocks.update.mockResolvedValue({ table, @@ -132,6 +137,7 @@ describe('/api/v2/tables/[tableId]', () => { expect((await response.json()).data.table).toMatchObject({ id: 'table-1', ownerEmail: 'owner@example.com', + maxRows: 5000, }) expect(mocks.read).toHaveBeenCalledWith({ principal, @@ -150,6 +156,7 @@ describe('/api/v2/tables/[tableId]', () => { expect((await response.json()).data.table).toMatchObject({ name: 'Contacts', ownerEmail: 'owner@example.com', + maxRows: 5000, }) }) diff --git a/apps/sim/app/api/v2/tables/route.test.ts b/apps/sim/app/api/v2/tables/route.test.ts index 3af4675170f..897f343a4a3 100644 --- a/apps/sim/app/api/v2/tables/route.test.ts +++ b/apps/sim/app/api/v2/tables/route.test.ts @@ -13,6 +13,7 @@ const mocks = vi.hoisted(() => ({ list: vi.fn(), create: vi.fn(), getUserEmailsByIds: vi.fn(), + getMaxRowsPerTable: vi.fn(), })) vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => ({ @@ -35,6 +36,9 @@ vi.mock('@/lib/users/queries', () => ({ getUserEmailsByIds: mocks.getUserEmailsByIds, requireResolvedUserEmail: (emails: Map, userId: string) => emails.get(userId)!, })) +vi.mock('@/lib/table/billing', () => ({ + getMaxRowsPerTable: mocks.getMaxRowsPerTable, +})) import { GET, POST } from '@/app/api/v2/tables/route' @@ -90,6 +94,7 @@ describe('/api/v2/tables', () => { mocks.operationRate.mockResolvedValue(rate) mocks.gate.mockResolvedValue(null) mocks.getUserEmailsByIds.mockResolvedValue(new Map([['owner-1', 'owner@example.com']])) + mocks.getMaxRowsPerTable.mockResolvedValue(5000) mocks.list.mockResolvedValue({ tables: [{ table, folderPath: '/' }], nextKeys: undefined, @@ -113,6 +118,7 @@ describe('/api/v2/tables', () => { folderPath: '/', description: null, ownerEmail: 'owner@example.com', + maxRows: 5000, }, ], nextCursor: null, @@ -166,6 +172,7 @@ describe('/api/v2/tables', () => { expect((await response.json()).data.table).toMatchObject({ id: 'table-1', ownerEmail: 'owner@example.com', + maxRows: 5000, }) expect(mocks.create).toHaveBeenCalledWith({ principal, diff --git a/apps/sim/app/api/v2/tables/utils.ts b/apps/sim/app/api/v2/tables/utils.ts index f46d3fa1c17..474dbd8bcf2 100644 --- a/apps/sim/app/api/v2/tables/utils.ts +++ b/apps/sim/app/api/v2/tables/utils.ts @@ -3,6 +3,7 @@ import type { V2ApiTable } from '@/lib/api/contracts/v2/tables' import type { OrchestrationErrorCode } from '@/lib/core/orchestration/types' import type { MultipartError } from '@/lib/core/utils/multipart' import type { RowData, TableDefinition, TablePredicate, TableSchema } from '@/lib/table' +import { getMaxRowsPerTable } from '@/lib/table/billing' import { getColumnId } from '@/lib/table/column-keys' import { TableLockedError } from '@/lib/table/mutation-locks' import { predicateToFilter } from '@/lib/table/query-builder/converters' @@ -35,6 +36,17 @@ function toIso(value: Date | string): string { return value instanceof Date ? value.toISOString() : String(value) } +function requireMaxRows( + maxRowsByWorkspaceId: ReadonlyMap, + workspaceId: string +): number { + const maxRows = maxRowsByWorkspaceId.get(workspaceId) + if (maxRows === undefined) { + throw new Error(`Table plan limit is missing for workspace ${workspaceId}`) + } + return maxRows +} + /** * Resolves a public v2 bulk-op predicate to the storage-id-keyed legacy `Filter` * the row runners consume. The public wire is column-NAME-keyed: shape-check @@ -58,7 +70,8 @@ export function v2BulkPredicateToFilter(predicate: TablePredicate, schema: Table function serializeApiTable( table: TableDefinition, folderPath: string, - ownerEmail: string + ownerEmail: string, + maxRows: number ): V2ApiTable { return { id: table.id, @@ -69,7 +82,7 @@ function serializeApiTable( columns: (table.schema as TableSchema).columns.map(normalizeColumn), }, rowCount: table.rowCount, - maxRows: table.maxRows, + maxRows, folderPath, locks: table.locks, // `jobStatus` is the presence signal — the service leaves the whole group @@ -91,11 +104,15 @@ function serializeApiTable( /** Resolves and serializes one table with public owner attribution. */ export async function toApiTable(table: TableDefinition, folderPath: string): Promise { - const emailByUserId = await getUserEmailsByIds([table.createdBy]) + const [emailByUserId, maxRows] = await Promise.all([ + getUserEmailsByIds([table.createdBy]), + getMaxRowsPerTable(table.workspaceId), + ]) return serializeApiTable( table, folderPath, - requireResolvedUserEmail(emailByUserId, table.createdBy) + requireResolvedUserEmail(emailByUserId, table.createdBy), + maxRows ) } @@ -103,9 +120,23 @@ export async function toApiTable(table: TableDefinition, folderPath: string): Pr export async function toApiTables( entries: readonly { table: TableDefinition; folderPath: string }[] ): Promise { - const emailByUserId = await getUserEmailsByIds(entries.map(({ table }) => table.createdBy)) + const workspaceIds = [...new Set(entries.map(({ table }) => table.workspaceId))] + const [emailByUserId, limits] = await Promise.all([ + getUserEmailsByIds(entries.map(({ table }) => table.createdBy)), + Promise.all( + workspaceIds.map( + async (workspaceId) => [workspaceId, await getMaxRowsPerTable(workspaceId)] as const + ) + ), + ]) + const maxRowsByWorkspaceId = new Map(limits) return entries.map(({ table, folderPath }) => - serializeApiTable(table, folderPath, requireResolvedUserEmail(emailByUserId, table.createdBy)) + serializeApiTable( + table, + folderPath, + requireResolvedUserEmail(emailByUserId, table.createdBy), + requireMaxRows(maxRowsByWorkspaceId, table.workspaceId) + ) ) } diff --git a/apps/sim/lib/copilot/tools/server/knowledge/knowledge-base.ts b/apps/sim/lib/copilot/tools/server/knowledge/knowledge-base.ts index d6f29c34cef..2e97eaf547d 100644 --- a/apps/sim/lib/copilot/tools/server/knowledge/knowledge-base.ts +++ b/apps/sim/lib/copilot/tools/server/knowledge/knowledge-base.ts @@ -953,6 +953,9 @@ export const knowledgeBaseServerTool: BaseServerTool ({ +const { mockDecryptSecret, mockExecuteProviderRequest, mockGenerateInternalDelegationToken } = + vi.hoisted(() => ({ mockDecryptSecret: vi.fn(), mockExecuteProviderRequest: vi.fn(), - mockGenerateInternalToken: vi.fn(), - }) -) + mockGenerateInternalDelegationToken: vi.fn(), + })) vi.mock('@/lib/auth/internal', () => ({ - generateInternalToken: mockGenerateInternalToken, + generateInternalDelegationToken: mockGenerateInternalDelegationToken, })) vi.mock('@/lib/core/security/encryption', () => ({ @@ -87,7 +86,7 @@ function createInput(registry: ResolvedSecretTraceRegistry) { describe('validateHallucination', () => { beforeEach(() => { vi.clearAllMocks() - mockGenerateInternalToken.mockResolvedValue('minted-internal-token') + mockGenerateInternalDelegationToken.mockResolvedValue('minted-internal-token') mockDecryptSecret.mockImplementation(async (encryptedValue: string) => ({ decrypted: encryptedValue === 'encrypted-reference-secret' ? 'reference-secret' : encryptedValue, @@ -133,7 +132,10 @@ describe('validateHallucination', () => { const result = await validateHallucination(createInput(registry)) expect(result).toMatchObject({ passed: true, score: 8 }) - expect(mockGenerateInternalToken).toHaveBeenCalledWith('user-1') + expect(mockGenerateInternalDelegationToken).toHaveBeenCalledWith({ + subjectUserId: 'user-1', + workflowId: 'workflow-1', + }) const [, searchOptions] = fetchMock.mock.calls[0] const searchBody = JSON.parse(String(searchOptions?.body)) as { @@ -197,6 +199,23 @@ describe('validateHallucination', () => { expect(registry.isComplete()).toBe(true) }) + it('fails validation when the delegated Knowledge query is rejected', async () => { + const registry = new ResolvedSecretTraceRegistry() + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response(null, { status: 401 })) + ) + + const result = await validateHallucination(createInput(registry)) + + expect(result).toEqual({ + passed: false, + error: + 'Validation error: Failed to query knowledge base: Knowledge base query failed with status 401', + }) + expect(mockExecuteProviderRequest).not.toHaveBeenCalled() + }) + /** * Forwarding the caller's signal means the scoring model can now be aborted. A * cancelled run must not be reported as a guardrail verdict — `passed: false` would diff --git a/apps/sim/lib/guardrails/validate_hallucination.ts b/apps/sim/lib/guardrails/validate_hallucination.ts index 4ffcf56d99b..cfa1f07e73d 100644 --- a/apps/sim/lib/guardrails/validate_hallucination.ts +++ b/apps/sim/lib/guardrails/validate_hallucination.ts @@ -1,9 +1,10 @@ import { db } from '@sim/db' import { account } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' import { isPlainRecord } from '@sim/utils/object' import { eq } from 'drizzle-orm' -import { generateInternalToken } from '@/lib/auth/internal' +import { generateInternalDelegationToken } from '@/lib/auth/internal' import { BILLING_ATTRIBUTION_HEADER, type BillingAttributionSnapshot, @@ -93,9 +94,14 @@ async function queryKnowledgeBase( resolvedSecretTraceRegistry: ResolvedSecretTraceRegistry ): Promise<{ context: string[]; registry: ResolvedSecretTraceRegistry }> { const resultRegistry = resolvedSecretTraceRegistry.forkForInputPaths([]) + if (!workflowId) throw new Error('Hallucination validation requires a workflow ID') + try { const searchUrl = `${getInternalApiBaseUrl()}/api/knowledge/search` - const internalToken = await generateInternalToken(actorUserId) + const internalToken = await generateInternalDelegationToken({ + subjectUserId: actorUserId, + workflowId, + }) const headers = new Headers({ 'Content-Type': 'application/json', Authorization: `Bearer ${internalToken}`, @@ -122,10 +128,7 @@ async function queryKnowledgeBase( }) if (!response.ok) { - logger.error(`[${requestId}] Knowledge base query failed`, { - status: response.status, - }) - return { context: [], registry: resultRegistry } + throw new Error(`Knowledge base query failed with status ${response.status}`) } const payload: unknown = await response.json() @@ -167,12 +170,13 @@ async function queryKnowledgeBase( }), registry: resultRegistry, } - } catch (error: any) { + } catch (error) { if (error instanceof KnowledgeProvenanceError) throw error + const message = getErrorMessage(error, 'Unknown Knowledge query error') logger.error(`[${requestId}] Error querying knowledge base`, { - error: error.message, + error: message, }) - return { context: [], registry: resultRegistry } + throw new Error(`Failed to query knowledge base: ${message}`, { cause: error }) } } diff --git a/apps/sim/lib/knowledge/api/internal-route.ts b/apps/sim/lib/knowledge/api/internal-route.ts index b2a5e881b54..3b3f0227838 100644 --- a/apps/sim/lib/knowledge/api/internal-route.ts +++ b/apps/sim/lib/knowledge/api/internal-route.ts @@ -236,10 +236,10 @@ export const internalKnowledgeAnalytics = { result: | { kind: 'single' - workspaceId: string + workspaceId?: string data: { knowledgeBaseId: string; mimeType: string; fileSize: number } } - | { kind: 'bulk'; workspaceId: string; data: { total: number }; knowledgeBaseId?: string } + | { kind: 'bulk'; workspaceId?: string; data: { total: number }; knowledgeBaseId?: string } }): void { const userId = internalKnowledgeActorUserId(principal) const documentCount = result.kind === 'bulk' ? result.data.total : 1 @@ -261,12 +261,12 @@ export const internalKnowledgeAnalytics = { 'knowledge_base_document_uploaded', { knowledge_base_id: knowledgeBaseId, - workspace_id: result.workspaceId, + workspace_id: result.workspaceId ?? '', document_count: documentCount, upload_type: result.kind, }, { - groups: { workspace: result.workspaceId }, + ...(result.workspaceId ? { groups: { workspace: result.workspaceId } } : {}), setOnce: { first_document_uploaded_at: new Date().toISOString() }, } ) diff --git a/apps/sim/lib/knowledge/application/authorization.ts b/apps/sim/lib/knowledge/application/authorization.ts index 60320ea04d8..15f9dc9b762 100644 --- a/apps/sim/lib/knowledge/application/authorization.ts +++ b/apps/sim/lib/knowledge/application/authorization.ts @@ -6,7 +6,7 @@ import type { export const KNOWLEDGE_DELEGATION_AUDIENCE = 'sim:knowledge' -export interface KnowledgeAuthorizationContext extends WorkspaceAuthorizationContext { +interface KnowledgeResourceIdentifiers { knowledgeBaseId?: string documentId?: string chunkId?: string @@ -14,6 +14,19 @@ export interface KnowledgeAuthorizationContext extends WorkspaceAuthorizationCon connectorId?: string } +export interface KnowledgeAuthorizationContext + extends WorkspaceAuthorizationContext, + KnowledgeResourceIdentifiers {} + +export interface LegacyPersonalKnowledgeAuthorizationContext extends KnowledgeResourceIdentifiers { + workspaceId: undefined + legacyPersonalOwnerUserId: string +} + +export type KnowledgeResourceAuthorizationContext = + | KnowledgeAuthorizationContext + | LegacyPersonalKnowledgeAuthorizationContext + export type KnowledgeAuthorizationOptions = Omit< WorkspaceAuthorizationOptions, 'delegation' diff --git a/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts b/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts index 5a384ef3647..58dfa02e432 100644 --- a/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts +++ b/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts @@ -1,28 +1,146 @@ +import { requirePrincipalSubjectUserId } from '@sim/auth/principal' import { - type AuthorizedWorkspaceUseCaseDefinition, defineAuthorizedWorkspaceUseCase, + type OperationUseCase, + type PrincipalForOperation, + requireAllowedWorkspacePrincipal, type WorkspaceOperation, + type WorkspaceUseCaseAuditEntry, } from '@/lib/core/application' +import { + OrchestrationError, + type OrchestrationRequestContext, +} from '@/lib/core/orchestration/types' import { type KnowledgeAuthorizationContext, + type KnowledgeResourceAuthorizationContext, knowledgeDelegationPolicy, + type LegacyPersonalKnowledgeAuthorizationContext, } from '@/lib/knowledge/application/authorization' -type AuthorizedKnowledgeUseCaseDefinition< +interface AuthorizedKnowledgeUseCaseContext< + O extends WorkspaceOperation, + I, + C extends KnowledgeResourceAuthorizationContext, +> { + principal: PrincipalForOperation + input: I + context: C + request?: OrchestrationRequestContext +} + +interface AuthorizedKnowledgeUseCaseResultContext< + O extends WorkspaceOperation, + I, + C extends KnowledgeResourceAuthorizationContext, + R, +> extends AuthorizedKnowledgeUseCaseContext { + result: R +} + +interface AuthorizedKnowledgeUseCaseDefinition< O extends WorkspaceOperation, I, - C extends KnowledgeAuthorizationContext, + C extends KnowledgeResourceAuthorizationContext, R, -> = Omit, 'authorizationOptions'> +> { + operation: O + resolveContext(args: { principal: PrincipalForOperation; input: I }): C | Promise + execute(args: AuthorizedKnowledgeUseCaseContext): Promise + projectAudit?( + args: AuthorizedKnowledgeUseCaseResultContext + ): WorkspaceUseCaseAuditEntry | WorkspaceUseCaseAuditEntry[] + afterSuccess?(args: AuthorizedKnowledgeUseCaseResultContext): void | Promise +} + +function isLegacyPersonalKnowledgeContext( + context: KnowledgeResourceAuthorizationContext +): context is LegacyPersonalKnowledgeAuthorizationContext { + return context.workspaceId === undefined +} + +function assertWorkspaceKnowledgeContext( + context: C +): asserts context is C & KnowledgeAuthorizationContext { + if (isLegacyPersonalKnowledgeContext(context)) { + throw new Error('Expected a workspace-scoped Knowledge authorization context') + } +} export function defineAuthorizedKnowledgeUseCase< const O extends WorkspaceOperation, I, - C extends KnowledgeAuthorizationContext, + C extends KnowledgeResourceAuthorizationContext, R, ->(definition: AuthorizedKnowledgeUseCaseDefinition) { - return defineAuthorizedWorkspaceUseCase({ - ...definition, - authorizationOptions: { delegation: knowledgeDelegationPolicy }, - }) +>(definition: AuthorizedKnowledgeUseCaseDefinition): OperationUseCase { + type WorkspaceContext = C & KnowledgeAuthorizationContext + type WorkspaceInput = { originalInput: I; context: WorkspaceContext } + const projectAudit = definition.projectAudit + const afterSuccess = definition.afterSuccess + + const workspaceUseCase = defineAuthorizedWorkspaceUseCase( + { + operation: definition.operation, + resolveContext: ({ input }: { input: WorkspaceInput }) => input.context, + authorizationOptions: { delegation: knowledgeDelegationPolicy }, + execute: ({ principal, input, context, request }) => + definition.execute({ + principal, + input: input.originalInput, + context, + request, + }), + ...(projectAudit + ? { + projectAudit: ({ principal, input, context, request, result }) => + projectAudit({ + principal, + input: input.originalInput, + context, + request, + result, + }), + } + : {}), + ...(afterSuccess + ? { + afterSuccess: ({ principal, input, context, request, result }) => + afterSuccess({ + principal, + input: input.originalInput, + context, + request, + result, + }), + } + : {}), + } + ) + + return { + operation: definition.operation, + async execute({ principal, input, request }) { + requireAllowedWorkspacePrincipal(principal, definition.operation) + const context = await definition.resolveContext({ principal, input }) + if (isLegacyPersonalKnowledgeContext(context)) { + if ( + principal.kind === 'workspace_api_key' || + requirePrincipalSubjectUserId(principal) !== context.legacyPersonalOwnerUserId + ) { + throw new OrchestrationError('not_found', 'Knowledge base not found') + } + const executionContext = { principal, input, context, request } + const result = await definition.execute(executionContext) + await definition.afterSuccess?.({ ...executionContext, result }) + return result + } + + assertWorkspaceKnowledgeContext(context) + return workspaceUseCase.execute({ + principal, + input: { originalInput: input, context }, + request, + }) + }, + } } diff --git a/apps/sim/lib/knowledge/application/billing.ts b/apps/sim/lib/knowledge/application/billing.ts index f2a08cd4f63..e7ca3425751 100644 --- a/apps/sim/lib/knowledge/application/billing.ts +++ b/apps/sim/lib/knowledge/application/billing.ts @@ -1,11 +1,13 @@ import type { Principal } from '@sim/auth/principal' -import { resolvePrincipalAttribution } from '@sim/auth/principal' +import { requirePrincipalSubjectUserId, resolvePrincipalAttribution } from '@sim/auth/principal' +import { checkActorUsageLimits } from '@/lib/billing/calculations/usage-monitor' import { type BillingAttributionSnapshot, + checkAttributedUsageLimits, resolveBillingAttribution, resolveSystemBillingAttribution, } from '@/lib/billing/core/billing-attribution' -import type { KnowledgeWorkspaceContext } from '@/lib/knowledge/application/contexts' +import type { KnowledgeResourceContext } from '@/lib/knowledge/application/contexts' export class KnowledgeUsageLimitExceededError extends Error { constructor(message: string) { @@ -16,8 +18,9 @@ export class KnowledgeUsageLimitExceededError extends Error { export function resolveKnowledgeAttributedUserId( principal: Principal, - context: KnowledgeWorkspaceContext + context: KnowledgeResourceContext ): string { + if (context.workspaceId === undefined) return requirePrincipalSubjectUserId(principal) return resolvePrincipalAttribution(principal, { workspaceBillingOwnerUserId: context.billedAccountUserId, }).attributedUserId @@ -25,8 +28,11 @@ export function resolveKnowledgeAttributedUserId( export function resolveKnowledgeBillingAttribution( principal: Principal, - context: KnowledgeWorkspaceContext + context: KnowledgeResourceContext ): Promise { + if (context.workspaceId === undefined) { + throw new Error('Legacy personal knowledge bases do not have workspace billing attribution') + } if (principal.kind === 'workspace_api_key') { return resolveSystemBillingAttribution(context.workspaceId) } @@ -35,3 +41,20 @@ export function resolveKnowledgeBillingAttribution( workspaceId: context.workspaceId, }) } + +export async function resolveKnowledgeUsageAdmission( + principal: Principal, + context: KnowledgeResourceContext, + resolveAttribution?: (workspaceId: string) => Promise +) { + const userId = resolveKnowledgeAttributedUserId(principal, context) + const billingAttribution = context.workspaceId + ? resolveAttribution + ? await resolveAttribution(context.workspaceId) + : await resolveKnowledgeBillingAttribution(principal, context) + : undefined + const usage = billingAttribution + ? await checkAttributedUsageLimits(billingAttribution) + : await checkActorUsageLimits(userId) + return { billingAttribution, usage, userId } +} diff --git a/apps/sim/lib/knowledge/application/chunks.ts b/apps/sim/lib/knowledge/application/chunks.ts index 38d4d56dcca..519ac5b1198 100644 --- a/apps/sim/lib/knowledge/application/chunks.ts +++ b/apps/sim/lib/knowledge/application/chunks.ts @@ -38,7 +38,7 @@ interface KnowledgeChunkInput extends KnowledgeDocumentChunkInput { interface ResolveChunkProvenanceInput { userId: string - workspaceId: string + workspaceId?: string } export interface ListKnowledgeChunksInput extends KnowledgeDocumentChunkInput, ChunkFilters {} @@ -149,7 +149,7 @@ export const createKnowledgeChunk = defineAuthorizedKnowledgeUseCase({ const registry = provenance ? await createDurableSecretProvenanceRegistry(provenance, { userId, - workspaceId: context.workspaceId, + ...(context.workspaceId ? { workspaceId: context.workspaceId } : {}), }) : undefined const chunk = await runWithKnowledgeModelInputProvenance(registry, () => @@ -209,7 +209,7 @@ export const updateKnowledgeChunk = defineAuthorizedKnowledgeUseCase({ const registry = provenance ? await createDurableSecretProvenanceRegistry(provenance, { userId, - workspaceId: context.workspaceId, + ...(context.workspaceId ? { workspaceId: context.workspaceId } : {}), }) : undefined const chunk = await runWithKnowledgeModelInputProvenance(registry, () => diff --git a/apps/sim/lib/knowledge/application/connectors.test.ts b/apps/sim/lib/knowledge/application/connectors.test.ts index 816565d737e..a354c10a6d7 100644 --- a/apps/sim/lib/knowledge/application/connectors.test.ts +++ b/apps/sim/lib/knowledge/application/connectors.test.ts @@ -44,6 +44,7 @@ vi.mock('@sim/platform-authz/workspace', () => ({ vi.mock('@/lib/knowledge/application/contexts', () => ({ resolveActiveKnowledgeBaseContext: mocks.resolveKnowledgeBase, + resolveActiveKnowledgeResourceContext: mocks.resolveKnowledgeBase, resolveActiveKnowledgeConnectorContext: mocks.resolveConnector, })) diff --git a/apps/sim/lib/knowledge/application/connectors.ts b/apps/sim/lib/knowledge/application/connectors.ts index 15e9e24b4c7..9832dd1b36e 100644 --- a/apps/sim/lib/knowledge/application/connectors.ts +++ b/apps/sim/lib/knowledge/application/connectors.ts @@ -10,9 +10,9 @@ import { resolveCredentialTokenIdentity } from '@/lib/credentials/access' import { defineAuthorizedKnowledgeUseCase } from '@/lib/knowledge/application/authorized-knowledge-use-case' import { resolveKnowledgeAttributedUserId } from '@/lib/knowledge/application/billing' import { - type ActiveKnowledgeBaseContext, - resolveActiveKnowledgeBaseContext, + type ActiveKnowledgeResourceBaseContext, resolveActiveKnowledgeConnectorContext, + resolveActiveKnowledgeResourceContext, } from '@/lib/knowledge/application/contexts' import { knowledgeOperations } from '@/lib/knowledge/application/operations' import { @@ -101,14 +101,21 @@ function requireSuccessfulOutcome( throw new OrchestrationError(outcome.errorCode, outcome.error) } -function connectorTarget(context: ActiveKnowledgeBaseContext) { +function connectorTarget(context: ActiveKnowledgeResourceBaseContext) { return { id: context.knowledgeBaseId, name: context.knowledgeBase.name, - workspaceId: context.workspaceId, + workspaceId: context.workspaceId ?? null, } } +function requireConnectorWorkspaceId(context: ActiveKnowledgeResourceBaseContext): string { + if (!context.workspaceId) { + throw new OrchestrationError('conflict', 'Knowledge base is missing workspace billing context') + } + return context.workspaceId +} + async function resolveConnectorCredentialAccessToken(input: { credentialId: string workspaceId: string @@ -188,7 +195,7 @@ async function validateConnectorSourceConfig(input: { export const listKnowledgeConnectors = defineAuthorizedKnowledgeUseCase({ operation: knowledgeOperations.listConnectors, resolveContext: ({ input }: { input: ListKnowledgeConnectorsInput }) => - resolveActiveKnowledgeBaseContext(input), + resolveActiveKnowledgeResourceContext(input), async execute({ context }) { const connectors = await db .select() @@ -226,9 +233,10 @@ export const readKnowledgeConnector = defineAuthorizedKnowledgeUseCase({ export const createKnowledgeConnector = defineAuthorizedKnowledgeUseCase({ operation: knowledgeOperations.createConnector, resolveContext: ({ input }: { input: CreateKnowledgeConnectorInput }) => - resolveActiveKnowledgeBaseContext(input), + resolveActiveKnowledgeResourceContext(input), async execute({ principal, input, context, request }) { const requestId = generateRequestId() + const workspaceId = requireConnectorWorkspaceId(context) const actingUserId = resolveKnowledgeAttributedUserId(principal, context) const outcome = await performCreateKnowledgeConnector({ knowledgeBase: connectorTarget(context), @@ -237,11 +245,11 @@ export const createKnowledgeConnector = defineAuthorizedKnowledgeUseCase({ apiKey: input.apiKey, sourceConfig: input.sourceConfig, syncIntervalMinutes: input.syncIntervalMinutes, - resolveBillingAttribution: () => input.resolveBillingAttribution(context.workspaceId), + resolveBillingAttribution: () => input.resolveBillingAttribution(workspaceId), resolveAccessToken: (credentialId) => resolveConnectorCredentialAccessToken({ credentialId, - workspaceId: context.workspaceId, + workspaceId, actingUserId, requestId, }), @@ -253,7 +261,7 @@ export const createKnowledgeConnector = defineAuthorizedKnowledgeUseCase({ recordProductAnalytics: false, }) requireSuccessfulOutcome(outcome, 'Knowledge connector creation failed') - return { connector: outcome.connector, workspaceId: context.workspaceId } + return { connector: outcome.connector, workspaceId } }, projectAudit: ({ input, context, result }) => ({ action: AuditAction.CONNECTOR_CREATED, @@ -283,14 +291,16 @@ export const updateKnowledgeConnector = defineAuthorizedKnowledgeUseCase({ knowledgeBase: connectorTarget(context), connectorId: context.connectorId, updates: input.updates, - validateSourceConfig: (connector, sourceConfig) => - validateConnectorSourceConfig({ + validateSourceConfig: (connector, sourceConfig) => { + const workspaceId = requireConnectorWorkspaceId(context) + return validateConnectorSourceConfig({ connector, sourceConfig, - workspaceId: context.workspaceId, + workspaceId, actingUserId, requestId, - }), + }) + }, userId: actingUserId, source: input.source ?? 'agent', requestId, @@ -371,10 +381,11 @@ export const syncKnowledgeConnector = defineAuthorizedKnowledgeUseCase({ resolveContext: ({ input }: { input: SyncKnowledgeConnectorInput }) => resolveActiveKnowledgeConnectorContext(input), async execute({ principal, input, context, request }) { + const workspaceId = requireConnectorWorkspaceId(context) const outcome = await performSyncKnowledgeConnector({ knowledgeBase: connectorTarget(context), connectorId: context.connectorId, - resolveBillingAttribution: () => input.resolveBillingAttribution(context.workspaceId), + resolveBillingAttribution: () => input.resolveBillingAttribution(workspaceId), rehydrate: input.rehydrate, userId: resolveKnowledgeAttributedUserId(principal, context), source: input.source ?? 'agent', @@ -386,7 +397,7 @@ export const syncKnowledgeConnector = defineAuthorizedKnowledgeUseCase({ requireSuccessfulOutcome(outcome, 'Knowledge connector sync failed') return { knowledgeBaseId: context.knowledgeBaseId, - workspaceId: context.workspaceId, + workspaceId, connectorId: context.connectorId, connectorType: context.connector.connectorType, } diff --git a/apps/sim/lib/knowledge/application/contexts.test.ts b/apps/sim/lib/knowledge/application/contexts.test.ts index 0e04796c5b7..7e3a30440fd 100644 --- a/apps/sim/lib/knowledge/application/contexts.test.ts +++ b/apps/sim/lib/knowledge/application/contexts.test.ts @@ -40,6 +40,7 @@ import { loadKnowledgeWorkspaceAuthorizationContext, resolveActiveKnowledgeBaseContext, resolveActiveKnowledgeConnectorContext, + resolveActiveKnowledgeResourceContext, resolveActiveKnowledgeTagContext, resolveCanonicalActiveKnowledgeDocumentContext, resolveKnowledgeWorkspaceContext, @@ -94,6 +95,62 @@ describe('knowledge application contexts', () => { ).rejects.toBe(failure) }) + it('resolves a legacy personal knowledge base only through the resource context', async () => { + mocks.getKnowledgeBase.mockResolvedValueOnce({ + id: 'legacy-knowledge', + userId: 'owner-1', + workspaceId: null, + }) + + await expect( + resolveActiveKnowledgeResourceContext({ knowledgeBaseId: 'legacy-knowledge' }) + ).resolves.toMatchObject({ + knowledgeBaseId: 'legacy-knowledge', + workspaceId: undefined, + legacyPersonalOwnerUserId: 'owner-1', + }) + expect(mocks.loadWorkspace).not.toHaveBeenCalled() + }) + + it('does not let a workspace assertion address a legacy personal knowledge base', async () => { + mocks.getKnowledgeBase.mockResolvedValueOnce({ + id: 'legacy-knowledge', + userId: 'owner-1', + workspaceId: null, + }) + + await expect( + resolveActiveKnowledgeResourceContext({ + knowledgeBaseId: 'legacy-knowledge', + assertedWorkspaceId: 'workspace-1', + }) + ).rejects.toMatchObject({ code: 'not_found' }) + }) + + it('keeps legacy personal ownership on canonical child contexts', async () => { + mocks.getDocumentById.mockResolvedValueOnce({ + id: 'legacy-document', + knowledgeBaseId: 'legacy-knowledge', + }) + mocks.getKnowledgeBase.mockResolvedValueOnce({ + id: 'legacy-knowledge', + userId: 'owner-1', + workspaceId: null, + }) + + await expect( + resolveCanonicalActiveKnowledgeDocumentContext({ + knowledgeBaseId: 'legacy-knowledge', + documentId: 'legacy-document', + }) + ).resolves.toMatchObject({ + documentId: 'legacy-document', + knowledgeBaseId: 'legacy-knowledge', + workspaceId: undefined, + legacyPersonalOwnerUserId: 'owner-1', + }) + }) + describe('canonical child resources', () => { beforeEach(() => { mocks.getKnowledgeBase.mockResolvedValue({ diff --git a/apps/sim/lib/knowledge/application/contexts.ts b/apps/sim/lib/knowledge/application/contexts.ts index b81227640a2..cf7ea274243 100644 --- a/apps/sim/lib/knowledge/application/contexts.ts +++ b/apps/sim/lib/knowledge/application/contexts.ts @@ -2,7 +2,10 @@ import { db } from '@sim/db' import { embedding } from '@sim/db/schema' import { and, eq } from 'drizzle-orm' import { OrchestrationError } from '@/lib/core/orchestration/types' -import type { KnowledgeAuthorizationContext } from '@/lib/knowledge/application/authorization' +import type { + KnowledgeAuthorizationContext, + LegacyPersonalKnowledgeAuthorizationContext, +} from '@/lib/knowledge/application/authorization' import type { ChunkData } from '@/lib/knowledge/chunks/types' import { type ActiveKnowledgeConnectorReference, @@ -23,27 +26,37 @@ export interface KnowledgeWorkspaceContext extends KnowledgeAuthorizationContext billedAccountUserId: string } +export interface LegacyPersonalKnowledgeContext + extends LegacyPersonalKnowledgeAuthorizationContext {} + +export type KnowledgeResourceContext = KnowledgeWorkspaceContext | LegacyPersonalKnowledgeContext + export interface ActiveKnowledgeBaseContext extends KnowledgeWorkspaceContext { knowledgeBaseId: string knowledgeBase: KnowledgeBaseWithCounts } -export interface ActiveKnowledgeDocumentContext extends ActiveKnowledgeBaseContext { +export type ActiveKnowledgeResourceBaseContext = KnowledgeResourceContext & { + knowledgeBaseId: string + knowledgeBase: KnowledgeBaseWithCounts +} + +export type ActiveKnowledgeDocumentContext = ActiveKnowledgeResourceBaseContext & { documentId: string document: ActiveKnowledgeDocument } -export interface ActiveKnowledgeTagContext extends ActiveKnowledgeBaseContext { +export type ActiveKnowledgeTagContext = ActiveKnowledgeResourceBaseContext & { tagDefinitionId: string tagDefinition: DocumentTagDefinition } -export interface ActiveKnowledgeConnectorContext extends ActiveKnowledgeBaseContext { +export type ActiveKnowledgeConnectorContext = ActiveKnowledgeResourceBaseContext & { connectorId: string connector: ActiveKnowledgeConnectorReference } -export interface ActiveKnowledgeChunkContext extends ActiveKnowledgeDocumentContext { +export type ActiveKnowledgeChunkContext = ActiveKnowledgeDocumentContext & { chunkId: string chunk: ChunkData } @@ -90,12 +103,41 @@ export async function resolveActiveKnowledgeBaseContext(input: { } } +export async function resolveActiveKnowledgeResourceContext(input: { + knowledgeBaseId: string + assertedWorkspaceId?: string +}): Promise { + const knowledgeBase = await getKnowledgeBaseById(input.knowledgeBaseId) + if ( + !knowledgeBase || + (input.assertedWorkspaceId !== undefined && + knowledgeBase.workspaceId !== input.assertedWorkspaceId) + ) { + throw new OrchestrationError('not_found', 'Knowledge base not found') + } + if (!knowledgeBase.workspaceId) { + return { + workspaceId: undefined, + legacyPersonalOwnerUserId: knowledgeBase.userId, + knowledgeBaseId: knowledgeBase.id, + knowledgeBase, + } + } + const workspaceContext = await loadKnowledgeWorkspaceContext(knowledgeBase.workspaceId) + if (!workspaceContext) throw new OrchestrationError('not_found', 'Knowledge base not found') + return { + ...workspaceContext, + knowledgeBaseId: knowledgeBase.id, + knowledgeBase, + } +} + export async function resolveActiveKnowledgeDocumentContext(input: { knowledgeBaseId: string documentId: string assertedWorkspaceId?: string }): Promise { - const context = await resolveActiveKnowledgeBaseContext(input) + const context = await resolveActiveKnowledgeResourceContext(input) const document = await getKnowledgeDocument(context.knowledgeBaseId, input.documentId) if (!document) throw new OrchestrationError('not_found', 'Document not found') return { @@ -114,7 +156,7 @@ export async function resolveCanonicalActiveKnowledgeDocumentContext(input: { if (!document || document.knowledgeBaseId !== input.knowledgeBaseId) { throw new OrchestrationError('not_found', 'Document not found') } - const context = await resolveActiveKnowledgeBaseContext({ + const context = await resolveActiveKnowledgeResourceContext({ knowledgeBaseId: document.knowledgeBaseId, assertedWorkspaceId: input.assertedWorkspaceId, }) @@ -159,7 +201,7 @@ export async function resolveActiveKnowledgeTagContext(input: { ) { throw new OrchestrationError('not_found', 'Tag definition not found') } - const context = await resolveActiveKnowledgeBaseContext({ + const context = await resolveActiveKnowledgeResourceContext({ knowledgeBaseId: tagDefinition.knowledgeBaseId, assertedWorkspaceId: input.assertedWorkspaceId, }) @@ -182,7 +224,7 @@ export async function resolveActiveKnowledgeConnectorContext(input: { ) { throw new OrchestrationError('not_found', 'Connector not found') } - const context = await resolveActiveKnowledgeBaseContext({ + const context = await resolveActiveKnowledgeResourceContext({ knowledgeBaseId: connector.knowledgeBaseId, assertedWorkspaceId: input.assertedWorkspaceId, }) diff --git a/apps/sim/lib/knowledge/application/documents.test.ts b/apps/sim/lib/knowledge/application/documents.test.ts index 20806987c63..e5cb922c47f 100644 --- a/apps/sim/lib/knowledge/application/documents.test.ts +++ b/apps/sim/lib/knowledge/application/documents.test.ts @@ -54,6 +54,7 @@ vi.mock('@/lib/billing/core/billing-attribution', () => ({ vi.mock('@/lib/knowledge/application/contexts', () => ({ resolveActiveKnowledgeBaseContext: mocks.resolveKnowledgeBase, + resolveActiveKnowledgeResourceContext: mocks.resolveKnowledgeBase, resolveActiveKnowledgeDocumentContext: mocks.resolveDocument, resolveCanonicalActiveKnowledgeDocumentContext: mocks.resolveCanonicalDocument, })) @@ -170,6 +171,48 @@ describe('knowledge document application use cases', () => { ) }) + it('lets the owner list documents in a legacy personal knowledge base', async () => { + mocks.resolveKnowledgeBase.mockResolvedValueOnce({ + workspaceId: undefined, + legacyPersonalOwnerUserId: 'user-1', + knowledgeBaseId: 'legacy-knowledge', + knowledgeBase: { id: 'legacy-knowledge', name: 'Personal docs', userId: 'user-1' }, + }) + + await expect( + listKnowledgeDocuments.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { knowledgeBaseId: 'legacy-knowledge' }, + }) + ).resolves.toMatchObject({ workspaceId: undefined }) + + expect(mocks.resolvePermission).not.toHaveBeenCalled() + expect(mocks.getDocuments).toHaveBeenCalledWith( + 'legacy-knowledge', + expect.any(Object), + expect.any(String) + ) + expect(mocks.recordAudit).not.toHaveBeenCalled() + }) + + it('conceals legacy personal documents from a non-owner', async () => { + mocks.resolveKnowledgeBase.mockResolvedValueOnce({ + workspaceId: undefined, + legacyPersonalOwnerUserId: 'user-1', + knowledgeBaseId: 'legacy-knowledge', + knowledgeBase: { id: 'legacy-knowledge', name: 'Personal docs', userId: 'user-1' }, + }) + + await expect( + listKnowledgeDocuments.execute({ + principal: { kind: 'session', userId: 'other-user', sessionId: 'session-2' }, + input: { knowledgeBaseId: 'legacy-knowledge' }, + }) + ).rejects.toMatchObject({ code: 'not_found' }) + + expect(mocks.getDocuments).not.toHaveBeenCalled() + }) + it('resolves current workspace-key billing while retaining key audit attribution', async () => { await uploadKnowledgeDocument.execute({ principal: { diff --git a/apps/sim/lib/knowledge/application/documents.ts b/apps/sim/lib/knowledge/application/documents.ts index 70a9f8af830..9be50d5c4d6 100644 --- a/apps/sim/lib/knowledge/application/documents.ts +++ b/apps/sim/lib/knowledge/application/documents.ts @@ -3,7 +3,10 @@ import { db } from '@sim/db' import { document as documentTable } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq, isNull } from 'drizzle-orm' -import { checkAttributedUsageLimits } from '@/lib/billing/core/billing-attribution' +import { + type BillingAttributionSnapshot, + checkAttributedUsageLimits, +} from '@/lib/billing/core/billing-attribution' import { authorizeWorkspaceOperation } from '@/lib/core/application' import { asOrchestrationError, OrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' @@ -19,12 +22,14 @@ import { KnowledgeUsageLimitExceededError, resolveKnowledgeAttributedUserId, resolveKnowledgeBillingAttribution, + resolveKnowledgeUsageAdmission, } from '@/lib/knowledge/application/billing' import { - type ActiveKnowledgeBaseContext, type ActiveKnowledgeDocumentContext, + type ActiveKnowledgeResourceBaseContext, resolveActiveKnowledgeBaseContext, resolveActiveKnowledgeDocumentContext, + resolveActiveKnowledgeResourceContext, resolveCanonicalActiveKnowledgeDocumentContext, } from '@/lib/knowledge/application/contexts' import { knowledgeOperations } from '@/lib/knowledge/application/operations' @@ -109,12 +114,10 @@ export interface CreateKnowledgeDocumentsInput extends UploadKnowledgeDocumentAd bulk: boolean processingOptions?: ProcessingOptions source?: 'ui' | 'api' | 'agent' - resolveBillingAttribution?( - workspaceId: string - ): Promise>> + resolveBillingAttribution?(workspaceId: string): Promise resolveSecretProvenances(input: { userId: string - workspaceId: string + workspaceId?: string }): KnowledgeDocumentWriteSecretProvenance[] | undefined } @@ -147,7 +150,7 @@ interface BulkDeleteKnowledgeDocumentsExecutionResult extends BulkDeleteKnowledgeDocumentsResult, KnowledgeBatchExecutionResult {} -interface BulkDeleteKnowledgeDocumentsContext extends ActiveKnowledgeBaseContext { +type BulkDeleteKnowledgeDocumentsContext = ActiveKnowledgeResourceBaseContext & { documentIds: string[] } @@ -157,9 +160,7 @@ export interface UpdateKnowledgeDocumentInput extends ReadKnowledgeDocumentInput updates?: Parameters[1] markFailedDueToTimeout?: boolean retryProcessing?: boolean - resolveBillingAttribution?( - workspaceId: string - ): Promise>> + resolveBillingAttribution?(workspaceId: string): Promise source?: string } @@ -178,19 +179,17 @@ export interface UpsertKnowledgeDocumentInput extends UploadKnowledgeDocumentAdm mimeType: string documentTagsData?: string processingOptions?: ProcessingOptions - resolveBillingAttribution( - workspaceId: string - ): Promise>> + resolveBillingAttribution(workspaceId: string): Promise resolveSecretProvenances(input: { userId: string - workspaceId: string + workspaceId?: string }): KnowledgeDocumentWriteSecretProvenance[] | undefined } export const listKnowledgeDocuments = defineAuthorizedKnowledgeUseCase({ operation: knowledgeOperations.listDocuments, resolveContext: ({ input }: { input: ListKnowledgeDocumentsInput }) => - resolveActiveKnowledgeBaseContext(input), + resolveActiveKnowledgeResourceContext(input), async execute({ input, context }) { const limit = input.limit ?? 50 const offset = input.offset ?? 0 @@ -324,7 +323,7 @@ export const uploadKnowledgeDocument = defineAuthorizedKnowledgeUseCase({ export const createKnowledgeDocuments = defineAuthorizedKnowledgeUseCase({ operation: knowledgeOperations.uploadDocument, resolveContext: ({ input }: { input: CreateKnowledgeDocumentsInput }) => - resolveActiveKnowledgeBaseContext(input), + resolveActiveKnowledgeResourceContext(input), async execute({ principal, input, context, request }) { if (input.documents.length === 0) { throw new OrchestrationError('validation', 'No documents specified') @@ -335,16 +334,16 @@ export const createKnowledgeDocuments = defineAuthorizedKnowledgeUseCase({ `At most ${MAX_KNOWLEDGE_DOCUMENTS_PER_CREATE} documents may be created at once` ) } - const billingAttribution = input.resolveBillingAttribution - ? await input.resolveBillingAttribution(context.workspaceId) - : await resolveKnowledgeBillingAttribution(principal, context) - const usage = await checkAttributedUsageLimits(billingAttribution) + const { billingAttribution, usage, userId } = await resolveKnowledgeUsageAdmission( + principal, + context, + input.resolveBillingAttribution + ) if (usage.isExceeded) { throw new KnowledgeUsageLimitExceededError( usage.message || 'Usage limit exceeded. Please upgrade your plan to continue.' ) } - const userId = resolveKnowledgeAttributedUserId(principal, context) const secretProvenances = input.resolveSecretProvenances({ userId, workspaceId: context.workspaceId, @@ -352,7 +351,7 @@ export const createKnowledgeDocuments = defineAuthorizedKnowledgeUseCase({ const knowledgeBase = { id: context.knowledgeBaseId, name: context.knowledgeBase.name, - workspaceId: context.workspaceId, + workspaceId: context.workspaceId ?? null, } if (input.bulk) { const outcome = await performUploadKnowledgeDocuments({ @@ -459,16 +458,18 @@ export const createKnowledgeDocuments = defineAuthorizedKnowledgeUseCase({ export const upsertKnowledgeDocument = defineAuthorizedKnowledgeUseCase({ operation: knowledgeOperations.uploadDocument, resolveContext: ({ input }: { input: UpsertKnowledgeDocumentInput }) => - resolveActiveKnowledgeBaseContext(input), + resolveActiveKnowledgeResourceContext(input), async execute({ principal, input, context }) { - const billingAttribution = await input.resolveBillingAttribution(context.workspaceId) - const usage = await checkAttributedUsageLimits(billingAttribution) + const { billingAttribution, usage, userId } = await resolveKnowledgeUsageAdmission( + principal, + context, + input.resolveBillingAttribution + ) if (usage.isExceeded) { throw new KnowledgeUsageLimitExceededError( usage.message || 'Usage limit exceeded. Please upgrade your plan to continue.' ) } - const userId = resolveKnowledgeAttributedUserId(principal, context) const secretProvenances = input.resolveSecretProvenances({ userId, workspaceId: context.workspaceId, @@ -629,7 +630,7 @@ export const bulkDeleteKnowledgeDocuments = defineAuthorizedKnowledgeUseCase({ BULK_DELETE_KNOWLEDGE_DOCUMENTS_COST_POLICY.maxItems ) return { - ...(await resolveActiveKnowledgeBaseContext(input)), + ...(await resolveActiveKnowledgeResourceContext(input)), documentIds, } }, @@ -650,12 +651,14 @@ export const bulkDeleteKnowledgeDocuments = defineAuthorizedKnowledgeUseCase({ documentId, assertedWorkspaceId: context.workspaceId, }) - await authorizeWorkspaceOperation( - principal, - knowledgeOperations.bulkDeleteDocuments, - canonical, - { delegation: knowledgeDelegationPolicy } - ) + if (canonical.workspaceId) { + await authorizeWorkspaceOperation( + principal, + knowledgeOperations.bulkDeleteDocuments, + canonical, + { delegation: knowledgeDelegationPolicy } + ) + } if (input.cancellationSignal?.aborted) break await deleteKnowledgeDocumentInKnowledgeBase( canonical.knowledgeBaseId, @@ -718,9 +721,13 @@ export const updateKnowledgeDocument = defineAuthorizedKnowledgeUseCase({ : await performRetryKnowledgeDocumentProcessing({ knowledgeBaseId: context.knowledgeBaseId, document: context.document, - billingAttribution: input.resolveBillingAttribution - ? await input.resolveBillingAttribution(context.workspaceId) - : await resolveKnowledgeBillingAttribution(principal, context), + billingAttribution: ( + await resolveKnowledgeUsageAdmission( + principal, + context, + input.resolveBillingAttribution + ) + ).billingAttribution, }) if (!outcome.success) { if (outcome.errorCode === 'internal') { @@ -771,7 +778,7 @@ export const updateKnowledgeDocument = defineAuthorizedKnowledgeUseCase({ export const bulkUpdateKnowledgeDocuments = defineAuthorizedKnowledgeUseCase({ operation: knowledgeOperations.bulkDocuments, resolveContext: ({ input }: { input: BulkKnowledgeDocumentsInput }) => - resolveActiveKnowledgeBaseContext(input), + resolveActiveKnowledgeResourceContext(input), async execute({ input, context }) { const result = input.selectAll ? await bulkDocumentOperationByFilter( diff --git a/apps/sim/lib/knowledge/application/search.test.ts b/apps/sim/lib/knowledge/application/search.test.ts index 90b6e7fb32a..e089eb98761 100644 --- a/apps/sim/lib/knowledge/application/search.test.ts +++ b/apps/sim/lib/knowledge/application/search.test.ts @@ -10,6 +10,7 @@ const mocks = vi.hoisted(() => ({ getKnowledgeBase: vi.fn(), resolveBilling: vi.fn(), checkUsage: vi.fn(), + checkActorUsage: vi.fn(), generateEmbedding: vi.fn(), executeSearch: vi.fn(), getDocumentMetadata: vi.fn(), @@ -34,6 +35,10 @@ vi.mock('@/lib/billing/core/billing-attribution', () => ({ checkAttributedUsageLimits: mocks.checkUsage, })) +vi.mock('@/lib/billing/calculations/usage-monitor', () => ({ + checkActorUsageLimits: mocks.checkActorUsage, +})) + vi.mock('@/lib/knowledge/application/contexts', () => ({ resolveKnowledgeWorkspaceContext: mocks.resolveWorkspace, })) @@ -77,6 +82,7 @@ const workspace = { const knowledgeBase = { id: 'knowledge-1', + userId: 'user-1', name: 'Docs', workspaceId: 'workspace-1', embeddingModel: 'text-embedding-3-small', @@ -93,6 +99,7 @@ describe('knowledge search application use case', () => { workspaceId: 'workspace-1', }) mocks.checkUsage.mockResolvedValue({ isExceeded: false }) + mocks.checkActorUsage.mockResolvedValue({ isExceeded: false }) mocks.generateEmbedding.mockResolvedValue({ embedding: [0.1], isBYOK: false }) mocks.executeSearch.mockResolvedValue([ { @@ -184,6 +191,51 @@ describe('knowledge search application use case', () => { expect(mocks.executeSearch).not.toHaveBeenCalled() }) + it('lets the owner search a legacy personal knowledge base with account billing', async () => { + mocks.getKnowledgeBase.mockResolvedValueOnce({ + ...knowledgeBase, + workspaceId: null, + }) + mocks.generateEmbedding.mockResolvedValueOnce({ embedding: [0.1], isBYOK: true }) + + const result = await searchKnowledge.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { + knowledgeBaseIds: ['knowledge-1'], + query: 'answer', + topK: 5, + }, + }) + + expect(result.workspaceId).toBeUndefined() + expect(mocks.resolveWorkspace).not.toHaveBeenCalled() + expect(mocks.resolvePermission).not.toHaveBeenCalled() + expect(mocks.resolveBilling).not.toHaveBeenCalled() + expect(mocks.checkActorUsage).toHaveBeenCalledWith('user-1') + expect(mocks.executeSearch).toHaveBeenCalled() + }) + + it('conceals a legacy personal knowledge base from a non-owner', async () => { + mocks.getKnowledgeBase.mockResolvedValueOnce({ + ...knowledgeBase, + workspaceId: null, + }) + + await expect( + searchKnowledge.execute({ + principal: { kind: 'session', userId: 'other-user', sessionId: 'session-2' }, + input: { + knowledgeBaseIds: ['knowledge-1'], + query: 'answer', + topK: 5, + }, + }) + ).rejects.toMatchObject({ code: 'not_found' }) + + expect(mocks.checkActorUsage).not.toHaveBeenCalled() + expect(mocks.executeSearch).not.toHaveBeenCalled() + }) + it('enforces semantic knowledge-base and result bounds for trusted callers', async () => { await expect( searchKnowledge.execute({ diff --git a/apps/sim/lib/knowledge/application/search.ts b/apps/sim/lib/knowledge/application/search.ts index 40411b6db55..50f863087d2 100644 --- a/apps/sim/lib/knowledge/application/search.ts +++ b/apps/sim/lib/knowledge/application/search.ts @@ -1,12 +1,16 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' +import { checkActorUsageLimits } from '@/lib/billing/calculations/usage-monitor' import { type BillingAttributionSnapshot, checkAttributedUsageLimits, toBillingContext, } from '@/lib/billing/core/billing-attribution' import { recordUsage } from '@/lib/billing/core/usage-log' -import { checkAndBillPayerOverageThreshold } from '@/lib/billing/threshold-billing' +import { + checkAndBillOverageThreshold, + checkAndBillPayerOverageThreshold, +} from '@/lib/billing/threshold-billing' import { OrchestrationError } from '@/lib/core/orchestration/types' import { PlatformEvents } from '@/lib/core/telemetry' import { generateRequestId } from '@/lib/core/utils/request' @@ -18,7 +22,7 @@ import { resolveKnowledgeBillingAttribution, } from '@/lib/knowledge/application/billing' import { - type KnowledgeWorkspaceContext, + type KnowledgeResourceContext, resolveKnowledgeWorkspaceContext, } from '@/lib/knowledge/application/contexts' import { knowledgeOperations } from '@/lib/knowledge/application/operations' @@ -82,13 +86,13 @@ export interface SearchKnowledgeInput { resolveBillingAttribution?(workspaceId: string): Promise prepareModelInputProvenance?(input: { userId: string - workspaceId: string + workspaceId?: string }): Promise /** Trusted execution provenance sink; never sourced from an HTTP or model payload. */ resultSecretRegistry?: ResolvedSecretTraceRegistry } -interface KnowledgeSearchContext extends KnowledgeWorkspaceContext { +type KnowledgeSearchContext = KnowledgeResourceContext & { knowledgeBases: KnowledgeBaseWithCounts[] } @@ -126,7 +130,7 @@ export interface SearchKnowledgeResult { topK: number totalResults: number cost?: KnowledgeSearchCost - workspaceId: string + workspaceId?: string userId: string resultSecretRegistry?: ResolvedSecretTraceRegistry } @@ -154,29 +158,43 @@ async function resolveKnowledgeSearchContext( ) } const knowledgeBases = await Promise.all(input.knowledgeBaseIds.map(getKnowledgeBaseById)) - const missingIds = input.knowledgeBaseIds.filter( - (_, index) => !knowledgeBases[index]?.workspaceId - ) + const missingIds = input.knowledgeBaseIds.filter((_, index) => !knowledgeBases[index]) if (missingIds.length > 0) { throw new OrchestrationError( 'not_found', `Knowledge bases not found or access denied: ${missingIds.join(', ')}` ) } - const canonicalWorkspaceIds = new Set(knowledgeBases.map((kb) => kb?.workspaceId)) + const canonicalWorkspaceIds = new Set(knowledgeBases.map((kb) => kb?.workspaceId ?? null)) if (canonicalWorkspaceIds.size !== 1) { throw new OrchestrationError( 'validation', 'Selected knowledge bases must belong to the same workspace' ) } - const canonicalWorkspaceId = knowledgeBases[0]?.workspaceId - if (!canonicalWorkspaceId || (input.workspaceId && input.workspaceId !== canonicalWorkspaceId)) { + const canonicalWorkspaceId = knowledgeBases[0]?.workspaceId ?? null + if (input.workspaceId && input.workspaceId !== canonicalWorkspaceId) { throw new OrchestrationError( 'not_found', `Knowledge bases not found or access denied: ${input.knowledgeBaseIds.join(', ')}` ) } + if (!canonicalWorkspaceId) { + const ownerUserIds = new Set(knowledgeBases.map((knowledgeBase) => knowledgeBase?.userId)) + if (ownerUserIds.size !== 1) { + throw new OrchestrationError( + 'not_found', + `Knowledge bases not found or access denied: ${input.knowledgeBaseIds.join(', ')}` + ) + } + const legacyPersonalOwnerUserId = knowledgeBases[0]?.userId + if (!legacyPersonalOwnerUserId) throw new Error('Legacy Knowledge base owner is missing') + return { + workspaceId: undefined, + legacyPersonalOwnerUserId, + knowledgeBases: knowledgeBases as KnowledgeBaseWithCounts[], + } + } const workspaceContext = await resolveKnowledgeWorkspaceContext({ workspaceId: canonicalWorkspaceId, }) @@ -292,13 +310,16 @@ export const searchKnowledge = defineAuthorizedKnowledgeUseCase({ principal.kind === 'delegated' && principal.serviceId === 'executor' ) - const billingAttribution = hasQuery - ? input.resolveBillingAttribution - ? await input.resolveBillingAttribution(context.workspaceId) - : await resolveKnowledgeBillingAttribution(principal, context) - : undefined - if (shouldMeter && billingAttribution) { - const usage = await checkAttributedUsageLimits(billingAttribution) + const billingAttribution = + hasQuery && context.workspaceId + ? input.resolveBillingAttribution + ? await input.resolveBillingAttribution(context.workspaceId) + : await resolveKnowledgeBillingAttribution(principal, context) + : undefined + if (shouldMeter && hasQuery) { + const usage = billingAttribution + ? await checkAttributedUsageLimits(billingAttribution) + : await checkActorUsageLimits(userId) if (usage.isExceeded) { throw new KnowledgeUsageLimitExceededError( usage.message || 'Usage limit exceeded. Please upgrade your plan to continue.' @@ -450,12 +471,12 @@ export const searchKnowledge = defineAuthorizedKnowledgeUseCase({ } } } - if (shouldMeter && billingAttribution && baseCost && baseCost.total > 0) { + if (shouldMeter && baseCost && baseCost.total > 0) { try { await recordUsage({ userId, - workspaceId: context.workspaceId, - ...toBillingContext(billingAttribution), + ...(context.workspaceId ? { workspaceId: context.workspaceId } : {}), + ...(billingAttribution ? toBillingContext(billingAttribution) : {}), entries: [ { category: 'model', @@ -466,7 +487,11 @@ export const searchKnowledge = defineAuthorizedKnowledgeUseCase({ }, ], }) - await checkAndBillPayerOverageThreshold(billingAttribution.billingEntity) + if (billingAttribution) { + await checkAndBillPayerOverageThreshold(billingAttribution.billingEntity) + } else { + await checkAndBillOverageThreshold(userId) + } } catch (error) { logger.error('Failed to record Knowledge search usage', { error }) } @@ -565,7 +590,7 @@ export const searchKnowledge = defineAuthorizedKnowledgeUseCase({ topK: input.topK, totalResults: results.length, cost, - workspaceId: context.workspaceId, + ...(context.workspaceId ? { workspaceId: context.workspaceId } : {}), userId, resultSecretRegistry: registry, } diff --git a/apps/sim/lib/knowledge/application/tags.test.ts b/apps/sim/lib/knowledge/application/tags.test.ts index da34362a01f..d5ca973db3c 100644 --- a/apps/sim/lib/knowledge/application/tags.test.ts +++ b/apps/sim/lib/knowledge/application/tags.test.ts @@ -37,6 +37,7 @@ vi.mock('@sim/platform-authz/workspace', () => ({ vi.mock('@/lib/knowledge/application/contexts', () => ({ resolveActiveKnowledgeBaseContext: mocks.resolveKnowledgeBase, + resolveActiveKnowledgeResourceContext: mocks.resolveKnowledgeBase, resolveActiveKnowledgeTagContext: mocks.resolveTag, resolveCanonicalActiveKnowledgeDocumentContext: mocks.resolveDocument, })) diff --git a/apps/sim/lib/knowledge/application/tags.ts b/apps/sim/lib/knowledge/application/tags.ts index 6b4ba45835a..3264a6b3c07 100644 --- a/apps/sim/lib/knowledge/application/tags.ts +++ b/apps/sim/lib/knowledge/application/tags.ts @@ -3,7 +3,7 @@ import { OrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { defineAuthorizedKnowledgeUseCase } from '@/lib/knowledge/application/authorized-knowledge-use-case' import { - resolveActiveKnowledgeBaseContext, + resolveActiveKnowledgeResourceContext, resolveActiveKnowledgeTagContext, resolveCanonicalActiveKnowledgeDocumentContext, } from '@/lib/knowledge/application/contexts' @@ -74,7 +74,7 @@ export interface DeleteKnowledgeDocumentTagDefinitionsInput export const listKnowledgeTags = defineAuthorizedKnowledgeUseCase({ operation: knowledgeOperations.listTags, resolveContext: ({ input }: { input: ListKnowledgeTagsInput }) => - resolveActiveKnowledgeBaseContext(input), + resolveActiveKnowledgeResourceContext(input), async execute({ context }) { return { tagDefinitions: await getDocumentTagDefinitions(context.knowledgeBaseId) } }, @@ -83,7 +83,7 @@ export const listKnowledgeTags = defineAuthorizedKnowledgeUseCase({ export const createKnowledgeTag = defineAuthorizedKnowledgeUseCase({ operation: knowledgeOperations.createTag, resolveContext: ({ input }: { input: CreateKnowledgeTagInput }) => - resolveActiveKnowledgeBaseContext(input), + resolveActiveKnowledgeResourceContext(input), async execute({ input, context }): Promise<{ tagDefinition: TagDefinition knowledgeBaseId: string @@ -200,7 +200,7 @@ export const deleteKnowledgeTag = defineAuthorizedKnowledgeUseCase({ export const readKnowledgeTagUsage = defineAuthorizedKnowledgeUseCase({ operation: knowledgeOperations.readTagUsage, resolveContext: ({ input }: { input: ListKnowledgeTagsInput }) => - resolveActiveKnowledgeBaseContext(input), + resolveActiveKnowledgeResourceContext(input), async execute({ context }) { return { usage: await getTagUsageStats(context.knowledgeBaseId, generateRequestId()) } }, @@ -209,7 +209,7 @@ export const readKnowledgeTagUsage = defineAuthorizedKnowledgeUseCase({ export const readDetailedKnowledgeTagUsage = defineAuthorizedKnowledgeUseCase({ operation: knowledgeOperations.readDetailedTagUsage, resolveContext: ({ input }: { input: ListKnowledgeTagsInput }) => - resolveActiveKnowledgeBaseContext(input), + resolveActiveKnowledgeResourceContext(input), async execute({ context }) { return { usage: await getTagUsage(context.knowledgeBaseId, generateRequestId()) } }, @@ -218,7 +218,7 @@ export const readDetailedKnowledgeTagUsage = defineAuthorizedKnowledgeUseCase({ export const readNextKnowledgeTagSlot = defineAuthorizedKnowledgeUseCase({ operation: knowledgeOperations.readNextTagSlot, resolveContext: ({ input }: { input: ReadNextKnowledgeTagSlotInput }) => - resolveActiveKnowledgeBaseContext(input), + resolveActiveKnowledgeResourceContext(input), async execute({ input, context }) { if (!(SUPPORTED_FIELD_TYPES as readonly string[]).includes(input.fieldType)) { throw new OrchestrationError('validation', 'Invalid field type') diff --git a/apps/sim/lib/table/__tests__/service-filter-threading.test.ts b/apps/sim/lib/table/__tests__/service-filter-threading.test.ts index c0e3432a822..e0cdbcc0af3 100644 --- a/apps/sim/lib/table/__tests__/service-filter-threading.test.ts +++ b/apps/sim/lib/table/__tests__/service-filter-threading.test.ts @@ -181,10 +181,10 @@ describe('bulk update/delete limited-subset ordering', () => { expect(dbChainMockFns.limit).toHaveBeenCalledWith(5) }) - it('updates every filter match without an implicit limit', async () => { + it('walks every update match in bounded pages when no operation limit is supplied', async () => { await updateRowsByFilter(TABLE, { filter: { score: { $gt: 0 } }, data: { name: 'x' } }, 'req-1') - expect(dbChainMockFns.orderBy).not.toHaveBeenCalled() - expect(dbChainMockFns.limit).not.toHaveBeenCalled() + expect(dbChainMockFns.orderBy).toHaveBeenCalled() + expect(dbChainMockFns.limit).toHaveBeenCalledWith(TABLE_LIMITS.UPDATE_BATCH_SIZE) }) it('orders the match query when deleteRowsByFilter has a limit', async () => { @@ -193,10 +193,10 @@ describe('bulk update/delete limited-subset ordering', () => { expect(dbChainMockFns.limit).toHaveBeenCalledWith(3) }) - it('deletes every filter match without an implicit limit', async () => { + it('walks every delete match in bounded pages when no operation limit is supplied', async () => { await deleteRowsByFilter(TABLE, { filter: { score: { $gt: 0 } } }, 'req-1') - expect(dbChainMockFns.orderBy).not.toHaveBeenCalled() - expect(dbChainMockFns.limit).not.toHaveBeenCalled() + expect(dbChainMockFns.orderBy).toHaveBeenCalled() + expect(dbChainMockFns.limit).toHaveBeenCalledWith(TABLE_LIMITS.DELETE_PAGE_SIZE) }) }) diff --git a/apps/sim/lib/table/rows/service.ts b/apps/sim/lib/table/rows/service.ts index 9f1a9042472..5254eaec1e7 100644 --- a/apps/sim/lib/table/rows/service.ts +++ b/apps/sim/lib/table/rows/service.ts @@ -58,6 +58,8 @@ import { nextRowPosition, resolveBatchInsertOrderKeys, resolveInsertOrderKey, + selectRowDataPage, + selectRowIdPage, } from '@/lib/table/rows/ordering' import { mutateTableRowsWithSecretProvenance } from '@/lib/table/rows/secret-provenance' import { @@ -1790,6 +1792,120 @@ export async function deleteRow( logger.info(`[${requestId}] Deleted row ${rowId} from table ${table.id}`) } +type BulkUpdateMatch = { id: string; data: RowData } + +/** Validates a bounded page of rows against a bulk merge patch. */ +function validateBulkUpdateMatches( + table: TableDefinition, + rows: BulkUpdateMatch[], + patch: RowData +): void { + for (const row of rows) { + const mergedData = { ...row.data, ...patch } + const sizeValidation = validateRowSize(mergedData) + if (!sizeValidation.valid) { + throw new OrchestrationError( + 'validation', + `Row ${row.id}: ${sizeValidation.errors.join(', ')}` + ) + } + + const schemaValidation = coerceRowToSchema(mergedData, table.schema) + if (!schemaValidation.valid) { + throw new OrchestrationError( + 'validation', + `Row ${row.id}: ${schemaValidation.errors.join(', ')}` + ) + } + } +} + +/** Persists one bounded bulk-update page and returns the rows actually changed. */ +async function persistBulkUpdateBatch( + table: TableDefinition, + rows: BulkUpdateMatch[], + patchJson: string, + now: Date, + secretProvenance: BulkUpdateData['secretProvenance'] +): Promise { + const ids = rows.map((row) => row.id) + return db.transaction(async (trx) => { + await setTableTxTimeouts(trx, { statementMs: 60_000 }) + return mutateTableRowsWithSecretProvenance(trx, { + rows: ids.map((rowId) => ({ rowId, provenance: secretProvenance })), + rowState: 'existing', + mode: 'merge', + mutate: async () => { + const affectedRowIds: string[] = [] + for (let index = 0; index < ids.length; index += TABLE_LIMITS.UPDATE_BATCH_SIZE) { + const batchIds = ids.slice(index, index + TABLE_LIMITS.UPDATE_BATCH_SIZE) + const updated = await trx + .update(userTableRows) + .set({ + data: sql`${userTableRows.data} || ${patchJson}::jsonb`, + updatedAt: now, + }) + .where( + and( + eq(userTableRows.tableId, table.id), + eq(userTableRows.workspaceId, table.workspaceId), + inArray(userTableRows.id, batchIds) + ) + ) + .returning({ id: userTableRows.id }) + affectedRowIds.push(...updated.map((row) => row.id)) + } + return { value: affectedRowIds, affectedRowIds } + }, + }) + }) +} + +/** Emits trigger and enrichment side effects for one committed bulk-update page. */ +function dispatchBulkUpdateEffects( + table: TableDefinition, + rows: BulkUpdateMatch[], + affectedRowIds: string[], + patch: RowData, + now: Date, + requestId: string, + actorUserId: BulkUpdateData['actorUserId'] +): void { + const affectedRowIdSet = new Set(affectedRowIds) + const affectedRows = rows.filter((row) => affectedRowIdSet.has(row.id)) + if (affectedRows.length === 0) return + + const oldRows = new Map(affectedRows.map((row) => [row.id, row.data])) + const updatedRows: TableRow[] = affectedRows.map((row) => ({ + id: row.id, + data: { ...row.data, ...patch }, + executions: {}, + position: 0, + createdAt: now, + updatedAt: now, + })) + void fireTableTrigger( + table.id, + table.name, + 'update', + updatedRows, + oldRows, + table.schema, + requestId + ) + void runWorkflowColumn({ + tableId: table.id, + workspaceId: table.workspaceId, + rowIds: affectedRowIds, + mode: 'new', + isManualRun: false, + requestId, + triggeredByUserId: actorUserId, + }).catch((error) => + logger.error(`[${requestId}] auto-dispatch (updateRowsByFilter) failed:`, error) + ) +} + /** * Updates multiple rows matching a filter. * @@ -1820,67 +1936,132 @@ export async function updateRowsByFilter( eq(userTableRows.workspaceId, table.workspaceId) ) - // A limit selects a SUBSET, so impose the default `(order_key, id)` order — - // without it Postgres returns planner-arbitrary rows and "update the first N" - // is nondeterministic. Sort is irrelevant (and skipped) when every match is updated. - const matchingRows = await withSeqscanOff(async (trx) => { - const base = trx - .select({ id: userTableRows.id, data: userTableRows.data }) - .from(userTableRows) - .where(and(baseConditions, filterClause)) - return data.limit === undefined - ? base - : base - .orderBy(buildRowOrderBySql(undefined, tableName, table.schema.columns)) - .limit(data.limit) - }) - - if (matchingRows.length === 0) { - return { affectedCount: 0, affectedRowIds: [] } - } - - // Coerce the patch itself in place — the write below persists `data.data` - // (as `patchJson`), so coercing only the per-row merged copies would be - // discarded. The merged validation in the loop still enforces required - // fields against the full row. coerceRowValues(data.data, table.schema) + const uniqueColumns = getUniqueColumns(table.schema) + const uniqueColumnsInUpdate = uniqueColumns.filter((col) => getColumnId(col) in data.data) + const patchJson = JSON.stringify(data.data) + const now = new Date() + const limit = data.limit + + if (limit === undefined) { + const cutoff = new Date() + let matchingRowCount = 0 + let singleMatchingRow: BulkUpdateMatch | undefined + let afterId: string | undefined + + while (true) { + const page = await selectRowDataPage({ + tableId: table.id, + workspaceId: table.workspaceId, + cutoff, + filterClause, + afterId, + limit: TABLE_LIMITS.UPDATE_BATCH_SIZE, + }) + if (page.length === 0) break - for (const row of matchingRows) { - const existingData = row.data as RowData - const mergedData = { ...existingData, ...data.data } + validateBulkUpdateMatches(table, page, data.data) + matchingRowCount += page.length + singleMatchingRow ??= page[0] + afterId = page[page.length - 1].id + if (page.length < TABLE_LIMITS.UPDATE_BATCH_SIZE) break + } - const sizeValidation = validateRowSize(mergedData) - if (!sizeValidation.valid) { - throw new OrchestrationError( - 'validation', - `Row ${row.id}: ${sizeValidation.errors.join(', ')}` + if (matchingRowCount === 0) { + return { affectedCount: 0, affectedRowIds: [] } + } + + if (uniqueColumnsInUpdate.length > 0) { + if (matchingRowCount > 1) { + throw new OrchestrationError( + 'validation', + `Cannot set unique column values when updating multiple rows. ` + + `Columns with unique constraint: ${uniqueColumnsInUpdate.map((column) => column.name).join(', ')}. ` + + `Updating ${matchingRowCount} rows with the same value would violate uniqueness.` + ) + } + if (!singleMatchingRow) { + throw new Error('Bulk update lost its selected row') + } + const uniqueValidation = await checkUniqueConstraintsDb( + table.id, + { ...singleMatchingRow.data, ...data.data }, + table.schema, + singleMatchingRow.id ) + if (!uniqueValidation.valid) { + throw new OrchestrationError( + 'validation', + `Unique constraint violation: ${uniqueValidation.errors.join(', ')}` + ) + } } - const schemaValidation = coerceRowToSchema(mergedData, table.schema) - if (!schemaValidation.valid) { - throw new OrchestrationError( - 'validation', - `Row ${row.id}: ${schemaValidation.errors.join(', ')}` + const affectedRowIds: string[] = [] + afterId = undefined + while (true) { + const batchRows = await selectRowDataPage({ + tableId: table.id, + workspaceId: table.workspaceId, + cutoff, + filterClause, + afterId, + limit: TABLE_LIMITS.UPDATE_BATCH_SIZE, + }) + if (batchRows.length === 0) break + + validateBulkUpdateMatches(table, batchRows, data.data) + const nextAfterId = batchRows[batchRows.length - 1].id + const batchAffectedRowIds = await persistBulkUpdateBatch( + table, + batchRows, + patchJson, + now, + data.secretProvenance + ) + affectedRowIds.push(...batchAffectedRowIds) + dispatchBulkUpdateEffects( + table, + batchRows, + batchAffectedRowIds, + data.data, + now, + requestId, + data.actorUserId ) + afterId = nextAfterId + if (batchRows.length < TABLE_LIMITS.UPDATE_BATCH_SIZE) break } + + logger.info(`[${requestId}] Updated ${affectedRowIds.length} rows in table ${table.id}`) + return { affectedCount: affectedRowIds.length, affectedRowIds } } - const uniqueColumns = getUniqueColumns(table.schema) - const uniqueColumnsInUpdate = uniqueColumns.filter((col) => getColumnId(col) in data.data) + const selectedRows = await withSeqscanOff(async (trx) => + trx + .select({ id: userTableRows.id, data: userTableRows.data }) + .from(userTableRows) + .where(and(baseConditions, filterClause)) + .orderBy(buildRowOrderBySql(undefined, tableName, table.schema.columns)) + .limit(limit) + ) + const matchingRows = selectedRows.map((row) => ({ id: row.id, data: row.data as RowData })) + if (matchingRows.length === 0) { + return { affectedCount: 0, affectedRowIds: [] } + } + + validateBulkUpdateMatches(table, matchingRows, data.data) if (uniqueColumnsInUpdate.length > 0) { if (matchingRows.length > 1) { throw new OrchestrationError( 'validation', `Cannot set unique column values when updating multiple rows. ` + - `Columns with unique constraint: ${uniqueColumnsInUpdate.map((c) => c.name).join(', ')}. ` + + `Columns with unique constraint: ${uniqueColumnsInUpdate.map((column) => column.name).join(', ')}. ` + `Updating ${matchingRows.length} rows with the same value would violate uniqueness.` ) } - - // Only one row — only the touched unique columns need re-checking. const row = matchingRows[0] - const mergedData = { ...(row.data as RowData), ...data.data } + const mergedData = { ...row.data, ...data.data } const uniqueValidation = await checkUniqueConstraintsDb( table.id, mergedData, @@ -1895,76 +2076,24 @@ export async function updateRowsByFilter( } } - const now = new Date() - const ids = matchingRows.map((r) => r.id) - const patchJson = JSON.stringify(data.data) - - const affectedRowIds = await db.transaction(async (trx) => { - await setTableTxTimeouts(trx, { statementMs: 60_000 }) - return mutateTableRowsWithSecretProvenance(trx, { - rows: ids.map((rowId) => ({ rowId, provenance: data.secretProvenance })), - rowState: 'existing', - mode: 'merge', - mutate: async () => { - const affectedRowIds: string[] = [] - for (let i = 0; i < ids.length; i += TABLE_LIMITS.UPDATE_BATCH_SIZE) { - const batchIds = ids.slice(i, i + TABLE_LIMITS.UPDATE_BATCH_SIZE) - const updated = await trx - .update(userTableRows) - .set({ - data: sql`${userTableRows.data} || ${patchJson}::jsonb`, - updatedAt: now, - }) - .where( - and( - eq(userTableRows.tableId, table.id), - eq(userTableRows.workspaceId, table.workspaceId), - inArray(userTableRows.id, batchIds) - ) - ) - .returning({ id: userTableRows.id }) - affectedRowIds.push(...updated.map((row) => row.id)) - } - return { value: affectedRowIds, affectedRowIds } - }, - }) - }) + const affectedRowIds = await persistBulkUpdateBatch( + table, + matchingRows, + patchJson, + now, + data.secretProvenance + ) logger.info(`[${requestId}] Updated ${affectedRowIds.length} rows in table ${table.id}`) - - const affectedRowIdSet = new Set(affectedRowIds) - const affectedRows = matchingRows.filter((row) => affectedRowIdSet.has(row.id)) - const oldRows = new Map(affectedRows.map((r) => [r.id, r.data as RowData])) - const updatedRows: TableRow[] = affectedRows.map((r) => ({ - id: r.id, - data: { ...(r.data as RowData), ...data.data }, - executions: {}, - position: 0, - createdAt: now, - updatedAt: now, - })) - if (updatedRows.length > 0) { - void fireTableTrigger( - table.id, - table.name, - 'update', - updatedRows, - oldRows, - table.schema, - requestId - ) - void runWorkflowColumn({ - tableId: table.id, - workspaceId: table.workspaceId, - rowIds: updatedRows.map((r) => r.id), - mode: 'new', - isManualRun: false, - requestId, - triggeredByUserId: data.actorUserId, - }).catch((err) => - logger.error(`[${requestId}] auto-dispatch (updateRowsByFilter) failed:`, err) - ) - } + dispatchBulkUpdateEffects( + table, + matchingRows, + affectedRowIds, + data.data, + now, + requestId, + data.actorUserId + ) return { affectedCount: affectedRowIds.length, @@ -2253,32 +2382,58 @@ export async function deleteRowsByFilter( eq(userTableRows.workspaceId, table.workspaceId) ) - // A limit deletes a SUBSET, so order deterministically by `(order_key, id)` — - // see updateRowsByFilter. Unbounded deletes affect every match, so order is moot. - const matchingRows = await withSeqscanOff(async (trx) => { - const base = trx - .select({ id: userTableRows.id, position: userTableRows.position }) - .from(userTableRows) - .where(and(baseConditions, filterClause)) - return data.limit === undefined - ? base - : base - .orderBy(buildRowOrderBySql(undefined, tableName, table.schema.columns)) - .limit(data.limit) - }) - - if (matchingRows.length === 0) { - return { affectedCount: 0, affectedRowIds: [] } + const limit = data.limit + const deletedRows: { id: string }[] = [] + if (limit === undefined) { + const cutoff = new Date() + let afterId: string | undefined + while (true) { + const page = await selectRowIdPage({ + tableId: table.id, + workspaceId: table.workspaceId, + cutoff, + filterClause, + afterId, + limit: TABLE_LIMITS.DELETE_PAGE_SIZE, + }) + if (page.length === 0) break + const nextAfterId = page[page.length - 1] + for (let index = 0; index < page.length; index += TABLE_LIMITS.DELETE_BATCH_SIZE) { + deletedRows.push( + ...(await deleteOrderedRowsByIds({ + tableId: table.id, + workspaceId: table.workspaceId, + rowIds: page.slice(index, index + TABLE_LIMITS.DELETE_BATCH_SIZE), + proof, + })) + ) + } + afterId = nextAfterId + if (page.length < TABLE_LIMITS.DELETE_PAGE_SIZE) break + } + } else { + const matchingRows = await withSeqscanOff(async (trx) => + trx + .select({ id: userTableRows.id }) + .from(userTableRows) + .where(and(baseConditions, filterClause)) + .orderBy(buildRowOrderBySql(undefined, tableName, table.schema.columns)) + .limit(limit) + ) + const rowIds = matchingRows.map((row) => row.id) + if (rowIds.length > 0) { + deletedRows.push( + ...(await deleteOrderedRowsByIds({ + tableId: table.id, + workspaceId: table.workspaceId, + rowIds, + proof, + })) + ) + } } - const rowIds = matchingRows.map((r) => r.id) - - const deletedRows = await deleteOrderedRowsByIds({ - tableId: table.id, - workspaceId: table.workspaceId, - rowIds, - proof, - }) + if (deletedRows.length === 0) return { affectedCount: 0, affectedRowIds: [] } const deletedRowIds = deletedRows.map((row) => row.id) logger.info(`[${requestId}] Deleted ${deletedRowIds.length} rows from table ${table.id}`) diff --git a/apps/sim/tools/agiloft/utils.test.ts b/apps/sim/tools/agiloft/utils.test.ts index 6cbe1d62fce..bfffee607da 100644 --- a/apps/sim/tools/agiloft/utils.test.ts +++ b/apps/sim/tools/agiloft/utils.test.ts @@ -81,6 +81,9 @@ describe('CRUD endpoints', () => { it('does not ask for the .json variant on operations whose JSON shape is undocumented', () => { expect(buildSearchRecordsUrl(BASE, { ...baseParams, query: "a='b'" })).not.toContain('.json') + expect( + buildLockRecordUrl(BASE, { ...baseParams, recordId: '18', lockAction: 'check' }) + ).not.toContain('.json') }) }) @@ -147,12 +150,6 @@ describe('buildSearchRecordsUrl', () => { }) describe('buildLockRecordUrl', () => { - it('requests the JSON variant consumed by the lock route', () => { - expect( - buildLockRecordUrl(BASE, { ...baseParams, recordId: '18', lockAction: 'check' }) - ).toContain('/EWLock/.json?') - }) - it('adds force only on unlock, where EWLock accepts it', () => { const unlock = buildLockRecordUrl(BASE, { ...baseParams, diff --git a/apps/sim/tools/agiloft/utils.ts b/apps/sim/tools/agiloft/utils.ts index 4d26964e110..8f9d084cde9 100644 --- a/apps/sim/tools/agiloft/utils.ts +++ b/apps/sim/tools/agiloft/utils.ts @@ -162,7 +162,7 @@ export function buildAttachmentInfoUrl(base: string, params: AgiloftAttachmentIn */ export function buildLockRecordUrl(base: string, params: AgiloftLockRecordParams): string { const id = encodeURIComponent(params.recordId.trim()) - let url = `${base}/ewws/EWLock/.json?${buildEwBaseQuery(params)}&id=${id}` + let url = `${base}/ewws/EWLock?${buildEwBaseQuery(params)}&id=${id}` if (params.lockAction === 'unlock' && params.force) { url += '&force=true' } diff --git a/packages/ts-sdk/src/index.test.ts b/packages/ts-sdk/src/index.test.ts index b0e4eec3bf8..ca7fef83f83 100644 --- a/packages/ts-sdk/src/index.test.ts +++ b/packages/ts-sdk/src/index.test.ts @@ -186,7 +186,7 @@ describe('SimStudioClient', () => { status: 200, json: vi.fn().mockResolvedValue(failed), headers: { get: vi.fn().mockReturnValue(null) }, - } as any) + }) await expect(client.executeWorkflow('workflow-id', {})).rejects.toMatchObject({ name: 'SimStudioError', From 0bc1fb976126cd118cc73aa8256f8d7e3d9b40a3 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Tue, 11 Aug 2026 16:25:17 -0700 Subject: [PATCH 3/7] fix(files): validate ensured folder paths --- .../workspace-file-folder-manager.test.ts | 14 +++++++++++++- .../workspace/workspace-file-folder-manager.ts | 11 ++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts index e0f9347d1fd..86564c0471d 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts @@ -3,10 +3,12 @@ */ import { describe, expect, it } from 'vitest' +import { MAX_FOLDER_PATH_SEGMENTS } from '@/lib/folders/paths' import { buildWorkspaceFileFolderPathMap, + ensureWorkspaceFileFolderPath, normalizeWorkspaceFileItemName, -} from './workspace-file-folder-manager' +} from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager' describe('workspace file folder paths', () => { it('builds nested paths from parent relationships', () => { @@ -30,4 +32,14 @@ describe('workspace file folder paths', () => { 'File name cannot contain path separators or dot segments' ) }) + + it('rejects oversized ensured paths before persisting any folders', async () => { + await expect( + ensureWorkspaceFileFolderPath({ + workspaceId: 'workspace-1', + userId: 'user-1', + pathSegments: Array.from({ length: MAX_FOLDER_PATH_SEGMENTS + 1 }, () => 'nested'), + }) + ).rejects.toThrow(`Folder paths cannot exceed ${MAX_FOLDER_PATH_SEGMENTS} segments`) + }) }) diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts index 03f11ee7710..918f963c1d1 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts @@ -9,6 +9,7 @@ import type { DbOrTx } from '@/lib/db/types' import { acquireFolderMutationLock } from '@/lib/folders/locks' import { deduplicateFolderName } from '@/lib/folders/naming' import { + buildFolderPath, buildFolderPathIndex, folderNameFromPath, parentFolderPath, @@ -558,10 +559,15 @@ export async function ensureWorkspaceFileFolderPath(params: { }): Promise { if (params.pathSegments.length === 0) return null + const pathSegments = params.pathSegments.map((segment) => + normalizeWorkspaceFileItemName(segment, 'Folder') + ) + buildFolderPath(pathSegments) + // Fast path: the whole chain already exists (the common case for repeated // writes into known folders) — per-segment indexed lookups instead of // loading the workspace's entire folder table. - const existing = await findWorkspaceFileFolderIdByPath(params.workspaceId, params.pathSegments) + const existing = await findWorkspaceFileFolderIdByPath(params.workspaceId, pathSegments) if (existing) return existing // Load all active folders once and build a lookup keyed by "name|parentId" @@ -585,8 +591,7 @@ export async function ensureWorkspaceFileFolderPath(params: { let parentId: string | null = null - for (const rawSegment of params.pathSegments) { - const name = normalizeWorkspaceFileItemName(rawSegment, 'Folder') + for (const name of pathSegments) { const lookupKey = `${name}|${parentId ?? ''}` const cached = folderByNameParent.get(lookupKey) From 9d7c60f71c374dcf27984cbf986545df73053899 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Tue, 11 Aug 2026 16:33:36 -0700 Subject: [PATCH 4/7] fix(files): project folder path validation --- .../app/api/tools/file/manage/route.test.ts | 22 +++++++++++++++++++ apps/sim/app/api/tools/file/manage/route.ts | 8 ++++++- .../workspace-file-folder-manager.test.ts | 5 ++++- .../workspace-file-folder-manager.ts | 10 ++++++++- 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/apps/sim/app/api/tools/file/manage/route.test.ts b/apps/sim/app/api/tools/file/manage/route.test.ts index 7fa6d78b4ac..6bebe9f88ab 100644 --- a/apps/sim/app/api/tools/file/manage/route.test.ts +++ b/apps/sim/app/api/tools/file/manage/route.test.ts @@ -4,6 +4,7 @@ import { createMockRequest, hybridAuthMockFns } from '@sim/testing' import JSZip from 'jszip' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { MAX_FOLDER_PATH_SEGMENTS } from '@/lib/folders/paths' const { mockAssertActiveWorkspaceAccess, @@ -456,6 +457,27 @@ describe('POST /api/tools/file/manage content provenance', () => { ) }) + it('returns 400 before moving when the target folder path exceeds canonical limits', async () => { + const response = await POST( + createMockRequest('POST', { + operation: 'move', + workspaceId: 'workspace-1', + fileId: 'file-1', + targetFolder: Array.from( + { length: MAX_FOLDER_PATH_SEGMENTS + 1 }, + (_, index) => `folder-${index}` + ).join('/'), + }) + ) + + expect(response.status).toBe(400) + await expect(response.json()).resolves.toMatchObject({ + success: false, + error: `Folder paths cannot exceed ${MAX_FOLDER_PATH_SEGMENTS} segments`, + }) + expect(mockMoveWorkspaceFileItems).not.toHaveBeenCalled() + }) + it('persists an authenticated file write with unavailable lineage as unknown', async () => { const response = await POST( createMockRequest( diff --git a/apps/sim/app/api/tools/file/manage/route.ts b/apps/sim/app/api/tools/file/manage/route.ts index 7c73861446e..71191f1887d 100644 --- a/apps/sim/app/api/tools/file/manage/route.ts +++ b/apps/sim/app/api/tools/file/manage/route.ts @@ -762,12 +762,18 @@ export const POST = withRouteHandler(async (request: NextRequest) => { .map((s) => s.trim()) .filter(Boolean) : [] + let targetFolderPath: string + try { + targetFolderPath = buildFolderPath(pathSegments) + } catch (error) { + throw new OrchestrationError('validation', getErrorMessage(error)) + } await moveWorkspaceFileItemsOperation.execute({ principal, input: { workspaceId, fileIds: [fileId], - targetFolderPath: buildFolderPath(pathSegments), + targetFolderPath, }, request, }) diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts index 86564c0471d..49f7de2c860 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts @@ -40,6 +40,9 @@ describe('workspace file folder paths', () => { userId: 'user-1', pathSegments: Array.from({ length: MAX_FOLDER_PATH_SEGMENTS + 1 }, () => 'nested'), }) - ).rejects.toThrow(`Folder paths cannot exceed ${MAX_FOLDER_PATH_SEGMENTS} segments`) + ).rejects.toMatchObject({ + code: 'validation', + message: `Folder paths cannot exceed ${MAX_FOLDER_PATH_SEGMENTS} segments`, + }) }) }) diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts index 918f963c1d1..0de445fdc7e 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts @@ -11,6 +11,7 @@ import { deduplicateFolderName } from '@/lib/folders/naming' import { buildFolderPath, buildFolderPathIndex, + FolderPathError, folderNameFromPath, parentFolderPath, parseFolderPath, @@ -562,7 +563,14 @@ export async function ensureWorkspaceFileFolderPath(params: { const pathSegments = params.pathSegments.map((segment) => normalizeWorkspaceFileItemName(segment, 'Folder') ) - buildFolderPath(pathSegments) + try { + buildFolderPath(pathSegments) + } catch (error) { + if (error instanceof FolderPathError) { + throw new OrchestrationError('validation', error.message) + } + throw error + } // Fast path: the whole chain already exists (the common case for repeated // writes into known folders) — per-segment indexed lookups instead of From 69763937ace72ae80567a3d37ffbf6279d840788 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Tue, 11 Aug 2026 16:57:58 -0700 Subject: [PATCH 5/7] fix(files): align archive regression fixture --- apps/sim/app/api/tools/file/manage/route.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/sim/app/api/tools/file/manage/route.test.ts b/apps/sim/app/api/tools/file/manage/route.test.ts index 6bebe9f88ab..2f26fef43b1 100644 --- a/apps/sim/app/api/tools/file/manage/route.test.ts +++ b/apps/sim/app/api/tools/file/manage/route.test.ts @@ -208,6 +208,7 @@ describe('POST /api/tools/file/manage content provenance', () => { mockEnsureWorkspaceFileFolderPath.mockImplementation( async ({ input }: { input: { pathSegments: string[] } }) => ({ folderId: input.pathSegments.length === 0 ? null : 'folder-1', + createdFolderIds: [], }) ) mockDownloadServableFileFromStorage.mockImplementation(async (file: { name: string }) => ({ @@ -692,7 +693,7 @@ describe('POST /api/tools/file/manage content provenance', () => { 'child.txt', 'text/plain', { - exactName: true, + exactName: false, folderId: 'folder-1', folderPath: undefined, secretProvenance: { status: 'unknown' }, From cd17e8880c79a521e53fb8dc03590c8168e9adb6 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Tue, 11 Aug 2026 17:12:47 -0700 Subject: [PATCH 6/7] fix(tables): avoid partial bulk update failures --- .../__tests__/bulk-update-concurrency.test.ts | 139 ++++++++++++++++++ apps/sim/lib/table/rows/service.ts | 117 ++++++++++----- 2 files changed, 216 insertions(+), 40 deletions(-) create mode 100644 apps/sim/lib/table/__tests__/bulk-update-concurrency.test.ts diff --git a/apps/sim/lib/table/__tests__/bulk-update-concurrency.test.ts b/apps/sim/lib/table/__tests__/bulk-update-concurrency.test.ts new file mode 100644 index 00000000000..b30a977529c --- /dev/null +++ b/apps/sim/lib/table/__tests__/bulk-update-concurrency.test.ts @@ -0,0 +1,139 @@ +/** + * @vitest-environment node + */ +import { dbChainMockFns, queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing' +import { sql } from 'drizzle-orm' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { TABLE_LIMITS } from '@/lib/table/constants' +import type { RowData, TableDefinition } from '@/lib/table/types' + +const mocks = vi.hoisted(() => ({ + selectRowDataPage: vi.fn(), + mutateTableRowsWithSecretProvenance: vi.fn(), + validateRowSize: vi.fn(), + coerceRowToSchema: vi.fn(), +})) + +vi.mock('@/lib/table/rows/ordering', () => ({ + selectRowDataPage: mocks.selectRowDataPage, +})) + +vi.mock('@/lib/table/rows/secret-provenance', () => ({ + mutateTableRowsWithSecretProvenance: mocks.mutateTableRowsWithSecretProvenance, +})) + +vi.mock('@/lib/table/sql', () => ({ + buildFilterClause: vi.fn(() => sql`true`), + buildPredicateClause: vi.fn(() => sql`true`), + buildSortClause: vi.fn(() => sql`true`), + escapeLikePattern: vi.fn((value: string) => value), + fieldPredicate: vi.fn(() => sql`true`), +})) + +vi.mock('@/lib/table/trigger', () => ({ + fireTableTrigger: vi.fn(), +})) + +vi.mock('@/lib/table/validation', () => ({ + validateRowSize: mocks.validateRowSize, + coerceRowToSchema: mocks.coerceRowToSchema, + coerceRowValues: vi.fn(), + getUniqueColumns: vi.fn(() => []), + checkUniqueConstraintsDb: vi.fn(async () => ({ valid: true, errors: [] })), + checkBatchUniqueConstraintsDb: vi.fn(async () => ({ valid: true, errors: [] })), +})) + +vi.mock('@/lib/table/workflow-columns', () => ({ + cancelWorkflowGroupRuns: vi.fn(), + runWorkflowColumn: vi.fn(async () => undefined), +})) + +import { updateRowsByFilter } from '@/lib/table/rows/service' + +const TABLE: TableDefinition = { + id: 'table-1', + name: 'Contacts', + description: null, + schema: { columns: [{ id: 'name', name: 'Name', type: 'string' }] }, + metadata: null, + rowCount: TABLE_LIMITS.UPDATE_BATCH_SIZE + 1, + maxRows: 10_000, + workspaceId: 'workspace-1', + createdBy: 'user-1', + locks: { schemaLocked: false, insertLocked: false, updateLocked: false, deleteLocked: false }, + archivedAt: null, + createdAt: new Date('2026-08-11T00:00:00.000Z'), + updatedAt: new Date('2026-08-11T00:00:00.000Z'), +} + +function row(id: string, data: RowData = { name: id }): { id: string; data: RowData } { + return { id, data } +} + +describe('bulk update concurrency', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mocks.validateRowSize.mockImplementation((data: RowData) => + data.concurrentlyInvalid + ? { valid: false, errors: ['row is no longer valid'] } + : { valid: true, errors: [] } + ) + mocks.coerceRowToSchema.mockReturnValue({ valid: true, errors: [] }) + mocks.mutateTableRowsWithSecretProvenance.mockImplementation( + async (_trx, options: { mutate: () => Promise<{ value: string[] }> }) => { + const outcome = await options.mutate() + return outcome.value + } + ) + }) + + it('fails preflight before opening a mutation transaction for an invalid selected row', async () => { + mocks.selectRowDataPage.mockResolvedValueOnce([ + row('invalid-row', { name: 'invalid', concurrentlyInvalid: true }), + ]) + + await expect( + updateRowsByFilter( + TABLE, + { filter: { status: 'active' }, data: { name: 'updated' } }, + 'request-1' + ) + ).rejects.toThrow('Row invalid-row: row is no longer valid') + + expect(dbChainMockFns.transaction).not.toHaveBeenCalled() + expect(dbChainMockFns.update).not.toHaveBeenCalled() + }) + + it('skips a later page invalidated after preflight without returning a partial failure', async () => { + const firstPage = Array.from({ length: TABLE_LIMITS.UPDATE_BATCH_SIZE }, (_, index) => + row(`row-${index.toString().padStart(4, '0')}`) + ) + const lastRow = row('row-last') + + mocks.selectRowDataPage + .mockResolvedValueOnce(firstPage) + .mockResolvedValueOnce([lastRow]) + .mockResolvedValueOnce(firstPage) + .mockResolvedValueOnce([lastRow]) + + queueTableRows(schemaMock.userTableRows, firstPage) + queueTableRows(schemaMock.userTableRows, [ + row(lastRow.id, { name: 'changed concurrently', concurrentlyInvalid: true }), + ]) + dbChainMockFns.returning.mockResolvedValueOnce(firstPage.map(({ id }) => ({ id }))) + + const result = await updateRowsByFilter( + TABLE, + { filter: { status: 'active' }, data: { name: 'updated' } }, + 'request-1' + ) + + expect(result).toEqual({ + affectedCount: firstPage.length, + affectedRowIds: firstPage.map(({ id }) => id), + }) + expect(dbChainMockFns.update).toHaveBeenCalledTimes(1) + expect(mocks.mutateTableRowsWithSecretProvenance).toHaveBeenCalledTimes(2) + }) +}) diff --git a/apps/sim/lib/table/rows/service.ts b/apps/sim/lib/table/rows/service.ts index 5254eaec1e7..162780b5f3f 100644 --- a/apps/sim/lib/table/rows/service.ts +++ b/apps/sim/lib/table/rows/service.ts @@ -15,7 +15,7 @@ import { tableJobs, userTableRows } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' -import { and, count, eq, inArray, lte, notInArray, type SQL, sql } from 'drizzle-orm' +import { and, asc, count, eq, inArray, lte, notInArray, type SQL, sql } from 'drizzle-orm' import { OrchestrationError } from '@/lib/core/orchestration/types' import { assertRowCapacity, @@ -1794,6 +1794,19 @@ export async function deleteRow( type BulkUpdateMatch = { id: string; data: RowData } +function bulkUpdateValidationError( + table: TableDefinition, + row: BulkUpdateMatch, + patch: RowData +): string | null { + const mergedData = { ...row.data, ...patch } + const sizeValidation = validateRowSize(mergedData) + if (!sizeValidation.valid) return sizeValidation.errors.join(', ') + + const schemaValidation = coerceRowToSchema(mergedData, table.schema) + return schemaValidation.valid ? null : schemaValidation.errors.join(', ') +} + /** Validates a bounded page of rows against a bulk merge patch. */ function validateBulkUpdateMatches( table: TableDefinition, @@ -1801,44 +1814,61 @@ function validateBulkUpdateMatches( patch: RowData ): void { for (const row of rows) { - const mergedData = { ...row.data, ...patch } - const sizeValidation = validateRowSize(mergedData) - if (!sizeValidation.valid) { - throw new OrchestrationError( - 'validation', - `Row ${row.id}: ${sizeValidation.errors.join(', ')}` - ) - } - - const schemaValidation = coerceRowToSchema(mergedData, table.schema) - if (!schemaValidation.valid) { - throw new OrchestrationError( - 'validation', - `Row ${row.id}: ${schemaValidation.errors.join(', ')}` - ) - } + const error = bulkUpdateValidationError(table, row, patch) + if (error) throw new OrchestrationError('validation', `Row ${row.id}: ${error}`) } } -/** Persists one bounded bulk-update page and returns the rows actually changed. */ -async function persistBulkUpdateBatch( - table: TableDefinition, - rows: BulkUpdateMatch[], - patchJson: string, - now: Date, +/** Persists one bounded bulk-update page and returns the locked rows actually changed. */ +async function persistBulkUpdateBatch(params: { + table: TableDefinition + rows: BulkUpdateMatch[] + patch: RowData + patchJson: string + filterClause: SQL + now: Date secretProvenance: BulkUpdateData['secretProvenance'] -): Promise { + requestId: string +}): Promise<{ rows: BulkUpdateMatch[]; affectedRowIds: string[] }> { + const { table, rows, patch, patchJson, filterClause, now, secretProvenance, requestId } = params const ids = rows.map((row) => row.id) - return db.transaction(async (trx) => { + const persistedRows: BulkUpdateMatch[] = [] + const affectedRowIds = await db.transaction(async (trx) => { await setTableTxTimeouts(trx, { statementMs: 60_000 }) return mutateTableRowsWithSecretProvenance(trx, { rows: ids.map((rowId) => ({ rowId, provenance: secretProvenance })), rowState: 'existing', mode: 'merge', mutate: async () => { + const currentRows = await trx + .select({ id: userTableRows.id, data: userTableRows.data }) + .from(userTableRows) + .where( + and( + eq(userTableRows.tableId, table.id), + eq(userTableRows.workspaceId, table.workspaceId), + inArray(userTableRows.id, ids), + filterClause + ) + ) + .orderBy(asc(userTableRows.id)) + const skippedRowIds: string[] = [] + for (const currentRow of currentRows) { + const row = { id: currentRow.id, data: currentRow.data as RowData } + if (bulkUpdateValidationError(table, row, patch)) skippedRowIds.push(row.id) + else persistedRows.push(row) + } + if (skippedRowIds.length > 0) { + logger.warn( + `[${requestId}] Skipping rows concurrently changed to values that cannot accept the bulk patch`, + { tableId: table.id, rowIds: skippedRowIds } + ) + } + + const validIds = persistedRows.map((row) => row.id) const affectedRowIds: string[] = [] - for (let index = 0; index < ids.length; index += TABLE_LIMITS.UPDATE_BATCH_SIZE) { - const batchIds = ids.slice(index, index + TABLE_LIMITS.UPDATE_BATCH_SIZE) + for (let index = 0; index < validIds.length; index += TABLE_LIMITS.UPDATE_BATCH_SIZE) { + const batchIds = validIds.slice(index, index + TABLE_LIMITS.UPDATE_BATCH_SIZE) const updated = await trx .update(userTableRows) .set({ @@ -1859,6 +1889,7 @@ async function persistBulkUpdateBatch( }, }) }) + return { rows: persistedRows, affectedRowIds } } /** Emits trigger and enrichment side effects for one committed bulk-update page. */ @@ -2010,20 +2041,22 @@ export async function updateRowsByFilter( }) if (batchRows.length === 0) break - validateBulkUpdateMatches(table, batchRows, data.data) const nextAfterId = batchRows[batchRows.length - 1].id - const batchAffectedRowIds = await persistBulkUpdateBatch( + const persisted = await persistBulkUpdateBatch({ table, - batchRows, + rows: batchRows, + patch: data.data, patchJson, + filterClause, now, - data.secretProvenance - ) - affectedRowIds.push(...batchAffectedRowIds) + secretProvenance: data.secretProvenance, + requestId, + }) + affectedRowIds.push(...persisted.affectedRowIds) dispatchBulkUpdateEffects( table, - batchRows, - batchAffectedRowIds, + persisted.rows, + persisted.affectedRowIds, data.data, now, requestId, @@ -2076,18 +2109,22 @@ export async function updateRowsByFilter( } } - const affectedRowIds = await persistBulkUpdateBatch( + const persisted = await persistBulkUpdateBatch({ table, - matchingRows, + rows: matchingRows, + patch: data.data, patchJson, + filterClause, now, - data.secretProvenance - ) + secretProvenance: data.secretProvenance, + requestId, + }) + const { affectedRowIds } = persisted logger.info(`[${requestId}] Updated ${affectedRowIds.length} rows in table ${table.id}`) dispatchBulkUpdateEffects( table, - matchingRows, + persisted.rows, affectedRowIds, data.data, now, From e0a5c39c6834ae8a5afb2adf24009faa2bc8e842 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Tue, 11 Aug 2026 17:26:16 -0700 Subject: [PATCH 7/7] fix(auth): project legacy knowledge audits --- .../authorized-workspace-use-case.ts | 16 +++++----- apps/sim/lib/core/application/index.ts | 1 + .../authorized-knowledge-use-case.ts | 17 +++++++++- .../knowledge/application/documents.test.ts | 31 +++++++++++++++++++ apps/sim/lib/mcp/middleware.ts | 2 -- 5 files changed, 56 insertions(+), 11 deletions(-) diff --git a/apps/sim/lib/core/application/authorized-workspace-use-case.ts b/apps/sim/lib/core/application/authorized-workspace-use-case.ts index ff0fc8faecb..bc3d20c14bb 100644 --- a/apps/sim/lib/core/application/authorized-workspace-use-case.ts +++ b/apps/sim/lib/core/application/authorized-workspace-use-case.ts @@ -75,16 +75,17 @@ function isAuthorizationOptionsResolver< return typeof options === 'function' } -function recordProjectedAuditEntries( +export function recordProjectedUseCaseAuditEntries( operation: O, - context: WorkspaceAuthorizationContext, - attribution: PrincipalAuditAttribution, + workspaceId: string | null | undefined, + principal: PrincipalForOperation, request: OrchestrationRequestContext | undefined, entries: readonly WorkspaceUseCaseAuditEntry[] ): void { + const attribution: PrincipalAuditAttribution = resolvePrincipalAuditAttribution(principal) for (const entry of entries) { recordAudit({ - workspaceId: context.workspaceId, + workspaceId, actorId: attribution.actorId, actorName: attribution.actorName, action: entry.action, @@ -135,11 +136,10 @@ export function defineAuthorizedWorkspaceUseCase< if (projectedAudit !== undefined) { const auditEntries = Array.isArray(projectedAudit) ? projectedAudit : [projectedAudit] if (auditEntries.length > 0) { - const auditAttribution = resolvePrincipalAuditAttribution(principal) - recordProjectedAuditEntries( + recordProjectedUseCaseAuditEntries( definition.operation, - context, - auditAttribution, + context.workspaceId, + principal, request, auditEntries ) diff --git a/apps/sim/lib/core/application/index.ts b/apps/sim/lib/core/application/index.ts index 7693dbdb111..8617e151734 100644 --- a/apps/sim/lib/core/application/index.ts +++ b/apps/sim/lib/core/application/index.ts @@ -3,6 +3,7 @@ export { type AuthorizedWorkspaceUseCaseDefinition, type AuthorizedWorkspaceUseCaseResultContext, defineAuthorizedWorkspaceUseCase, + recordProjectedUseCaseAuditEntries, type WorkspaceUseCaseAuditEntry, } from '@/lib/core/application/authorized-workspace-use-case' export type { diff --git a/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts b/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts index 58dfa02e432..bf1e3659c38 100644 --- a/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts +++ b/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts @@ -3,6 +3,7 @@ import { defineAuthorizedWorkspaceUseCase, type OperationUseCase, type PrincipalForOperation, + recordProjectedUseCaseAuditEntries, requireAllowedWorkspacePrincipal, type WorkspaceOperation, type WorkspaceUseCaseAuditEntry, @@ -131,7 +132,21 @@ export function defineAuthorizedKnowledgeUseCase< } const executionContext = { principal, input, context, request } const result = await definition.execute(executionContext) - await definition.afterSuccess?.({ ...executionContext, result }) + const resultContext = { ...executionContext, result } + const projectedAudit = definition.projectAudit?.(resultContext) + if (projectedAudit !== undefined) { + const auditEntries = Array.isArray(projectedAudit) ? projectedAudit : [projectedAudit] + if (auditEntries.length > 0) { + recordProjectedUseCaseAuditEntries( + definition.operation, + context.workspaceId, + principal, + request, + auditEntries + ) + } + } + await definition.afterSuccess?.(resultContext) return result } diff --git a/apps/sim/lib/knowledge/application/documents.test.ts b/apps/sim/lib/knowledge/application/documents.test.ts index e5cb922c47f..21fd1734e69 100644 --- a/apps/sim/lib/knowledge/application/documents.test.ts +++ b/apps/sim/lib/knowledge/application/documents.test.ts @@ -195,6 +195,37 @@ describe('knowledge document application use cases', () => { expect(mocks.recordAudit).not.toHaveBeenCalled() }) + it('projects mutation audit entries for an owning legacy personal principal', async () => { + mocks.resolveDocument.mockResolvedValueOnce({ + workspaceId: undefined, + legacyPersonalOwnerUserId: 'user-1', + knowledgeBaseId: 'legacy-knowledge', + knowledgeBase: { id: 'legacy-knowledge', name: 'Personal docs', userId: 'user-1' }, + documentId: document.id, + document: { ...document, knowledgeBaseId: 'legacy-knowledge' }, + }) + + await deleteKnowledgeDocument.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { knowledgeBaseId: 'legacy-knowledge', documentId: document.id, source: 'legacy' }, + }) + + expect(mocks.resolvePermission).not.toHaveBeenCalled() + expect(mocks.recordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + workspaceId: undefined, + actorId: 'user-1', + action: 'document.deleted', + resourceId: document.id, + metadata: expect.objectContaining({ + operation: 'knowledge.documents.delete', + knowledgeBaseId: 'legacy-knowledge', + actor: { kind: 'session', userId: 'user-1' }, + }), + }) + ) + }) + it('conceals legacy personal documents from a non-owner', async () => { mocks.resolveKnowledgeBase.mockResolvedValueOnce({ workspaceId: undefined, diff --git a/apps/sim/lib/mcp/middleware.ts b/apps/sim/lib/mcp/middleware.ts index 17c3cc7f6cf..6987f57e960 100644 --- a/apps/sim/lib/mcp/middleware.ts +++ b/apps/sim/lib/mcp/middleware.ts @@ -24,7 +24,6 @@ export interface McpAuthContext { userEmail?: string | null authType?: AuthTypeValue workspaceId: string - canWrite: boolean requestId: string /** * The caller's resolved workspace permission, which satisfies but may exceed @@ -203,7 +202,6 @@ async function validateMcpAuth( userEmail: auth.userEmail, authType: auth.authType, workspaceId, - canWrite: permissionSatisfies(userPermissions as PermissionType, 'write'), requestId, permission: userPermissions, },