Skip to content

Commit 7c2e16c

Browse files
committed
fix bug
1 parent 37a2a73 commit 7c2e16c

2 files changed

Lines changed: 69 additions & 30 deletions

File tree

apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,30 @@ describe('executeCreateWorkflow billing attribution', () => {
381381
expect(performCreateWorkflowMock).not.toHaveBeenCalled()
382382
})
383383

384+
it('rejects canonically ambiguous workflow-folder VFS paths', async () => {
385+
listFoldersMock.mockResolvedValue([
386+
{ folderId: 'folder-cafe-nfc', folderName: 'Caf\u00e9', parentId: null },
387+
{ folderId: 'folder-cafe-nfd', folderName: 'Cafe\u0301', parentId: null },
388+
])
389+
390+
const result = await executeCreateWorkflow(
391+
{
392+
name: 'Created Workflow',
393+
workspaceId: 'workspace-1',
394+
folderPath: 'workflows/Caf%C3%A9',
395+
},
396+
executionContext
397+
)
398+
399+
expect(result).toEqual({
400+
success: false,
401+
error:
402+
'Folder path is ambiguous after canonicalization: workflows/Caf%C3%A9. Rename one of the conflicting folders and retry.',
403+
})
404+
expect(performCreateWorkflowMock).not.toHaveBeenCalled()
405+
expect(workflowAuthzMockFns.mockAssertFolderMutable).not.toHaveBeenCalled()
406+
})
407+
384408
it('keeps same-workspace creation and subsequent execution on the immutable payer', async () => {
385409
const context: ExecutionContext = { ...executionContext, workflowId: '' }
386410
performCreateWorkflowMock.mockResolvedValue({

apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -404,10 +404,9 @@ export async function executeCreateWorkflow(
404404
if (!relativePath) {
405405
folderId = null
406406
} else {
407-
folderId = lookupFolderIdByPath(folderPath, await loadFolderPathToIdMap(workspaceId))
408-
if (!folderId) {
409-
return { success: false, error: `Folder not found at ${folderPath}` }
410-
}
407+
const target = resolveFolderIdByPath(folderPath, await loadFolderPathIndex(workspaceId))
408+
if ('error' in target) return { success: false, error: target.error }
409+
folderId = target.folderId
411410
}
412411
}
413412

@@ -1245,40 +1244,57 @@ function workflowFolderRelativePath(rawPath: string): string {
12451244
return trimmed.startsWith('workflows/') ? trimmed.slice('workflows/'.length) : trimmed
12461245
}
12471246

1247+
type FolderPathIndex = Map<string, string | null>
1248+
12481249
/**
1249-
* Load a lookup from each folder's canonical encoded VFS path to its id by
1250-
* inverting the same {@link buildVfsFolderPathMap} the VFS uses to serve folder
1251-
* paths, so a path the agent sees via glob round-trips to the right id. Fetched
1252-
* once per manage_folder call and reused across target + parent resolution.
1250+
* Load an index from each canonical encoded VFS path to its folder id. A null
1251+
* value records that multiple folder ids collapse to the same canonical path,
1252+
* so callers can reject the ambiguous path instead of silently choosing one.
12531253
*/
1254-
async function loadFolderPathToIdMap(workspaceId: string): Promise<Map<string, string>> {
1255-
const byPath = new Map<string, string>()
1254+
async function loadFolderPathIndex(workspaceId: string): Promise<FolderPathIndex> {
1255+
const byPath: FolderPathIndex = new Map()
12561256
for (const [folderId, encodedPath] of buildVfsFolderPathMap(
12571257
await listFolders(workspaceId)
12581258
).entries()) {
1259-
byPath.set(encodedPath, folderId)
1259+
if (!byPath.has(encodedPath)) {
1260+
byPath.set(encodedPath, folderId)
1261+
} else if (byPath.get(encodedPath) !== folderId) {
1262+
byPath.set(encodedPath, null)
1263+
}
12601264
}
12611265
return byPath
12621266
}
12631267

1264-
function lookupFolderIdByPath(rawPath: string, byPath: Map<string, string>): string | null {
1268+
function resolveFolderIdByPath(
1269+
rawPath: string,
1270+
byPath: FolderPathIndex,
1271+
label = 'Folder'
1272+
): { folderId: string } | { error: string } {
12651273
const relative = workflowFolderRelativePath(rawPath)
1266-
if (!relative) return null
1267-
return byPath.get(encodeVfsPathSegments(decodeVfsPathSegments(relative))) ?? null
1274+
if (!relative) return { error: `${label} not found at ${rawPath}` }
1275+
1276+
const canonicalPath = encodeVfsPathSegments(decodeVfsPathSegments(relative))
1277+
if (!byPath.has(canonicalPath)) return { error: `${label} not found at ${rawPath}` }
1278+
1279+
const folderId = byPath.get(canonicalPath)
1280+
if (!folderId) {
1281+
return {
1282+
error: `${label} path is ambiguous after canonicalization: ${rawPath}. Rename one of the conflicting folders and retry.`,
1283+
}
1284+
}
1285+
return { folderId }
12681286
}
12691287

12701288
/** Resolve the folder a manage_folder op targets, preferring folderId over path. */
12711289
async function resolveManageFolderTarget(
12721290
params: ManageFolderParams,
1273-
getFolderPaths: () => Promise<Map<string, string>>
1291+
getFolderPaths: () => Promise<FolderPathIndex>
12741292
): Promise<{ folderId: string } | { error: string }> {
12751293
const directId = typeof params.folderId === 'string' ? params.folderId.trim() : ''
12761294
if (directId) return { folderId: directId }
12771295
const path = typeof params.path === 'string' ? params.path.trim() : ''
12781296
if (!path) return { error: 'Provide the folder path (e.g. "workflows/Marketing") or folderId.' }
1279-
const folderId = lookupFolderIdByPath(path, await getFolderPaths())
1280-
if (!folderId) return { error: `Folder not found at ${path}` }
1281-
return { folderId }
1297+
return resolveFolderIdByPath(path, await getFolderPaths())
12821298
}
12831299

12841300
/**
@@ -1288,16 +1304,16 @@ async function resolveManageFolderTarget(
12881304
*/
12891305
async function resolveManageFolderParent(
12901306
params: ManageFolderParams,
1291-
getFolderPaths: () => Promise<Map<string, string>>
1307+
getFolderPaths: () => Promise<FolderPathIndex>
12921308
): Promise<{ parentId: string | null } | { error: string }> {
12931309
const directId = typeof params.parentId === 'string' ? params.parentId.trim() : ''
12941310
if (directId) return { parentId: directId }
12951311
if (params.parentId === null) return { parentId: null }
12961312
const dest = typeof params.destinationPath === 'string' ? params.destinationPath.trim() : ''
12971313
if (!dest || !workflowFolderRelativePath(dest)) return { parentId: null }
1298-
const parentId = lookupFolderIdByPath(dest, await getFolderPaths())
1299-
if (!parentId) return { error: `Destination folder not found at ${dest}` }
1300-
return { parentId }
1314+
const parent = resolveFolderIdByPath(dest, await getFolderPaths(), 'Destination folder')
1315+
if ('error' in parent) return parent
1316+
return { parentId: parent.folderId }
13011317
}
13021318

13031319
/**
@@ -1317,8 +1333,8 @@ export async function executeManageFolder(
13171333
// Fetch the workspace folder list at most once, lazily — only when a path
13181334
// (vs an explicit id) actually needs resolving, and shared across the
13191335
// target + parent lookups a single move/create performs.
1320-
let folderPathsPromise: Promise<Map<string, string>> | undefined
1321-
const getFolderPaths = () => (folderPathsPromise ??= loadFolderPathToIdMap(workspaceId))
1336+
let folderPathsPromise: Promise<FolderPathIndex> | undefined
1337+
const getFolderPaths = () => (folderPathsPromise ??= loadFolderPathIndex(workspaceId))
13221338

13231339
switch (operation) {
13241340
case 'create': {
@@ -1333,14 +1349,13 @@ export async function executeManageFolder(
13331349
name = segments[segments.length - 1]
13341350
const parentSegments = segments.slice(0, -1)
13351351
if (parentSegments.length > 0) {
1336-
const resolved = lookupFolderIdByPath(
1352+
const parent = resolveFolderIdByPath(
13371353
encodeVfsPathSegments(parentSegments),
1338-
await getFolderPaths()
1354+
await getFolderPaths(),
1355+
'Parent folder'
13391356
)
1340-
if (!resolved) {
1341-
return { success: false, error: `Parent folder not found for ${path}` }
1342-
}
1343-
parentId = resolved
1357+
if ('error' in parent) return { success: false, error: parent.error }
1358+
parentId = parent.folderId
13441359
}
13451360
} else {
13461361
const parent = await resolveManageFolderParent(params, getFolderPaths)

0 commit comments

Comments
 (0)