Skip to content

Commit 81e052d

Browse files
committed
fix(files): resolve an overwrite target by id, not a joined reference
A slash-delimited reference cannot express a folder whose own name contains a slash. `Q3/Q4` joins to two segments and re-reads as two levels, so the overwrite lookup never found the existing file and the write landed as a duplicate beside it. The guard on folder id and name meant it could not overwrite the WRONG file, which is why this was bounded rather than dangerous. I introduced it: before this branch the segments came only from splitting the file name, so no segment could contain a slash. Prepending the picked folder's decoded segments is what made one able to. When a folder was picked its canonical path is unambiguous, so the target is found inside that folder by name and resolved by id — the same route the named append now takes, using the same authorized expansion. A typed path with no picked folder keeps the joined reference, which is exactly as expressive as the name it came from.
1 parent 94176fc commit 81e052d

2 files changed

Lines changed: 74 additions & 2 deletions

File tree

apps/sim/lib/internal/file/operations.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,52 @@ describe('file manage folder wiring', () => {
458458
expect(String(body.error)).toContain('No file named notes.md in /Reports')
459459
})
460460

461+
/*
462+
* A folder genuinely named `Q3/Q4` is one level, but the joined reference the
463+
* overwrite lookup used re-read it as two — so the existing file was never
464+
* found and the write landed as a duplicate beside it.
465+
*/
466+
it('overwrites inside a folder whose name contains a slash', async () => {
467+
mockListWorkspaceFileFolders.mockResolvedValue({
468+
folders: [
469+
{
470+
id: 'folder-slashy',
471+
parentId: null,
472+
name: 'Q3/Q4',
473+
path: 'Q3\\/Q4',
474+
createdAt: CONTENT_UPDATED_AT,
475+
updatedAt: CONTENT_UPDATED_AT,
476+
},
477+
],
478+
})
479+
mockListAllWorkspaceFiles.mockResolvedValue({
480+
files: [
481+
{ ...workspaceFile('existing-in-slashy'), name: 'notes.md', folderId: 'folder-slashy' },
482+
],
483+
})
484+
mockEnsureWorkspaceFileFolderPath.mockResolvedValue({
485+
folderId: 'folder-slashy',
486+
createdFolderIds: [],
487+
})
488+
489+
await POST(
490+
createMockRequest('POST', {
491+
operation: 'write',
492+
workspaceId: 'workspace-1',
493+
fileName: 'notes.md',
494+
folderPath: '/Q3%2FQ4',
495+
content: 'hello',
496+
overwrite: true,
497+
})
498+
)
499+
500+
// Found by id, not by a reference that would split the folder name in two.
501+
expect(mockResolveWorkspaceFileReference).toHaveBeenCalledWith(
502+
'workspace-1',
503+
'existing-in-slashy'
504+
)
505+
})
506+
461507
it('lists what a folder holds, folders and files together', async () => {
462508
const response = await POST(
463509
createMockRequest('POST', { operation: 'list', workspaceId: 'workspace-1' })

apps/sim/lib/internal/file/operations.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -505,17 +505,42 @@ async function resolveWriteOverwriteTarget(options: {
505505
principal: Principal
506506
workspaceId: string
507507
folderId: string | null
508+
/** Canonical destination when one was picked; unambiguous where a joined reference is not. */
509+
folderPath?: string
508510
folderSegments: string[]
509511
leafName: string
510512
}) {
511-
const { principal, workspaceId, folderId, folderSegments, leafName } = options
513+
const { principal, workspaceId, folderId, folderPath, folderSegments, leafName } = options
514+
/*
515+
* A slash-delimited reference cannot express a folder whose own name contains
516+
* a slash: `Q3/Q4` joins to two segments and re-reads as two levels, so the
517+
* existing file is never found and the write lands as a duplicate. When the
518+
* destination came from a picker it arrives as a canonical path, which is
519+
* unambiguous, so the target is looked up inside that folder by name and
520+
* resolved by the id — the same route a named append takes.
521+
*/
522+
let reference = [...folderSegments, leafName].join('/')
523+
if (folderPath) {
524+
const scoped = await expandFolderPathsToFiles({
525+
principal,
526+
workspaceId,
527+
folderPaths: [folderPath],
528+
includeSubfolders: false,
529+
})
530+
const match = scoped.find(
531+
(file) => file.name === leafName && (file.folderId ?? null) === folderId
532+
)
533+
if (!match) return null
534+
reference = match.id
535+
}
536+
512537
let existing: Awaited<ReturnType<typeof resolveWorkspaceFileReference>>
513538
try {
514539
existing = await resolveWorkspaceFileReference({
515540
principal,
516541
operation: fileOperations.updateContent,
517542
workspaceId,
518-
reference: [...folderSegments, leafName].join('/'),
543+
reference,
519544
})
520545
} catch (error) {
521546
if (error instanceof OrchestrationError && error.code === 'not_found') return null
@@ -1016,6 +1041,7 @@ export async function executeFileManageOperation(
10161041
principal,
10171042
workspaceId,
10181043
folderId: folderId ?? null,
1044+
folderPath,
10191045
folderSegments,
10201046
leafName,
10211047
})

0 commit comments

Comments
 (0)