Skip to content

Commit 76318e4

Browse files
authored
fix(fork): preserve folder structure across a fork edge for files, tables, and knowledge bases (#6752)
* fix(fork): carry folder structure across a fork edge for files, tables, and KBs Only workflow folders were mirrored into the target workspace on fork create and on sync. Copied files, tables, and knowledge bases were written with a hardcoded `folderId: null`, so a push or pull flattened them all into the target root and lost the source's grouping — visible as a fork sync that drops folder structure when copying files to the parent. `resolveForkFolderMapping` already did the real work (prune to folders holding copied content plus ancestors, reuse same-named target folders, remap parentId), but was pinned to `resourceType: 'workflow'` on both reads and on the folder-ceiling check. Parameterize it by resource type and run it per family, threading the resulting map into each copy instead of nulling. The four folder-bearing families own disjoint trees and folder ids are globally unique, so the per-family maps merge cleanly for the `sim:folder/<id>` content rewrite, which previously resolved only for workflow folders. Existing forks are healed on their next sync rather than by a migration: `rehomeFlattenedForkResources` re-homes mapped files/tables/KBs whose target `folder_id` is still NULL — the exact signature of the old flattening — so a placement chosen in the target is never overwritten and the pass converges to a no-op. `BlobCopyTask.targetFolderId` is optional so tasks queued by an earlier deploy replay at the root exactly as before. * refactor(fork): page the re-home lookups and reuse the plan's identity rows Self-review of the folder-transit change surfaced two scaling problems in the re-home pass, both of which grow with the size of the fork edge rather than the size of the sync: - The resource lookups built `IN (...)` lists straight from the edge's mapping rows, so a large fork could hand Postgres a list approaching the bind-parameter ceiling and a pathological query plan. Page them at 500, matching the paging the rest of the fork copy already uses. - The pass re-read the whole edge mapping via `getEdgeMappingRows`, which the promote plan had already loaded in the same transaction — a second full load of identical rows. Expose them on `ForkPromotePlan` and pass them in, which also drops a mock from the re-home tests. Also tally moved rows from `returning()` rather than the planned batch size, so the log line reports what the `folder_id IS NULL` guard actually wrote instead of what was attempted. * fix(fork): drop the sync-time re-home pass, keep folder transit forward-only Review surfaced three findings and every one of them was in the re-home pass, none in the forward-looking fix: - It keyed mapping orientation off `direction`, but the promote route resolves the edge from whichever workspace the caller is acting in, so a caller in the PARENT pushing to its child is `direction: 'push'` with the parent as source. The plan derives this as `sourceWorkspaceId === edge.parentWorkspaceId` for exactly that reason. - Moving a file into a mirrored folder can violate `workspace_files_workspace_folder_name_active_unique`, which would abort the whole promote transaction and take the workflow sync down with it. - `folder_id IS NULL` cannot distinguish "flattened by the old copy" from "the user moved this to the root", so the pass re-applied on every sync and would fight a deliberate placement indefinitely. The first two are fixable; the third is not without a one-time marker per edge, which means a migration. A heal that re-applies forever is worse than no heal, so remove the pass entirely rather than ship it half-right. Folder structure now transits correctly from this point forward, which is the actual reported bug; healing already-flattened resources can be a separate change with a marker to make it run exactly once. Reverts the `ForkPromotePlan.mappingRows` field with it — it existed only to feed this pass.
1 parent 0077f6f commit 76318e4

10 files changed

Lines changed: 281 additions & 28 deletions

File tree

apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/**
22
* @vitest-environment node
33
*/
4+
import { folder as folderTable } from '@sim/db/schema'
45
import {
56
dbChainMockFns,
67
resetDbChainMock,
@@ -237,4 +238,95 @@ describe('planForkFileCopies', () => {
237238
})
238239
expect(tx.insert).not.toHaveBeenCalled()
239240
})
241+
242+
it('mirrors the source file-folder subtree and places each copy inside it', async () => {
243+
const sourceMeta = {
244+
id: 'wf_src1',
245+
key: 'workspace/src-ws/1-abc-a.txt',
246+
userId: 'uploader-1',
247+
workspaceId: 'src-ws',
248+
folderId: 'child-folder',
249+
context: 'workspace',
250+
chatId: null,
251+
originalName: 'a.txt',
252+
displayName: null,
253+
contentType: 'text/plain',
254+
size: 4321,
255+
deletedAt: null,
256+
uploadedAt: new Date('2026-01-01'),
257+
updatedAt: new Date('2026-01-01'),
258+
contentUpdatedAt: new Date('2026-01-01'),
259+
}
260+
// A two-level source tree; only the branch holding the copied file is mirrored.
261+
const sourceFolders = [
262+
{
263+
id: 'root-folder',
264+
name: 'Reports',
265+
parentId: null,
266+
workspaceId: 'src-ws',
267+
resourceType: 'file',
268+
deletedAt: null,
269+
},
270+
{
271+
id: 'child-folder',
272+
name: 'Q1',
273+
parentId: 'root-folder',
274+
workspaceId: 'src-ws',
275+
resourceType: 'file',
276+
deletedAt: null,
277+
},
278+
{
279+
id: 'unrelated',
280+
name: 'Archive',
281+
parentId: null,
282+
workspaceId: 'src-ws',
283+
resourceType: 'file',
284+
deletedAt: null,
285+
},
286+
]
287+
const insertedFolders: Array<Record<string, unknown>> = []
288+
let folderSelectCall = 0
289+
const tx = {
290+
select: vi.fn(() => ({
291+
from: (table: unknown) => ({
292+
where: () => {
293+
if (table !== folderTable) return Promise.resolve([sourceMeta])
294+
// First folder read is the source tree; the second is the (empty) target tree.
295+
return Promise.resolve(folderSelectCall++ === 0 ? sourceFolders : [])
296+
},
297+
}),
298+
})),
299+
insert: vi.fn(() => ({
300+
values: (rows: Array<Record<string, unknown>>) => {
301+
insertedFolders.push(...rows)
302+
return Promise.resolve()
303+
},
304+
})),
305+
} as unknown as DbOrTx
306+
307+
const result = await planForkFileCopies({
308+
tx,
309+
sourceWorkspaceId: 'src-ws',
310+
childWorkspaceId: 'child-ws',
311+
userId: 'user-1',
312+
fileIds: ['wf_src1'],
313+
now: new Date('2026-02-01'),
314+
})
315+
316+
// The file's folder and its ancestor are recreated; the unrelated branch is pruned.
317+
expect(insertedFolders).toHaveLength(2)
318+
const byName = new Map(insertedFolders.map((row) => [row.name, row]))
319+
expect(byName.has('Archive')).toBe(false)
320+
const newRoot = byName.get('Reports')!
321+
const newChild = byName.get('Q1')!
322+
expect(newRoot).toMatchObject({ parentId: null, workspaceId: 'child-ws' })
323+
// Nesting survives: the copied child points at the copied parent, not the source's.
324+
expect(newChild.parentId).toBe(newRoot.id)
325+
expect(newChild.id).not.toBe('child-folder')
326+
327+
// The copied file lands in the mirrored folder rather than the target root.
328+
expect(result.blobTasks[0].targetFolderId).toBe(newChild.id)
329+
expect(result.folderIdMap.get('child-folder')).toBe(newChild.id)
330+
expect(result.folderIdMap.get('root-folder')).toBe(newRoot.id)
331+
})
240332
})

apps/sim/ee/workspace-forking/lib/copy/copy-files.ts

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
} from '@/lib/uploads/core/storage-service'
2020
import type { StorageContext } from '@/lib/uploads/shared/types'
2121
import { MAX_FILE_SIZE } from '@/lib/uploads/utils/validation'
22+
import { resolveForkFolderMapping } from '@/ee/workspace-forking/lib/copy/copy-workflows'
2223
import {
2324
type ForkContentRefMaps,
2425
rewriteForkContentRefs,
@@ -55,6 +56,13 @@ export interface BlobCopyTask {
5556
displayName: string | null
5657
userId: string
5758
workspaceId: string
59+
/**
60+
* Target file-folder id, already created inside the copy transaction by
61+
* {@link resolveForkFolderMapping}. Optional because tasks queued by an earlier deploy have
62+
* no such field: those replay as `undefined` and finalize at the target root, exactly as
63+
* they did before folder structure transited a fork edge.
64+
*/
65+
targetFolderId?: string | null
5866
}
5967

6068
export interface PlanForkFileCopiesResult {
@@ -74,6 +82,11 @@ export interface PlanForkFileCopiesResult {
7482
idMap: Map<string, string>
7583
/** Blob duplications plus deferred metadata to finalize after the fork transaction commits. */
7684
blobTasks: BlobCopyTask[]
85+
/**
86+
* source file-folder id -> target file-folder id for the mirrored subtree. Merged into the
87+
* content-ref maps so `sim:folder/<id>` mentions inside copied bodies resolve to the copy.
88+
*/
89+
folderIdMap: Map<string, string>
7790
}
7891

7992
async function getFinalizedFileCopies(
@@ -124,7 +137,9 @@ export async function planForkFileCopies(params: {
124137
const keyMap = new Map<string, string>()
125138
const idMap = new Map<string, string>()
126139
const blobTasks: BlobCopyTask[] = []
127-
if (fileIds.length === 0 && fileKeys.length === 0) return { keyMap, idMap, blobTasks }
140+
let folderIdMap = new Map<string, string>()
141+
if (fileIds.length === 0 && fileKeys.length === 0)
142+
return { keyMap, idMap, blobTasks, folderIdMap }
128143

129144
// Match by id and/or storage key (OR'd) so either selection shape resolves to the same
130145
// source rows. Batch the metadata read (one query for all selected files): non-deleted,
@@ -148,6 +163,19 @@ export async function planForkFileCopies(params: {
148163
)
149164
)
150165

166+
// Mirror the file-folder subtree holding the selected files (plus ancestors) into the target
167+
// and place each copy inside it. Scoped to `resourceType: 'file'`: file folders are a tree of
168+
// their own, disjoint from the workflow folders the workflow copy mirrors.
169+
folderIdMap = await resolveForkFolderMapping({
170+
tx,
171+
sourceWorkspaceId,
172+
targetWorkspaceId: childWorkspaceId,
173+
userId,
174+
now: params.now,
175+
resourceType: 'file',
176+
contentFolderIds: metas.map((meta) => meta.folderId),
177+
})
178+
151179
for (const meta of metas) {
152180
const childFileId = generateId()
153181
// Use the canonical workspace-file key (`workspace/{id}/...`) so the file-serve
@@ -168,10 +196,13 @@ export async function planForkFileCopies(params: {
168196
displayName: meta.displayName,
169197
userId,
170198
workspaceId: childWorkspaceId,
199+
// An unmapped folder (pruned, or archived mid-copy) re-roots the file, matching how a
200+
// copied workflow falls back to the target root.
201+
targetFolderId: meta.folderId ? (folderIdMap.get(meta.folderId) ?? null) : null,
171202
})
172203
}
173204

174-
return { keyMap, idMap, blobTasks }
205+
return { keyMap, idMap, blobTasks, folderIdMap }
175206
}
176207

177208
/**
@@ -269,7 +300,7 @@ export async function executeForkFileBlobCopies(
269300
key: task.targetKey,
270301
userId: task.userId,
271302
workspaceId: task.workspaceId,
272-
folderId: null,
303+
folderId: task.targetFolderId ?? null,
273304
context: task.context,
274305
chatId: null,
275306
originalName: task.fileName,
@@ -312,7 +343,7 @@ export async function executeForkFileBlobCopies(
312343
.update(workspaceFiles)
313344
.set({
314345
userId: task.userId,
315-
folderId: null,
346+
folderId: task.targetFolderId ?? null,
316347
context: task.context,
317348
chatId: null,
318349
originalName: task.fileName,

apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @vitest-environment node
33
*/
44

5+
import { folder as folderTable } from '@sim/db/schema'
56
import { sha256Hex } from '@sim/security/hash'
67
import {
78
dbChainMockFns,
@@ -1343,12 +1344,31 @@ describe('copyForkResourceContainers skill copy', () => {
13431344

13441345
describe('copyForkResourceContainers knowledge-base tag definitions', () => {
13451346
/** Sequential tx mock: each select resolves the next queued row set; inserts are captured per call. */
1346-
function makeKbTx(selects: Array<Array<Record<string, unknown>>>) {
1347+
/**
1348+
* Sequential tx mock over the KB-copy selects, with the folder-mirroring reads served
1349+
* separately: the copy resolves the source KB folder subtree before inserting, and dispatching
1350+
* on the queried table keeps the queue positional over the KB selects alone instead of
1351+
* silently shifting whenever that mapping issues a query.
1352+
*/
1353+
function makeKbTx(
1354+
selects: Array<Array<Record<string, unknown>>>,
1355+
sourceFolders: Array<Record<string, unknown>> = []
1356+
) {
13471357
let call = 0
1358+
// The mapper reads the source tree first, then the target's; serving the same rows to both
1359+
// would make every source folder look already-present and suppress the mirroring.
1360+
let folderCall = 0
13481361
const inserts: Array<Array<Record<string, unknown>>> = []
13491362
const tx = {
13501363
select: () => ({
1351-
from: () => ({ where: () => Promise.resolve(selects[call++] ?? []) }),
1364+
from: (table: unknown) => ({
1365+
where: () => {
1366+
if (table === folderTable) {
1367+
return Promise.resolve(folderCall++ === 0 ? sourceFolders : [])
1368+
}
1369+
return Promise.resolve(selects[call++] ?? [])
1370+
},
1371+
}),
13521372
}),
13531373
insert: () => ({
13541374
values: (rows: Array<Record<string, unknown>>) => {
@@ -1437,6 +1457,45 @@ describe('copyForkResourceContainers knowledge-base tag definitions', () => {
14371457
// Only the KB row itself is inserted - no empty tag-definition insert.
14381458
expect(inserts).toHaveLength(1)
14391459
})
1460+
1461+
it('mirrors the source knowledge-base folder and copies the KB into it, not the target root', async () => {
1462+
const foldered = { ...sourceBase, folderId: 'kb-folder' }
1463+
const { tx, inserts } = makeKbTx(
1464+
[[foldered], []],
1465+
[
1466+
{
1467+
id: 'kb-folder',
1468+
name: 'Policies',
1469+
parentId: null,
1470+
workspaceId: 'src-ws',
1471+
resourceType: 'knowledge_base',
1472+
deletedAt: null,
1473+
},
1474+
]
1475+
)
1476+
1477+
await copyForkResourceContainers({
1478+
tx,
1479+
sourceWorkspaceId: 'src-ws',
1480+
childWorkspaceId: 'child-ws',
1481+
userId: 'user-1',
1482+
now: new Date(),
1483+
selection: kbSelection,
1484+
workflowIdMap: new Map(),
1485+
documentMappingContext: { edgeChildWorkspaceId: 'child-ws', sourceIsParent: true },
1486+
})
1487+
1488+
// insert #0 is the mirrored folder, #1 the KB row placed inside it.
1489+
const newFolder = inserts[0][0]
1490+
expect(newFolder).toMatchObject({
1491+
name: 'Policies',
1492+
workspaceId: 'child-ws',
1493+
resourceType: 'knowledge_base',
1494+
})
1495+
// A fresh id: reusing the source's would point the child KB at a folder it cannot see.
1496+
expect(newFolder.id).not.toBe('kb-folder')
1497+
expect(inserts[1][0].folderId).toBe(newFolder.id)
1498+
})
14401499
})
14411500

14421501
describe('planForkMappedKbDocumentCopies', () => {

apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ import {
7171
recordKnowledgeBaseFileOwnership,
7272
} from '@/lib/uploads/server/metadata'
7373
import { MAX_FILE_SIZE } from '@/lib/uploads/utils/validation'
74+
import { resolveForkFolderMapping } from '@/ee/workspace-forking/lib/copy/copy-workflows'
7475
import {
7576
deleteCopiedResourceMappingsByTargets,
7677
type ForkMappingUpsert,
@@ -333,6 +334,11 @@ export interface CopyResourcesResult {
333334
contentPlan: ForkContentPlan
334335
/** Names of the copied resources, by kind, for the fork report breakdown. */
335336
names: ForkCopiedResourceNames
337+
/**
338+
* source folder id -> target folder id for every family mirrored here (tables, knowledge
339+
* bases). Merged by the caller with the workflow and file maps for content-ref rewriting.
340+
*/
341+
folderIdMap: Map<string, string>
336342
}
337343

338344
function setId(idMap: Map<ForkResourceType, Map<string, string>>, type: ForkResourceType) {
@@ -371,6 +377,12 @@ export async function copyForkResourceContainers(
371377
const resolveEnvName = params.resolveEnvName
372378
const idMap = new Map<ForkResourceType, Map<string, string>>()
373379
const mappingEntries: ForkMappingUpsert[] = []
380+
/**
381+
* Mirrored folder ids across every family copied here. Table and knowledge-base folders live
382+
* in disjoint trees, and folder ids are globally unique, so merging them into one map is
383+
* unambiguous and lets callers rewrite `sim:folder/<id>` refs in a single pass.
384+
*/
385+
const folderIdMap = new Map<string, string>()
374386
const contentPlan: ForkContentPlan = {
375387
sourceWorkspaceId,
376388
childWorkspaceId,
@@ -618,6 +630,17 @@ export async function copyForkResourceContainers(
618630
isNull(userTableDefinitions.archivedAt)
619631
)
620632
)
633+
const tableFolderIdMap = await resolveForkFolderMapping({
634+
tx,
635+
sourceWorkspaceId,
636+
targetWorkspaceId: childWorkspaceId,
637+
userId,
638+
now,
639+
resourceType: 'table',
640+
contentFolderIds: definitions.map((definition) => definition.folderId),
641+
})
642+
for (const [source, target] of tableFolderIdMap) folderIdMap.set(source, target)
643+
621644
const inserts: (typeof userTableDefinitions.$inferInsert)[] = []
622645
for (const definition of definitions) {
623646
const childTableId = generateId()
@@ -631,13 +654,13 @@ export async function copyForkResourceContainers(
631654
id: childTableId,
632655
workspaceId: childWorkspaceId,
633656
/**
634-
* Folders never transit a fork edge. `folder_id` is a global id with no workspace in
635-
* it, so the spread above would leave the child's table pointing at a folder owned by
636-
* the SOURCE workspace — invisible in the fork, and mutated from under it if the
637-
* source later deletes that folder (`ON DELETE SET NULL`). Forked tables land at the
638-
* root, like forked files already do.
657+
* `folder_id` is a global id with no workspace in it, so the spread above would leave
658+
* the child's table pointing at a folder owned by the SOURCE workspace — invisible in
659+
* the fork, and mutated from under it if the source later deletes that folder
660+
* (`ON DELETE SET NULL`). Remap it onto the mirrored target subtree instead; an
661+
* unmapped folder re-roots the table.
639662
*/
640-
folderId: null,
663+
folderId: definition.folderId ? (tableFolderIdMap.get(definition.folderId) ?? null) : null,
641664
schema: remappedSchema,
642665
createdBy: userId,
643666
rowsVersion: 0,
@@ -674,6 +697,17 @@ export async function copyForkResourceContainers(
674697
isNull(knowledgeBase.deletedAt)
675698
)
676699
)
700+
const kbFolderIdMap = await resolveForkFolderMapping({
701+
tx,
702+
sourceWorkspaceId,
703+
targetWorkspaceId: childWorkspaceId,
704+
userId,
705+
now,
706+
resourceType: 'knowledge_base',
707+
contentFolderIds: bases.map((base) => base.folderId),
708+
})
709+
for (const [source, target] of kbFolderIdMap) folderIdMap.set(source, target)
710+
677711
const inserts: (typeof knowledgeBase.$inferInsert)[] = []
678712
const kbEntryBySourceId = new Map<string, ForkContentKbEntry>()
679713
for (const base of bases) {
@@ -682,8 +716,8 @@ export async function copyForkResourceContainers(
682716
...base,
683717
id: childKbId,
684718
workspaceId: childWorkspaceId,
685-
/** Same reasoning as the table copy above: folders do not transit a fork edge. */
686-
folderId: null,
719+
/** Same reasoning as the table copy above: remapped, never carried across verbatim. */
720+
folderId: base.folderId ? (kbFolderIdMap.get(base.folderId) ?? null) : null,
687721
userId,
688722
deletedAt: null,
689723
createdAt: now,
@@ -741,7 +775,7 @@ export async function copyForkResourceContainers(
741775
})
742776
}
743777

744-
return { idMap, mappingEntries, contentPlan, names }
778+
return { idMap, mappingEntries, contentPlan, names, folderIdMap }
745779
}
746780

747781
/**

0 commit comments

Comments
 (0)