diff --git a/apps/sim/lib/copilot/generated/tool-catalog-v1.ts b/apps/sim/lib/copilot/generated/tool-catalog-v1.ts index db0ca729e07..8c188e04b99 100644 --- a/apps/sim/lib/copilot/generated/tool-catalog-v1.ts +++ b/apps/sim/lib/copilot/generated/tool-catalog-v1.ts @@ -439,7 +439,11 @@ export const CreateWorkflow: ToolCatalogEntry = { parameters: { type: 'object', properties: { - folderId: { type: 'string', description: 'Optional folder ID.' }, + folderPath: { + type: 'string', + description: + 'Optional canonical workflow-folder VFS path copied from glob("workflows/**"), for example "workflows/Dream" or "workflows/Client%20Work/Intake". Omit for the workspace root.', + }, name: { type: 'string', description: 'Workflow name.' }, workspaceId: { type: 'string', description: 'Optional workspace ID.' }, }, diff --git a/apps/sim/lib/copilot/generated/tool-schemas-v1.ts b/apps/sim/lib/copilot/generated/tool-schemas-v1.ts index 34d854aa5f1..e8d9f1221f2 100644 --- a/apps/sim/lib/copilot/generated/tool-schemas-v1.ts +++ b/apps/sim/lib/copilot/generated/tool-schemas-v1.ts @@ -221,9 +221,10 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { parameters: { type: 'object', properties: { - folderId: { + folderPath: { type: 'string', - description: 'Optional folder ID.', + description: + 'Optional canonical workflow-folder VFS path copied from glob("workflows/**"), for example "workflows/Dream" or "workflows/Client%20Work/Intake". Omit for the workspace root.', }, name: { type: 'string', diff --git a/apps/sim/lib/copilot/tools/handlers/param-types.ts b/apps/sim/lib/copilot/tools/handlers/param-types.ts index 01544f6587c..16fdbd8bf02 100644 --- a/apps/sim/lib/copilot/tools/handlers/param-types.ts +++ b/apps/sim/lib/copilot/tools/handlers/param-types.ts @@ -32,6 +32,9 @@ export interface GetBlockUpstreamReferencesParams { export interface CreateWorkflowParams { name?: string workspaceId?: string + /** Canonical workflow-folder VFS path, for example `workflows/Dream`. */ + folderPath?: string + /** Legacy executor input. New tool calls use folderPath and resolve the ID internally. */ folderId?: string } diff --git a/apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts index 1720cd1c668..2ec9cb72264 100644 --- a/apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts @@ -124,7 +124,7 @@ vi.mock('../access', () => ({ import { applyCreateWorkflowOutputToContext } from '@/lib/copilot/request/tools/workflow-context' import { performUpdateWorkflow } from '@/lib/workflows/orchestration' -import { verifyFolderWorkspace } from '@/lib/workflows/utils' +import { listFolders, verifyFolderWorkspace } from '@/lib/workflows/utils' import { executeCreateWorkflow, executeMoveWorkflow, @@ -136,6 +136,7 @@ import { } from './mutations' const performUpdateWorkflowMock = vi.mocked(performUpdateWorkflow) +const listFoldersMock = vi.mocked(listFolders) const verifyFolderWorkspaceMock = vi.mocked(verifyFolderWorkspace) const billingAttribution: BillingAttributionSnapshot = { actorUserId: 'user-1', @@ -291,6 +292,7 @@ describe('executeCreateWorkflow billing attribution', () => { payerUsage: { currentUsage: 1, limit: 10 }, }) reserveExecutionSlotMock.mockResolvedValue({ reserved: true, created: true }) + listFoldersMock.mockResolvedValue([]) }) it('ignores legacy description input instead of persisting it', async () => { @@ -320,6 +322,89 @@ describe('executeCreateWorkflow billing attribution', () => { }) }) + it('canonicalizes a workflow-folder VFS path and resolves its internal ID', async () => { + listFoldersMock.mockResolvedValue([ + { folderId: 'folder-dream', folderName: 'Dream', parentId: null }, + { + folderId: 'folder-launch-plans', + folderName: 'Launch Plans', + parentId: 'folder-dream', + }, + ]) + performCreateWorkflowMock.mockResolvedValue({ + success: true, + workflow: { + id: 'created-workflow', + name: 'Created Workflow', + workspaceId: 'workspace-1', + folderId: 'folder-launch-plans', + }, + }) + + const result = await executeCreateWorkflow( + { + name: 'Created Workflow', + workspaceId: 'workspace-1', + folderPath: 'workflows/Dream/Launch%20Plans', + }, + executionContext + ) + + expect(result.success).toBe(true) + expect(performCreateWorkflowMock).toHaveBeenCalledWith({ + userId: 'user-1', + workspaceId: 'workspace-1', + name: 'Created Workflow', + folderId: 'folder-launch-plans', + }) + expect(workflowAuthzMockFns.mockAssertFolderMutable).toHaveBeenCalledWith('folder-launch-plans') + }) + + it('fails clearly when a workflow-folder VFS path does not exist', async () => { + listFoldersMock.mockResolvedValue([ + { folderId: 'folder-existing', folderName: 'Existing', parentId: null }, + ]) + + const result = await executeCreateWorkflow( + { + name: 'Created Workflow', + workspaceId: 'workspace-1', + folderPath: 'workflows/Dream', + }, + executionContext + ) + + expect(result).toEqual({ + success: false, + error: 'Folder not found at workflows/Dream', + }) + expect(performCreateWorkflowMock).not.toHaveBeenCalled() + }) + + it('rejects canonically ambiguous workflow-folder VFS paths', async () => { + listFoldersMock.mockResolvedValue([ + { folderId: 'folder-cafe-nfc', folderName: 'Caf\u00e9', parentId: null }, + { folderId: 'folder-cafe-nfd', folderName: 'Cafe\u0301', parentId: null }, + ]) + + const result = await executeCreateWorkflow( + { + name: 'Created Workflow', + workspaceId: 'workspace-1', + folderPath: 'workflows/Caf%C3%A9', + }, + executionContext + ) + + expect(result).toEqual({ + success: false, + error: + 'Folder path is ambiguous after canonicalization: workflows/Caf%C3%A9. Rename one of the conflicting folders and retry.', + }) + expect(performCreateWorkflowMock).not.toHaveBeenCalled() + expect(workflowAuthzMockFns.mockAssertFolderMutable).not.toHaveBeenCalled() + }) + it('keeps same-workspace creation and subsequent execution on the immutable payer', async () => { const context: ExecutionContext = { ...executionContext, workflowId: '' } performCreateWorkflowMock.mockResolvedValue({ diff --git a/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts index 65d6a84f60e..a6a1d675dc4 100644 --- a/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts +++ b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts @@ -393,9 +393,23 @@ export async function executeCreateWorkflow( } const workspaceId = params?.workspaceId || context.workspaceId || (await getDefaultWorkspaceId(context.userId)) - const folderId = params?.folderId || null await ensureWorkspaceAccess(workspaceId, context.userId, 'write') + + const folderPath = typeof params?.folderPath === 'string' ? params.folderPath.trim() : '' + let folderId = + typeof params?.folderId === 'string' && params.folderId.trim() ? params.folderId.trim() : null + if (folderPath) { + const relativePath = workflowFolderRelativePath(folderPath) + if (!relativePath) { + folderId = null + } else { + const target = resolveFolderIdByPath(folderPath, await loadFolderPathIndex(workspaceId)) + if ('error' in target) return { success: false, error: target.error } + folderId = target.folderId + } + } + await assertFolderMutable(folderId) assertWorkflowMutationNotAborted(context) @@ -1230,40 +1244,57 @@ function workflowFolderRelativePath(rawPath: string): string { return trimmed.startsWith('workflows/') ? trimmed.slice('workflows/'.length) : trimmed } +type FolderPathIndex = Map + /** - * Load a lookup from each folder's canonical encoded VFS path to its id by - * inverting the same {@link buildVfsFolderPathMap} the VFS uses to serve folder - * paths, so a path the agent sees via glob round-trips to the right id. Fetched - * once per manage_folder call and reused across target + parent resolution. + * Load an index from each canonical encoded VFS path to its folder id. A null + * value records that multiple folder ids collapse to the same canonical path, + * so callers can reject the ambiguous path instead of silently choosing one. */ -async function loadFolderPathToIdMap(workspaceId: string): Promise> { - const byPath = new Map() +async function loadFolderPathIndex(workspaceId: string): Promise { + const byPath: FolderPathIndex = new Map() for (const [folderId, encodedPath] of buildVfsFolderPathMap( await listFolders(workspaceId) ).entries()) { - byPath.set(encodedPath, folderId) + if (!byPath.has(encodedPath)) { + byPath.set(encodedPath, folderId) + } else if (byPath.get(encodedPath) !== folderId) { + byPath.set(encodedPath, null) + } } return byPath } -function lookupFolderIdByPath(rawPath: string, byPath: Map): string | null { +function resolveFolderIdByPath( + rawPath: string, + byPath: FolderPathIndex, + label = 'Folder' +): { folderId: string } | { error: string } { const relative = workflowFolderRelativePath(rawPath) - if (!relative) return null - return byPath.get(encodeVfsPathSegments(decodeVfsPathSegments(relative))) ?? null + if (!relative) return { error: `${label} not found at ${rawPath}` } + + const canonicalPath = encodeVfsPathSegments(decodeVfsPathSegments(relative)) + if (!byPath.has(canonicalPath)) return { error: `${label} not found at ${rawPath}` } + + const folderId = byPath.get(canonicalPath) + if (!folderId) { + return { + error: `${label} path is ambiguous after canonicalization: ${rawPath}. Rename one of the conflicting folders and retry.`, + } + } + return { folderId } } /** Resolve the folder a manage_folder op targets, preferring folderId over path. */ async function resolveManageFolderTarget( params: ManageFolderParams, - getFolderPaths: () => Promise> + getFolderPaths: () => Promise ): Promise<{ folderId: string } | { error: string }> { const directId = typeof params.folderId === 'string' ? params.folderId.trim() : '' if (directId) return { folderId: directId } const path = typeof params.path === 'string' ? params.path.trim() : '' if (!path) return { error: 'Provide the folder path (e.g. "workflows/Marketing") or folderId.' } - const folderId = lookupFolderIdByPath(path, await getFolderPaths()) - if (!folderId) return { error: `Folder not found at ${path}` } - return { folderId } + return resolveFolderIdByPath(path, await getFolderPaths()) } /** @@ -1273,16 +1304,16 @@ async function resolveManageFolderTarget( */ async function resolveManageFolderParent( params: ManageFolderParams, - getFolderPaths: () => Promise> + getFolderPaths: () => Promise ): Promise<{ parentId: string | null } | { error: string }> { const directId = typeof params.parentId === 'string' ? params.parentId.trim() : '' if (directId) return { parentId: directId } if (params.parentId === null) return { parentId: null } const dest = typeof params.destinationPath === 'string' ? params.destinationPath.trim() : '' if (!dest || !workflowFolderRelativePath(dest)) return { parentId: null } - const parentId = lookupFolderIdByPath(dest, await getFolderPaths()) - if (!parentId) return { error: `Destination folder not found at ${dest}` } - return { parentId } + const parent = resolveFolderIdByPath(dest, await getFolderPaths(), 'Destination folder') + if ('error' in parent) return parent + return { parentId: parent.folderId } } /** @@ -1302,8 +1333,8 @@ export async function executeManageFolder( // Fetch the workspace folder list at most once, lazily — only when a path // (vs an explicit id) actually needs resolving, and shared across the // target + parent lookups a single move/create performs. - let folderPathsPromise: Promise> | undefined - const getFolderPaths = () => (folderPathsPromise ??= loadFolderPathToIdMap(workspaceId)) + let folderPathsPromise: Promise | undefined + const getFolderPaths = () => (folderPathsPromise ??= loadFolderPathIndex(workspaceId)) switch (operation) { case 'create': { @@ -1318,14 +1349,13 @@ export async function executeManageFolder( name = segments[segments.length - 1] const parentSegments = segments.slice(0, -1) if (parentSegments.length > 0) { - const resolved = lookupFolderIdByPath( + const parent = resolveFolderIdByPath( encodeVfsPathSegments(parentSegments), - await getFolderPaths() + await getFolderPaths(), + 'Parent folder' ) - if (!resolved) { - return { success: false, error: `Parent folder not found for ${path}` } - } - parentId = resolved + if ('error' in parent) return { success: false, error: parent.error } + parentId = parent.folderId } } else { const parent = await resolveManageFolderParent(params, getFolderPaths) diff --git a/bun.lock b/bun.lock index 7e81dd4365f..17ed5e2d22c 100644 --- a/bun.lock +++ b/bun.lock @@ -1,6 +1,5 @@ { "lockfileVersion": 1, - "configVersion": 0, "workspaces": { "": { "name": "simstudio",