From bacb1e3ee4cbae662e1e16a08b6bdf43e5c4f2d0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 15 Aug 2026 17:05:37 -0700 Subject: [PATCH 1/3] fix(fork): carry folder structure across a fork edge for files, tables, and KBs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/` 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. --- .../lib/copy/copy-files.test.ts | 92 ++++++ .../workspace-forking/lib/copy/copy-files.ts | 39 ++- .../lib/copy/copy-resources.test.ts | 63 +++- .../lib/copy/copy-resources.ts | 52 +++- .../lib/copy/copy-workflows.ts | 26 +- .../workspace-forking/lib/create-fork.test.ts | 3 + .../ee/workspace-forking/lib/create-fork.ts | 18 +- .../lib/promote/copy-unmapped.test.ts | 4 + .../lib/promote/copy-unmapped.ts | 11 +- .../lib/promote/promote.test.ts | 8 + .../workspace-forking/lib/promote/promote.ts | 17 ++ .../lib/promote/rehome-mapped.test.ts | 236 +++++++++++++++ .../lib/promote/rehome-mapped.ts | 270 ++++++++++++++++++ 13 files changed, 811 insertions(+), 28 deletions(-) create mode 100644 apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.test.ts create mode 100644 apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.ts diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts index 88a57454338..622c01cf582 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts @@ -1,6 +1,7 @@ /** * @vitest-environment node */ +import { folder as folderTable } from '@sim/db/schema' import { dbChainMockFns, resetDbChainMock, @@ -237,4 +238,95 @@ describe('planForkFileCopies', () => { }) expect(tx.insert).not.toHaveBeenCalled() }) + + it('mirrors the source file-folder subtree and places each copy inside it', async () => { + const sourceMeta = { + id: 'wf_src1', + key: 'workspace/src-ws/1-abc-a.txt', + userId: 'uploader-1', + workspaceId: 'src-ws', + folderId: 'child-folder', + context: 'workspace', + chatId: null, + originalName: 'a.txt', + displayName: null, + contentType: 'text/plain', + size: 4321, + deletedAt: null, + uploadedAt: new Date('2026-01-01'), + updatedAt: new Date('2026-01-01'), + contentUpdatedAt: new Date('2026-01-01'), + } + // A two-level source tree; only the branch holding the copied file is mirrored. + const sourceFolders = [ + { + id: 'root-folder', + name: 'Reports', + parentId: null, + workspaceId: 'src-ws', + resourceType: 'file', + deletedAt: null, + }, + { + id: 'child-folder', + name: 'Q1', + parentId: 'root-folder', + workspaceId: 'src-ws', + resourceType: 'file', + deletedAt: null, + }, + { + id: 'unrelated', + name: 'Archive', + parentId: null, + workspaceId: 'src-ws', + resourceType: 'file', + deletedAt: null, + }, + ] + const insertedFolders: Array> = [] + let folderSelectCall = 0 + const tx = { + select: vi.fn(() => ({ + from: (table: unknown) => ({ + where: () => { + if (table !== folderTable) return Promise.resolve([sourceMeta]) + // First folder read is the source tree; the second is the (empty) target tree. + return Promise.resolve(folderSelectCall++ === 0 ? sourceFolders : []) + }, + }), + })), + insert: vi.fn(() => ({ + values: (rows: Array>) => { + insertedFolders.push(...rows) + return Promise.resolve() + }, + })), + } as unknown as DbOrTx + + const result = await planForkFileCopies({ + tx, + sourceWorkspaceId: 'src-ws', + childWorkspaceId: 'child-ws', + userId: 'user-1', + fileIds: ['wf_src1'], + now: new Date('2026-02-01'), + }) + + // The file's folder and its ancestor are recreated; the unrelated branch is pruned. + expect(insertedFolders).toHaveLength(2) + const byName = new Map(insertedFolders.map((row) => [row.name, row])) + expect(byName.has('Archive')).toBe(false) + const newRoot = byName.get('Reports')! + const newChild = byName.get('Q1')! + expect(newRoot).toMatchObject({ parentId: null, workspaceId: 'child-ws' }) + // Nesting survives: the copied child points at the copied parent, not the source's. + expect(newChild.parentId).toBe(newRoot.id) + expect(newChild.id).not.toBe('child-folder') + + // The copied file lands in the mirrored folder rather than the target root. + expect(result.blobTasks[0].targetFolderId).toBe(newChild.id) + expect(result.folderIdMap.get('child-folder')).toBe(newChild.id) + expect(result.folderIdMap.get('root-folder')).toBe(newRoot.id) + }) }) diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts index 1a7dff31853..cfa4888170d 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts @@ -19,6 +19,7 @@ import { } from '@/lib/uploads/core/storage-service' import type { StorageContext } from '@/lib/uploads/shared/types' import { MAX_FILE_SIZE } from '@/lib/uploads/utils/validation' +import { resolveForkFolderMapping } from '@/ee/workspace-forking/lib/copy/copy-workflows' import { type ForkContentRefMaps, rewriteForkContentRefs, @@ -55,6 +56,13 @@ export interface BlobCopyTask { displayName: string | null userId: string workspaceId: string + /** + * Target file-folder id, already created inside the copy transaction by + * {@link resolveForkFolderMapping}. Optional because tasks queued by an earlier deploy have + * no such field: those replay as `undefined` and finalize at the target root, exactly as + * they did before folder structure transited a fork edge. + */ + targetFolderId?: string | null } export interface PlanForkFileCopiesResult { @@ -74,6 +82,11 @@ export interface PlanForkFileCopiesResult { idMap: Map /** Blob duplications plus deferred metadata to finalize after the fork transaction commits. */ blobTasks: BlobCopyTask[] + /** + * source file-folder id -> target file-folder id for the mirrored subtree. Merged into the + * content-ref maps so `sim:folder/` mentions inside copied bodies resolve to the copy. + */ + folderIdMap: Map } async function getFinalizedFileCopies( @@ -124,7 +137,9 @@ export async function planForkFileCopies(params: { const keyMap = new Map() const idMap = new Map() const blobTasks: BlobCopyTask[] = [] - if (fileIds.length === 0 && fileKeys.length === 0) return { keyMap, idMap, blobTasks } + let folderIdMap = new Map() + if (fileIds.length === 0 && fileKeys.length === 0) + return { keyMap, idMap, blobTasks, folderIdMap } // Match by id and/or storage key (OR'd) so either selection shape resolves to the same // source rows. Batch the metadata read (one query for all selected files): non-deleted, @@ -148,6 +163,19 @@ export async function planForkFileCopies(params: { ) ) + // Mirror the file-folder subtree holding the selected files (plus ancestors) into the target + // and place each copy inside it. Scoped to `resourceType: 'file'`: file folders are a tree of + // their own, disjoint from the workflow folders the workflow copy mirrors. + folderIdMap = await resolveForkFolderMapping({ + tx, + sourceWorkspaceId, + targetWorkspaceId: childWorkspaceId, + userId, + now: params.now, + resourceType: 'file', + contentFolderIds: metas.map((meta) => meta.folderId), + }) + for (const meta of metas) { const childFileId = generateId() // Use the canonical workspace-file key (`workspace/{id}/...`) so the file-serve @@ -168,10 +196,13 @@ export async function planForkFileCopies(params: { displayName: meta.displayName, userId, workspaceId: childWorkspaceId, + // An unmapped folder (pruned, or archived mid-copy) re-roots the file, matching how a + // copied workflow falls back to the target root. + targetFolderId: meta.folderId ? (folderIdMap.get(meta.folderId) ?? null) : null, }) } - return { keyMap, idMap, blobTasks } + return { keyMap, idMap, blobTasks, folderIdMap } } /** @@ -269,7 +300,7 @@ export async function executeForkFileBlobCopies( key: task.targetKey, userId: task.userId, workspaceId: task.workspaceId, - folderId: null, + folderId: task.targetFolderId ?? null, context: task.context, chatId: null, originalName: task.fileName, @@ -312,7 +343,7 @@ export async function executeForkFileBlobCopies( .update(workspaceFiles) .set({ userId: task.userId, - folderId: null, + folderId: task.targetFolderId ?? null, context: task.context, chatId: null, originalName: task.fileName, diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts index e15e0055fdf..b965aa0654e 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts @@ -2,6 +2,7 @@ * @vitest-environment node */ +import { folder as folderTable } from '@sim/db/schema' import { sha256Hex } from '@sim/security/hash' import { dbChainMockFns, @@ -1343,12 +1344,31 @@ describe('copyForkResourceContainers skill copy', () => { describe('copyForkResourceContainers knowledge-base tag definitions', () => { /** Sequential tx mock: each select resolves the next queued row set; inserts are captured per call. */ - function makeKbTx(selects: Array>>) { + /** + * Sequential tx mock over the KB-copy selects, with the folder-mirroring reads served + * separately: the copy resolves the source KB folder subtree before inserting, and dispatching + * on the queried table keeps the queue positional over the KB selects alone instead of + * silently shifting whenever that mapping issues a query. + */ + function makeKbTx( + selects: Array>>, + sourceFolders: Array> = [] + ) { let call = 0 + // The mapper reads the source tree first, then the target's; serving the same rows to both + // would make every source folder look already-present and suppress the mirroring. + let folderCall = 0 const inserts: Array>> = [] const tx = { select: () => ({ - from: () => ({ where: () => Promise.resolve(selects[call++] ?? []) }), + from: (table: unknown) => ({ + where: () => { + if (table === folderTable) { + return Promise.resolve(folderCall++ === 0 ? sourceFolders : []) + } + return Promise.resolve(selects[call++] ?? []) + }, + }), }), insert: () => ({ values: (rows: Array>) => { @@ -1437,6 +1457,45 @@ describe('copyForkResourceContainers knowledge-base tag definitions', () => { // Only the KB row itself is inserted - no empty tag-definition insert. expect(inserts).toHaveLength(1) }) + + it('mirrors the source knowledge-base folder and copies the KB into it, not the target root', async () => { + const foldered = { ...sourceBase, folderId: 'kb-folder' } + const { tx, inserts } = makeKbTx( + [[foldered], []], + [ + { + id: 'kb-folder', + name: 'Policies', + parentId: null, + workspaceId: 'src-ws', + resourceType: 'knowledge_base', + deletedAt: null, + }, + ] + ) + + await copyForkResourceContainers({ + tx, + sourceWorkspaceId: 'src-ws', + childWorkspaceId: 'child-ws', + userId: 'user-1', + now: new Date(), + selection: kbSelection, + workflowIdMap: new Map(), + documentMappingContext: { edgeChildWorkspaceId: 'child-ws', sourceIsParent: true }, + }) + + // insert #0 is the mirrored folder, #1 the KB row placed inside it. + const newFolder = inserts[0][0] + expect(newFolder).toMatchObject({ + name: 'Policies', + workspaceId: 'child-ws', + resourceType: 'knowledge_base', + }) + // A fresh id: reusing the source's would point the child KB at a folder it cannot see. + expect(newFolder.id).not.toBe('kb-folder') + expect(inserts[1][0].folderId).toBe(newFolder.id) + }) }) describe('planForkMappedKbDocumentCopies', () => { diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts index 991b1fd7a1d..dc3c9dbcbdd 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts @@ -71,6 +71,7 @@ import { recordKnowledgeBaseFileOwnership, } from '@/lib/uploads/server/metadata' import { MAX_FILE_SIZE } from '@/lib/uploads/utils/validation' +import { resolveForkFolderMapping } from '@/ee/workspace-forking/lib/copy/copy-workflows' import { deleteCopiedResourceMappingsByTargets, type ForkMappingUpsert, @@ -333,6 +334,11 @@ export interface CopyResourcesResult { contentPlan: ForkContentPlan /** Names of the copied resources, by kind, for the fork report breakdown. */ names: ForkCopiedResourceNames + /** + * source folder id -> target folder id for every family mirrored here (tables, knowledge + * bases). Merged by the caller with the workflow and file maps for content-ref rewriting. + */ + folderIdMap: Map } function setId(idMap: Map>, type: ForkResourceType) { @@ -371,6 +377,12 @@ export async function copyForkResourceContainers( const resolveEnvName = params.resolveEnvName const idMap = new Map>() const mappingEntries: ForkMappingUpsert[] = [] + /** + * Mirrored folder ids across every family copied here. Table and knowledge-base folders live + * in disjoint trees, and folder ids are globally unique, so merging them into one map is + * unambiguous and lets callers rewrite `sim:folder/` refs in a single pass. + */ + const folderIdMap = new Map() const contentPlan: ForkContentPlan = { sourceWorkspaceId, childWorkspaceId, @@ -618,6 +630,17 @@ export async function copyForkResourceContainers( isNull(userTableDefinitions.archivedAt) ) ) + const tableFolderIdMap = await resolveForkFolderMapping({ + tx, + sourceWorkspaceId, + targetWorkspaceId: childWorkspaceId, + userId, + now, + resourceType: 'table', + contentFolderIds: definitions.map((definition) => definition.folderId), + }) + for (const [source, target] of tableFolderIdMap) folderIdMap.set(source, target) + const inserts: (typeof userTableDefinitions.$inferInsert)[] = [] for (const definition of definitions) { const childTableId = generateId() @@ -631,13 +654,13 @@ export async function copyForkResourceContainers( id: childTableId, workspaceId: childWorkspaceId, /** - * Folders never transit a fork edge. `folder_id` is a global id with no workspace in - * it, so the spread above would leave the child's table pointing at a folder owned by - * the SOURCE workspace — invisible in the fork, and mutated from under it if the - * source later deletes that folder (`ON DELETE SET NULL`). Forked tables land at the - * root, like forked files already do. + * `folder_id` is a global id with no workspace in it, so the spread above would leave + * the child's table pointing at a folder owned by the SOURCE workspace — invisible in + * the fork, and mutated from under it if the source later deletes that folder + * (`ON DELETE SET NULL`). Remap it onto the mirrored target subtree instead; an + * unmapped folder re-roots the table. */ - folderId: null, + folderId: definition.folderId ? (tableFolderIdMap.get(definition.folderId) ?? null) : null, schema: remappedSchema, createdBy: userId, rowsVersion: 0, @@ -674,6 +697,17 @@ export async function copyForkResourceContainers( isNull(knowledgeBase.deletedAt) ) ) + const kbFolderIdMap = await resolveForkFolderMapping({ + tx, + sourceWorkspaceId, + targetWorkspaceId: childWorkspaceId, + userId, + now, + resourceType: 'knowledge_base', + contentFolderIds: bases.map((base) => base.folderId), + }) + for (const [source, target] of kbFolderIdMap) folderIdMap.set(source, target) + const inserts: (typeof knowledgeBase.$inferInsert)[] = [] const kbEntryBySourceId = new Map() for (const base of bases) { @@ -682,8 +716,8 @@ export async function copyForkResourceContainers( ...base, id: childKbId, workspaceId: childWorkspaceId, - /** Same reasoning as the table copy above: folders do not transit a fork edge. */ - folderId: null, + /** Same reasoning as the table copy above: remapped, never carried across verbatim. */ + folderId: base.folderId ? (kbFolderIdMap.get(base.folderId) ?? null) : null, userId, deletedAt: null, createdAt: now, @@ -741,7 +775,7 @@ export async function copyForkResourceContainers( }) } - return { idMap, mappingEntries, contentPlan, names } + return { idMap, mappingEntries, contentPlan, names, folderIdMap } } /** diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts index cc8da0e3600..711f6db0215 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts @@ -3,6 +3,7 @@ import { createLogger } from '@sim/logger' import { generateId } from '@sim/utils/id' import { isRecordLike } from '@sim/utils/object' import { and, eq, inArray, isNull } from 'drizzle-orm' +import type { FolderResourceType } from '@/lib/api/contracts/folders' import type { DbOrTx } from '@/lib/db/types' import { assertFolderCollectionHasRoom } from '@/lib/folders/queries' import { remapConditionEdgeHandle } from '@/lib/workflows/condition-ids' @@ -44,12 +45,17 @@ interface ResolveForkFolderMappingParams { userId: string now: Date /** - * Source folder ids that will directly hold copied content (workflows); null entries + * Which folder tree to mirror. `folder` rows are one table discriminated by this column, and + * the four folder-bearing families (`workflow`, `file`, `knowledge_base`, `table`) each own a + * disjoint tree, so a mapping run is always scoped to exactly one of them - reading across + * types would alias unrelated same-named folders onto each other. + */ + resourceType: FolderResourceType + /** + * Source folder ids that will directly hold copied content of `resourceType`; null entries * (root-placed content) are ignored. A source folder is copied into the target only when * its subtree contains at least one of these, so a fork/sync never creates folders that - * would end up empty. Copied workspace FILES never influence this set: their folders are a - * separate tree (`folder` rows with `resourceType = 'file'`, which this copy only ever reads - * as `'workflow'`) and are flattened to root by the copy. + * would end up empty. */ contentFolderIds: ReadonlyArray } @@ -61,8 +67,11 @@ interface ResolveForkFolderMappingParams { * parent are reused instead of duplicated. Folders whose subtree holds no copied content are * pruned - never created - though a pruned folder still maps onto an existing target folder * when one matches, so previously-synced content refs keep resolving. Returns a map from - * source folder id to target folder id; a copied workflow whose folder is absent from the + * source folder id to target folder id; copied content whose folder is absent from the * map is placed at the target's root (see {@link copyWorkflowStateIntoTarget}). + * + * Call once per folder-bearing family being copied; the returned maps are disjoint (folder ids + * are globally unique) and safe to merge for content-reference rewriting. */ export async function resolveForkFolderMapping({ tx, @@ -70,6 +79,7 @@ export async function resolveForkFolderMapping({ targetWorkspaceId, userId, now, + resourceType, contentFolderIds, }: ResolveForkFolderMappingParams): Promise> { const map = new Map() @@ -80,7 +90,7 @@ export async function resolveForkFolderMapping({ .where( and( eq(folderTable.workspaceId, sourceWorkspaceId), - eq(folderTable.resourceType, 'workflow'), + eq(folderTable.resourceType, resourceType), isNull(folderTable.deletedAt) ) ) @@ -107,7 +117,7 @@ export async function resolveForkFolderMapping({ .where( and( eq(folderTable.workspaceId, targetWorkspaceId), - eq(folderTable.resourceType, 'workflow'), + eq(folderTable.resourceType, resourceType), isNull(folderTable.deletedAt) ) ) @@ -172,7 +182,7 @@ export async function resolveForkFolderMapping({ * transaction's `lock_timeout`, which the fork sets deliberately, so an ordinary * concurrent `createFolder` can still slip a row in between the count and the insert. */ - await assertFolderCollectionHasRoom(targetWorkspaceId, 'workflow', tx, { + await assertFolderCollectionHasRoom(targetWorkspaceId, resourceType, tx, { additionalRows: newFolders.length, }) await tx.insert(folderTable).values(newFolders) diff --git a/apps/sim/ee/workspace-forking/lib/create-fork.test.ts b/apps/sim/ee/workspace-forking/lib/create-fork.test.ts index 56278fa1b86..00ba481cc4c 100644 --- a/apps/sim/ee/workspace-forking/lib/create-fork.test.ts +++ b/apps/sim/ee/workspace-forking/lib/create-fork.test.ts @@ -134,10 +134,12 @@ describe('createFork storage headroom gate', () => { keyMap: new Map(), idMap: new Map(), blobTasks: [], + folderIdMap: new Map(), }) mockCopyForkResourceContainers.mockResolvedValue({ idMap: new Map(), mappingEntries: [], + folderIdMap: new Map(), contentPlan: { sourceWorkspaceId: 'src-ws', childWorkspaceId: 'child-ws', @@ -224,6 +226,7 @@ describe('createFork storage headroom gate', () => { keyMap: new Map([['workspace/src-ws/a.png', 'workspace/child/a.png']]), idMap: new Map([['file-1', 'file-1-copy']]), blobTasks: [], + folderIdMap: new Map(), }) await createFork(forkParams({ files: ['file-1'] })) diff --git a/apps/sim/ee/workspace-forking/lib/create-fork.ts b/apps/sim/ee/workspace-forking/lib/create-fork.ts index 09821fafa5d..67755d8f1d9 100644 --- a/apps/sim/ee/workspace-forking/lib/create-fork.ts +++ b/apps/sim/ee/workspace-forking/lib/create-fork.ts @@ -228,14 +228,15 @@ export async function createFork(params: CreateForkParams): Promise` mentions in skill/file bodies). // Scoped to the folders that will actually receive a copied workflow (plus ancestors): a // fork copies only DEPLOYED workflows, so folders holding none would be created empty in - // the child and are pruned instead. Copied files don't extend this set - they use the - // separate workspace-file-folder entity and land at the child's root. - const folderIdMap = await resolveForkFolderMapping({ + // the child and are pruned instead. The file/table/knowledge-base trees are mirrored + // separately by their own copies and merged in below. + const workflowFolderIdMap = await resolveForkFolderMapping({ tx, sourceWorkspaceId: source.id, targetWorkspaceId: childWorkspaceId, userId, now, + resourceType: 'workflow', contentFolderIds: deployedWorkflows .filter((wf) => workflowIdMap.has(wf.id)) .map((wf) => wf.folderId), @@ -264,6 +265,17 @@ export async function createFork(params: CreateForkParams): Promise` ref in copied content + * resolves regardless of which family's folder it names. + */ + const folderIdMap = new Map([ + ...workflowFolderIdMap, + ...fileResult.folderIdMap, + ...resourceResult.folderIdMap, + ]) + const resolveCopied = (kind: ForkRemapKind, sourceId: string): string | null => { if (kind === 'file') return fileResult.keyMap.get(sourceId) ?? null const resourceType = FORK_KIND_TO_RESOURCE_TYPE[kind] diff --git a/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.test.ts b/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.test.ts index 250c5fc85d6..c177ceee97d 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.test.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.test.ts @@ -244,6 +244,7 @@ describe('copyPromoteUnmappedResources - files + folder content-refs', () => { vi.clearAllMocks() mockCopyForkResourceContainers.mockResolvedValue({ idMap: new Map(), + folderIdMap: new Map(), mappingEntries: [], contentPlan: { sourceWorkspaceId: 'src-ws', @@ -313,6 +314,7 @@ describe('copyPromoteUnmappedResources - files + folder content-refs', () => { it('copies selected files (keyMap + blobTasks), persists the file mapping, and threads file + folder content-ref maps', async () => { mockPlanForkFileCopies.mockResolvedValue({ keyMap: new Map([['workspace/SRC/a.png', 'workspace/DST/a.png']]), + folderIdMap: new Map(), idMap: new Map([['file-src', 'file-dst']]), blobTasks: [ { @@ -386,6 +388,7 @@ describe('copyPromoteUnmappedResources - files + folder content-refs', () => { // mapping row is what makes the next sync resolve the copy instead of re-offering it. mockCopyForkResourceContainers.mockResolvedValue({ idMap: new Map([['table', new Map([['tbl-unref', 'tbl-copy']])]]), + folderIdMap: new Map(), mappingEntries: [ { resourceType: 'table', parentResourceId: 'tbl-unref', childResourceId: 'tbl-copy' }, ], @@ -409,6 +412,7 @@ describe('copyPromoteUnmappedResources - files + folder content-refs', () => { }) mockPlanForkFileCopies.mockResolvedValue({ keyMap: new Map(), + folderIdMap: new Map(), idMap: new Map(), blobTasks: [], }) diff --git a/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.ts b/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.ts index 4cd8f703258..19269023429 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.ts @@ -173,7 +173,11 @@ export async function copyPromoteUnmappedResources(params: { now: Date selection: PromoteCopySelection workflowIdMap: Map - /** source folder id -> target folder id, so copied skill/markdown bodies rewrite `sim:folder/`. */ + /** + * source workflow-folder id -> target folder id, so copied skill/markdown bodies rewrite + * `sim:folder/`. The file / table / knowledge-base trees are mirrored by the copies run + * here and unioned onto this map before the content rewrite. + */ folderIdMap: Map /** Base resolver (persisted mappings + env identity), used to detect already-mapped KBs (U-docs). */ resolver: ForkReferenceResolver @@ -251,6 +255,7 @@ export async function copyPromoteUnmappedResources(params: { keyMap: new Map(), idMap: new Map(), blobTasks: [] as BlobCopyTask[], + folderIdMap: new Map(), } // U-docs: documents referenced under an already-mapped (not copied this sync) KB. Skip any doc @@ -304,7 +309,9 @@ export async function copyPromoteUnmappedResources(params: { const contentRefMaps = serializeContentRefMaps({ workspaceId: { from: sourceWorkspaceId, to: targetWorkspaceId }, workflows: workflowIdMap, - folders: folderIdMap, + // Workflow folders (mapped by the caller) unioned with the file / table / knowledge-base + // folders this copy mirrored, so a `sim:folder/` ref resolves whichever tree it names. + folders: new Map([...folderIdMap, ...fileResult.folderIdMap, ...result.folderIdMap]), fileKeys: fileResult.keyMap, fileIds: fileResult.idMap, skills: result.idMap.get('skill'), diff --git a/apps/sim/ee/workspace-forking/lib/promote/promote.test.ts b/apps/sim/ee/workspace-forking/lib/promote/promote.test.ts index c44dd9c9e2a..1670968af3c 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/promote.test.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/promote.test.ts @@ -107,6 +107,14 @@ vi.mock('@/ee/workspace-forking/lib/mapping/mapping-store', () => ({ deleteWorkflowIdentityByIds: vi.fn(), upsertEdgeMappings: vi.fn(), })) +// Re-homing has its own suite (`rehome-mapped.test.ts`); stub it here so these promote +// orchestration tests are not coupled to its queries. +vi.mock('@/ee/workspace-forking/lib/promote/rehome-mapped', () => ({ + rehomeFlattenedForkResources: vi.fn(async () => ({ + folderIdMap: new Map(), + rehomed: { file: 0, table: 0, knowledge_base: 0 }, + })), +})) vi.mock('@/ee/workspace-forking/lib/promote/cleared-refs', () => ({ collectForkSyncBlockers: mockCollectBlockers, verifyForkDropAcknowledgments: mockVerifyDrops, diff --git a/apps/sim/ee/workspace-forking/lib/promote/promote.ts b/apps/sim/ee/workspace-forking/lib/promote/promote.ts index 22a25a9e42c..7fab017fe33 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/promote.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/promote.ts @@ -82,6 +82,7 @@ import { type PromoteRunWorkflowSnapshot, upsertPromoteRun, } from '@/ee/workspace-forking/lib/promote/promote-run-store' +import { rehomeFlattenedForkResources } from '@/ee/workspace-forking/lib/promote/rehome-mapped' import { buildForkTriggerPlan, type ForkTriggerMappingInput, @@ -557,9 +558,25 @@ export async function promoteFork(params: PromoteForkParams): Promise item.sourceMeta.folderId), }) + // Heal earlier syncs that landed mapped files/tables/KBs at the target root before folder + // structure transited a fork edge. Touches only still-flattened rows, so it converges to a + // no-op and never overrides a placement chosen in the target. + const rehomeResult = await rehomeFlattenedForkResources({ + tx, + edge, + sourceWorkspaceId, + targetWorkspaceId, + direction, + userId, + now, + requestId, + }) + for (const [source, target] of rehomeResult.folderIdMap) folderIdMap.set(source, target) + let resolver = plan.resolver let copyContentPlan: ForkContentPlan | null = null let copyContentRefMaps: SerializableForkContentRefMaps | null = null diff --git a/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.test.ts b/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.test.ts new file mode 100644 index 00000000000..c866584200a --- /dev/null +++ b/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.test.ts @@ -0,0 +1,236 @@ +/** + * @vitest-environment node + */ +import { folder as folderTable, knowledgeBase, workspaceFiles } from '@sim/db/schema' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { DbOrTx } from '@/lib/db/types' + +const { mockGetEdgeMappingRows } = vi.hoisted(() => ({ + mockGetEdgeMappingRows: vi.fn(), +})) + +vi.mock('@/ee/workspace-forking/lib/mapping/mapping-store', () => ({ + getEdgeMappingRows: mockGetEdgeMappingRows, +})) + +import { rehomeFlattenedForkResources } from '@/ee/workspace-forking/lib/promote/rehome-mapped' + +interface UpdateCall { + table: unknown + values: Record +} + +/** + * Table-dispatched tx mock. Reads are keyed by table (and, for the two-phase folder mapping, + * by call order: source tree first, then target tree) so a test never has to count queries. + */ +function makeTx(rows: { + files?: Record[] + knowledgeBases?: Record[] + sourceFolders?: Record[] +}) { + const updates: UpdateCall[] = [] + const insertedFolders: Record[] = [] + let folderCall = 0 + const tx = { + select: () => ({ + from: (table: unknown) => ({ + where: () => { + if (table === folderTable) { + return Promise.resolve(folderCall++ === 0 ? (rows.sourceFolders ?? []) : []) + } + if (table === workspaceFiles) return Promise.resolve(rows.files ?? []) + if (table === knowledgeBase) return Promise.resolve(rows.knowledgeBases ?? []) + return Promise.resolve([]) + }, + }), + }), + insert: () => ({ + values: (values: Record[]) => { + insertedFolders.push(...values) + return Promise.resolve() + }, + }), + update: (table: unknown) => ({ + set: (values: Record) => ({ + where: () => { + updates.push({ table, values }) + return Promise.resolve() + }, + }), + }), + } + return { tx: tx as unknown as DbOrTx, updates, insertedFolders } +} + +const edge = { + childWorkspaceId: 'child-ws', + parentWorkspaceId: 'parent-ws', +} as Parameters[0]['edge'] + +const baseParams = { + edge, + sourceWorkspaceId: 'child-ws', + targetWorkspaceId: 'parent-ws', + direction: 'push' as const, + userId: 'user-1', + now: new Date('2026-08-15T00:00:00.000Z'), +} + +describe('rehomeFlattenedForkResources', () => { + beforeEach(() => { + vi.clearAllMocks() + mockGetEdgeMappingRows.mockResolvedValue([]) + }) + + it('mirrors the source folder and moves a root-flattened mapped file into it', async () => { + // Push: the child is the source, so the mapping row's child side is the source key. + mockGetEdgeMappingRows.mockResolvedValue([ + { + id: 'map-1', + childWorkspaceId: 'child-ws', + resourceType: 'file', + parentResourceId: 'workspace/parent-ws/a.png', + childResourceId: 'workspace/child-ws/a.png', + }, + ]) + const { tx, updates, insertedFolders } = makeTx({ + // Both the target lookup (flattened row) and the source lookup read this table; the + // rows carry the fields each phase needs. + files: [ + { id: 'file-target', key: 'workspace/parent-ws/a.png', folderId: null }, + { id: 'file-source', key: 'workspace/child-ws/a.png', folderId: 'src-folder' }, + ], + sourceFolders: [ + { + id: 'src-folder', + name: 'Contracts', + parentId: null, + workspaceId: 'child-ws', + resourceType: 'file', + deletedAt: null, + }, + ], + }) + + const result = await rehomeFlattenedForkResources({ ...baseParams, tx }) + + expect(insertedFolders).toHaveLength(1) + expect(insertedFolders[0]).toMatchObject({ + name: 'Contracts', + workspaceId: 'parent-ws', + resourceType: 'file', + }) + const newFolderId = insertedFolders[0].id as string + expect(updates).toHaveLength(1) + expect(updates[0].table).toBe(workspaceFiles) + expect(updates[0].values).toEqual({ folderId: newFolderId }) + expect(result.rehomed.file).toBe(1) + expect(result.folderIdMap.get('src-folder')).toBe(newFolderId) + }) + + it('leaves a resource alone when the source itself sits at the root', async () => { + mockGetEdgeMappingRows.mockResolvedValue([ + { + id: 'map-1', + childWorkspaceId: 'child-ws', + resourceType: 'file', + parentResourceId: 'workspace/parent-ws/a.png', + childResourceId: 'workspace/child-ws/a.png', + }, + ]) + const { tx, updates, insertedFolders } = makeTx({ + files: [ + { id: 'file-target', key: 'workspace/parent-ws/a.png', folderId: null }, + { id: 'file-source', key: 'workspace/child-ws/a.png', folderId: null }, + ], + }) + + const result = await rehomeFlattenedForkResources({ ...baseParams, tx }) + + expect(insertedFolders).toHaveLength(0) + expect(updates).toHaveLength(0) + expect(result.rehomed.file).toBe(0) + }) + + it('never touches a target already placed in a folder, so a deliberate move survives a re-sync', async () => { + mockGetEdgeMappingRows.mockResolvedValue([ + { + id: 'map-1', + childWorkspaceId: 'child-ws', + resourceType: 'knowledge_base', + parentResourceId: 'kb-target', + childResourceId: 'kb-source', + }, + ]) + // The target read filters on `folderId IS NULL`, so an already-placed row is simply absent. + const { tx, updates } = makeTx({ + knowledgeBases: [], + sourceFolders: [ + { + id: 'src-folder', + name: 'Policies', + parentId: null, + workspaceId: 'child-ws', + resourceType: 'knowledge_base', + deletedAt: null, + }, + ], + }) + + const result = await rehomeFlattenedForkResources({ ...baseParams, tx }) + + expect(updates).toHaveLength(0) + expect(result.rehomed.knowledge_base).toBe(0) + }) + + it('no-ops entirely when the edge has no mapped resources', async () => { + const { tx, updates, insertedFolders } = makeTx({}) + + const result = await rehomeFlattenedForkResources({ ...baseParams, tx }) + + expect(updates).toHaveLength(0) + expect(insertedFolders).toHaveLength(0) + expect(result.rehomed).toEqual({ file: 0, table: 0, knowledge_base: 0 }) + }) + + it('orients pull the other way: the parent side is the source', async () => { + mockGetEdgeMappingRows.mockResolvedValue([ + { + id: 'map-1', + childWorkspaceId: 'child-ws', + resourceType: 'file', + parentResourceId: 'workspace/parent-ws/a.png', + childResourceId: 'workspace/child-ws/a.png', + }, + ]) + const { tx, updates } = makeTx({ + files: [ + // On a pull the CHILD is the target, so its key is the one that must still be flattened. + { id: 'file-target', key: 'workspace/child-ws/a.png', folderId: null }, + { id: 'file-source', key: 'workspace/parent-ws/a.png', folderId: 'src-folder' }, + ], + sourceFolders: [ + { + id: 'src-folder', + name: 'Contracts', + parentId: null, + workspaceId: 'parent-ws', + resourceType: 'file', + deletedAt: null, + }, + ], + }) + + const result = await rehomeFlattenedForkResources({ + ...baseParams, + tx, + direction: 'pull', + sourceWorkspaceId: 'parent-ws', + targetWorkspaceId: 'child-ws', + }) + + expect(updates).toHaveLength(1) + expect(result.rehomed.file).toBe(1) + }) +}) diff --git a/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.ts b/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.ts new file mode 100644 index 00000000000..dff5c5ca9b1 --- /dev/null +++ b/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.ts @@ -0,0 +1,270 @@ +import { knowledgeBase, userTableDefinitions, workspaceFiles } from '@sim/db/schema' +import { createLogger } from '@sim/logger' +import { and, eq, inArray, isNull } from 'drizzle-orm' +import type { DbOrTx } from '@/lib/db/types' +import { resolveForkFolderMapping } from '@/ee/workspace-forking/lib/copy/copy-workflows' +import type { ForkEdge } from '@/ee/workspace-forking/lib/lineage/lineage' +import { getEdgeMappingRows } from '@/ee/workspace-forking/lib/mapping/mapping-store' + +const logger = createLogger('WorkspaceForkRehomeMapped') + +/** + * The families whose copies predate folder-structure transit. Workflows are absent on purpose: + * their folder placement has always been remapped, and a workflow sync rewrites `folder_id` on + * every replace, so they are already self-healing. + */ +const REHOMED_FAMILIES = ['file', 'table', 'knowledge_base'] as const + +export interface RehomeMappedResult { + /** source folder id -> target folder id for any subtree mirrored while re-homing. */ + folderIdMap: Map + /** Rows re-homed per family, for the promote log line. */ + rehomed: Record<(typeof REHOMED_FAMILIES)[number], number> +} + +/** + * Re-home already-mapped files / tables / knowledge bases that an earlier sync flattened to the + * target root, mirroring the source folder subtree they belong to and moving them into it. + * + * Deliberately narrow: only rows whose target `folder_id` is currently NULL are touched. Copies + * made before folder structure transited a fork edge all landed at the root, so NULL is the exact + * signature of the damage - and skipping non-NULL rows means a placement the user chose in the + * target is never overwritten by a later sync. A resource that is legitimately at the source root + * has nothing to map and stays put, so the pass converges to a no-op once healed. + * + * Runs inside the promote transaction, after the folder mapping for workflows, so a refusal from + * the folder-ceiling check rolls the whole sync back rather than leaving a half-moved tree. + */ +export async function rehomeFlattenedForkResources(params: { + tx: DbOrTx + edge: ForkEdge + sourceWorkspaceId: string + targetWorkspaceId: string + direction: 'push' | 'pull' + userId: string + now: Date + requestId?: string +}): Promise { + const { tx, edge, sourceWorkspaceId, targetWorkspaceId, direction, userId, now } = params + const folderIdMap = new Map() + const rehomed = { file: 0, table: 0, knowledge_base: 0 } + + const mappingRows = await getEdgeMappingRows(tx, edge.childWorkspaceId) + // A pull copies parent -> child, so the parent side is the source; a push reverses it. + const sourceIsParent = direction === 'pull' + + for (const family of REHOMED_FAMILIES) { + /** + * Source resource key -> target resource key for this family. Files map by STORAGE KEY (that + * is what `file-upload` subblocks reference and what the mapping rows store); tables and + * knowledge bases map by row id. + */ + const sourceToTarget = new Map() + for (const row of mappingRows) { + if (row.resourceType !== family || !row.childResourceId) continue + const source = sourceIsParent ? row.parentResourceId : row.childResourceId + const target = sourceIsParent ? row.childResourceId : row.parentResourceId + sourceToTarget.set(source, target) + } + if (sourceToTarget.size === 0) continue + + const moves = + family === 'file' + ? await planFileRehome(tx, sourceWorkspaceId, targetWorkspaceId, sourceToTarget) + : await planContainerRehome( + tx, + family, + sourceWorkspaceId, + targetWorkspaceId, + sourceToTarget + ) + if (moves.length === 0) continue + + const familyFolderIdMap = await resolveForkFolderMapping({ + tx, + sourceWorkspaceId, + targetWorkspaceId, + userId, + now, + resourceType: family, + contentFolderIds: moves.map((move) => move.sourceFolderId), + }) + for (const [source, target] of familyFolderIdMap) folderIdMap.set(source, target) + + // Group by destination so each folder is one UPDATE regardless of how many rows land in it. + const targetIdsByFolder = new Map() + for (const move of moves) { + const targetFolderId = familyFolderIdMap.get(move.sourceFolderId) + if (!targetFolderId) continue + const bucket = targetIdsByFolder.get(targetFolderId) + if (bucket) bucket.push(move.targetId) + else targetIdsByFolder.set(targetFolderId, [move.targetId]) + } + + for (const [targetFolderId, targetIds] of targetIdsByFolder) { + if (family === 'file') { + await tx + .update(workspaceFiles) + .set({ folderId: targetFolderId }) + .where(and(inArray(workspaceFiles.id, targetIds), isNull(workspaceFiles.folderId))) + } else if (family === 'table') { + await tx + .update(userTableDefinitions) + .set({ folderId: targetFolderId, updatedAt: now }) + .where( + and(inArray(userTableDefinitions.id, targetIds), isNull(userTableDefinitions.folderId)) + ) + } else { + await tx + .update(knowledgeBase) + .set({ folderId: targetFolderId, updatedAt: now }) + .where(and(inArray(knowledgeBase.id, targetIds), isNull(knowledgeBase.folderId))) + } + rehomed[family] += targetIds.length + } + } + + const total = rehomed.file + rehomed.table + rehomed.knowledge_base + if (total > 0) { + logger.info(`[${params.requestId ?? 'unknown'}] Re-homed root-flattened fork resources`, { + targetWorkspaceId, + direction, + ...rehomed, + }) + } + + return { folderIdMap, rehomed } +} + +interface RehomeMove { + targetId: string + sourceFolderId: string +} + +/** + * Files map by storage key, so the source lookup keys on `workspace_files.key` while the target + * update keys on `workspace_files.id`. Only durable `workspace`-context rows participate, matching + * what the copy is willing to duplicate in the first place. + */ +async function planFileRehome( + tx: DbOrTx, + sourceWorkspaceId: string, + targetWorkspaceId: string, + sourceToTarget: Map +): Promise { + const targetRows = await tx + .select({ id: workspaceFiles.id, key: workspaceFiles.key }) + .from(workspaceFiles) + .where( + and( + inArray(workspaceFiles.key, Array.from(sourceToTarget.values())), + eq(workspaceFiles.workspaceId, targetWorkspaceId), + eq(workspaceFiles.context, 'workspace'), + isNull(workspaceFiles.folderId), + isNull(workspaceFiles.deletedAt) + ) + ) + if (targetRows.length === 0) return [] + const targetIdByKey = new Map(targetRows.map((row) => [row.key, row.id])) + + // Only the sources whose target is actually still flattened need their folder read. + const wantedSourceKeys = Array.from(sourceToTarget) + .filter(([, targetKey]) => targetIdByKey.has(targetKey)) + .map(([sourceKey]) => sourceKey) + if (wantedSourceKeys.length === 0) return [] + + const sourceRows = await tx + .select({ key: workspaceFiles.key, folderId: workspaceFiles.folderId }) + .from(workspaceFiles) + .where( + and( + inArray(workspaceFiles.key, wantedSourceKeys), + eq(workspaceFiles.workspaceId, sourceWorkspaceId), + eq(workspaceFiles.context, 'workspace'), + isNull(workspaceFiles.deletedAt) + ) + ) + + const moves: RehomeMove[] = [] + for (const source of sourceRows) { + if (!source.folderId) continue + const targetKey = sourceToTarget.get(source.key) + const targetId = targetKey ? targetIdByKey.get(targetKey) : undefined + if (targetId) moves.push({ targetId, sourceFolderId: source.folderId }) + } + return moves +} + +/** Tables and knowledge bases both map by row id and differ only in table + soft-delete column. */ +async function planContainerRehome( + tx: DbOrTx, + family: 'table' | 'knowledge_base', + sourceWorkspaceId: string, + targetWorkspaceId: string, + sourceToTarget: Map +): Promise { + const targetIds = Array.from(sourceToTarget.values()) + const sourceIds = Array.from(sourceToTarget.keys()) + + const [targetRows, sourceRows] = + family === 'table' + ? await Promise.all([ + tx + .select({ id: userTableDefinitions.id }) + .from(userTableDefinitions) + .where( + and( + inArray(userTableDefinitions.id, targetIds), + eq(userTableDefinitions.workspaceId, targetWorkspaceId), + isNull(userTableDefinitions.folderId), + isNull(userTableDefinitions.archivedAt) + ) + ), + tx + .select({ id: userTableDefinitions.id, folderId: userTableDefinitions.folderId }) + .from(userTableDefinitions) + .where( + and( + inArray(userTableDefinitions.id, sourceIds), + eq(userTableDefinitions.workspaceId, sourceWorkspaceId), + isNull(userTableDefinitions.archivedAt) + ) + ), + ]) + : await Promise.all([ + tx + .select({ id: knowledgeBase.id }) + .from(knowledgeBase) + .where( + and( + inArray(knowledgeBase.id, targetIds), + eq(knowledgeBase.workspaceId, targetWorkspaceId), + isNull(knowledgeBase.folderId), + isNull(knowledgeBase.deletedAt) + ) + ), + tx + .select({ id: knowledgeBase.id, folderId: knowledgeBase.folderId }) + .from(knowledgeBase) + .where( + and( + inArray(knowledgeBase.id, sourceIds), + eq(knowledgeBase.workspaceId, sourceWorkspaceId), + isNull(knowledgeBase.deletedAt) + ) + ), + ]) + + const flattenedTargetIds = new Set(targetRows.map((row) => row.id)) + if (flattenedTargetIds.size === 0) return [] + + const moves: RehomeMove[] = [] + for (const source of sourceRows) { + if (!source.folderId) continue + const targetId = sourceToTarget.get(source.id) + if (targetId && flattenedTargetIds.has(targetId)) { + moves.push({ targetId, sourceFolderId: source.folderId }) + } + } + return moves +} From ffc36852b8b5423eeaf1533a542dc46457d26996 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 15 Aug 2026 17:16:25 -0700 Subject: [PATCH 2/3] refactor(fork): page the re-home lookups and reuse the plan's identity rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../lib/promote/promote-plan.ts | 8 + .../workspace-forking/lib/promote/promote.ts | 2 +- .../lib/promote/rehome-mapped.test.ts | 95 ++++---- .../lib/promote/rehome-mapped.ts | 222 ++++++++++-------- 4 files changed, 174 insertions(+), 153 deletions(-) diff --git a/apps/sim/ee/workspace-forking/lib/promote/promote-plan.ts b/apps/sim/ee/workspace-forking/lib/promote/promote-plan.ts index c666c345c30..1db9f7a8d07 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/promote-plan.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/promote-plan.ts @@ -8,6 +8,7 @@ import type { ForkEdge } from '@/ee/workspace-forking/lib/lineage/lineage' import { detectForkCascadeReferences } from '@/ee/workspace-forking/lib/mapping/cascade' import { buildForkResolver, + type ForkMappingRow, getEdgeMappingRows, resourceTypeToForkKind, } from '@/ee/workspace-forking/lib/mapping/mapping-store' @@ -89,6 +90,12 @@ export interface ForkPromotePlan { willUpdate: number willCreate: number willArchive: number + /** + * The edge's persisted identity rows, already read to build {@link ForkPromotePlan.resolver}. + * Exposed so later stages of the same transaction reuse them instead of re-reading the whole + * edge mapping, which for a large fork is a second full load of the same rows. + */ + mappingRows: ForkMappingRow[] } /** @@ -551,5 +558,6 @@ export async function computeForkPromotePlan(params: { willUpdate, willCreate, willArchive: archivedTargetIds.length, + mappingRows, } } diff --git a/apps/sim/ee/workspace-forking/lib/promote/promote.ts b/apps/sim/ee/workspace-forking/lib/promote/promote.ts index 7fab017fe33..0edaa888fb0 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/promote.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/promote.ts @@ -567,7 +567,7 @@ export async function promoteFork(params: PromoteForkParams): Promise ({ - mockGetEdgeMappingRows: vi.fn(), -})) - -vi.mock('@/ee/workspace-forking/lib/mapping/mapping-store', () => ({ - getEdgeMappingRows: mockGetEdgeMappingRows, -})) - +import type { ForkMappingRow } from '@/ee/workspace-forking/lib/mapping/mapping-store' import { rehomeFlattenedForkResources } from '@/ee/workspace-forking/lib/promote/rehome-mapped' interface UpdateCall { @@ -55,7 +47,9 @@ function makeTx(rows: { set: (values: Record) => ({ where: () => { updates.push({ table, values }) - return Promise.resolve() + // The real update reports the rows it actually moved; echo one id back so the + // caller's tally reflects a genuine write rather than the planned batch size. + return { returning: () => Promise.resolve([{ id: 'moved' }]) } }, }), }), @@ -63,13 +57,28 @@ function makeTx(rows: { return { tx: tx as unknown as DbOrTx, updates, insertedFolders } } -const edge = { - childWorkspaceId: 'child-ws', - parentWorkspaceId: 'parent-ws', -} as Parameters[0]['edge'] +const fileMapping: ForkMappingRow[] = [ + { + id: 'map-1', + childWorkspaceId: 'child-ws', + resourceType: 'file', + parentResourceId: 'workspace/parent-ws/a.png', + childResourceId: 'workspace/child-ws/a.png', + }, +] + +const kbMapping: ForkMappingRow[] = [ + { + id: 'map-2', + childWorkspaceId: 'child-ws', + resourceType: 'knowledge_base', + parentResourceId: 'kb-target', + childResourceId: 'kb-source', + }, +] const baseParams = { - edge, + mappingRows: [] as ForkMappingRow[], sourceWorkspaceId: 'child-ws', targetWorkspaceId: 'parent-ws', direction: 'push' as const, @@ -80,20 +89,10 @@ const baseParams = { describe('rehomeFlattenedForkResources', () => { beforeEach(() => { vi.clearAllMocks() - mockGetEdgeMappingRows.mockResolvedValue([]) }) it('mirrors the source folder and moves a root-flattened mapped file into it', async () => { // Push: the child is the source, so the mapping row's child side is the source key. - mockGetEdgeMappingRows.mockResolvedValue([ - { - id: 'map-1', - childWorkspaceId: 'child-ws', - resourceType: 'file', - parentResourceId: 'workspace/parent-ws/a.png', - childResourceId: 'workspace/child-ws/a.png', - }, - ]) const { tx, updates, insertedFolders } = makeTx({ // Both the target lookup (flattened row) and the source lookup read this table; the // rows carry the fields each phase needs. @@ -113,7 +112,11 @@ describe('rehomeFlattenedForkResources', () => { ], }) - const result = await rehomeFlattenedForkResources({ ...baseParams, tx }) + const result = await rehomeFlattenedForkResources({ + ...baseParams, + mappingRows: fileMapping, + tx, + }) expect(insertedFolders).toHaveLength(1) expect(insertedFolders[0]).toMatchObject({ @@ -130,15 +133,6 @@ describe('rehomeFlattenedForkResources', () => { }) it('leaves a resource alone when the source itself sits at the root', async () => { - mockGetEdgeMappingRows.mockResolvedValue([ - { - id: 'map-1', - childWorkspaceId: 'child-ws', - resourceType: 'file', - parentResourceId: 'workspace/parent-ws/a.png', - childResourceId: 'workspace/child-ws/a.png', - }, - ]) const { tx, updates, insertedFolders } = makeTx({ files: [ { id: 'file-target', key: 'workspace/parent-ws/a.png', folderId: null }, @@ -146,7 +140,11 @@ describe('rehomeFlattenedForkResources', () => { ], }) - const result = await rehomeFlattenedForkResources({ ...baseParams, tx }) + const result = await rehomeFlattenedForkResources({ + ...baseParams, + mappingRows: fileMapping, + tx, + }) expect(insertedFolders).toHaveLength(0) expect(updates).toHaveLength(0) @@ -154,15 +152,6 @@ describe('rehomeFlattenedForkResources', () => { }) it('never touches a target already placed in a folder, so a deliberate move survives a re-sync', async () => { - mockGetEdgeMappingRows.mockResolvedValue([ - { - id: 'map-1', - childWorkspaceId: 'child-ws', - resourceType: 'knowledge_base', - parentResourceId: 'kb-target', - childResourceId: 'kb-source', - }, - ]) // The target read filters on `folderId IS NULL`, so an already-placed row is simply absent. const { tx, updates } = makeTx({ knowledgeBases: [], @@ -178,7 +167,11 @@ describe('rehomeFlattenedForkResources', () => { ], }) - const result = await rehomeFlattenedForkResources({ ...baseParams, tx }) + const result = await rehomeFlattenedForkResources({ + ...baseParams, + mappingRows: kbMapping, + tx, + }) expect(updates).toHaveLength(0) expect(result.rehomed.knowledge_base).toBe(0) @@ -195,15 +188,6 @@ describe('rehomeFlattenedForkResources', () => { }) it('orients pull the other way: the parent side is the source', async () => { - mockGetEdgeMappingRows.mockResolvedValue([ - { - id: 'map-1', - childWorkspaceId: 'child-ws', - resourceType: 'file', - parentResourceId: 'workspace/parent-ws/a.png', - childResourceId: 'workspace/child-ws/a.png', - }, - ]) const { tx, updates } = makeTx({ files: [ // On a pull the CHILD is the target, so its key is the one that must still be flattened. @@ -224,6 +208,7 @@ describe('rehomeFlattenedForkResources', () => { const result = await rehomeFlattenedForkResources({ ...baseParams, + mappingRows: fileMapping, tx, direction: 'pull', sourceWorkspaceId: 'parent-ws', diff --git a/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.ts b/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.ts index dff5c5ca9b1..13f744490f8 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.ts @@ -1,13 +1,21 @@ import { knowledgeBase, userTableDefinitions, workspaceFiles } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { and, eq, inArray, isNull } from 'drizzle-orm' import type { DbOrTx } from '@/lib/db/types' import { resolveForkFolderMapping } from '@/ee/workspace-forking/lib/copy/copy-workflows' -import type { ForkEdge } from '@/ee/workspace-forking/lib/lineage/lineage' -import { getEdgeMappingRows } from '@/ee/workspace-forking/lib/mapping/mapping-store' +import type { ForkMappingRow } from '@/ee/workspace-forking/lib/mapping/mapping-store' const logger = createLogger('WorkspaceForkRehomeMapped') +/** + * Identity rows arrive in one shot from the promote plan, but the resource lookups they drive are + * `IN (...)` filters whose length scales with the whole edge. Page them so a large fork can never + * approach the bind-parameter ceiling or hand the planner a pathological list; matches the paging + * the rest of the fork copy already uses. + */ +const REHOME_PAGE = 500 + /** * The families whose copies predate folder-structure transit. Workflows are absent on purpose: * their folder placement has always been remapped, and a workflow sync rewrites `folder_id` on @@ -15,11 +23,13 @@ const logger = createLogger('WorkspaceForkRehomeMapped') */ const REHOMED_FAMILIES = ['file', 'table', 'knowledge_base'] as const +type RehomedFamily = (typeof REHOMED_FAMILIES)[number] + export interface RehomeMappedResult { /** source folder id -> target folder id for any subtree mirrored while re-homing. */ folderIdMap: Map - /** Rows re-homed per family, for the promote log line. */ - rehomed: Record<(typeof REHOMED_FAMILIES)[number], number> + /** Rows actually moved per family, for the promote log line. */ + rehomed: Record } /** @@ -32,12 +42,14 @@ export interface RehomeMappedResult { * target is never overwritten by a later sync. A resource that is legitimately at the source root * has nothing to map and stays put, so the pass converges to a no-op once healed. * - * Runs inside the promote transaction, after the folder mapping for workflows, so a refusal from - * the folder-ceiling check rolls the whole sync back rather than leaving a half-moved tree. + * Runs inside the promote transaction, after the sync-blocker gate, so a blocked sync moves + * nothing and a refusal from the folder-ceiling check rolls the whole sync back rather than + * leaving a half-moved tree. */ export async function rehomeFlattenedForkResources(params: { tx: DbOrTx - edge: ForkEdge + /** The edge's identity rows, reused from the promote plan rather than re-read. */ + mappingRows: readonly ForkMappingRow[] sourceWorkspaceId: string targetWorkspaceId: string direction: 'push' | 'pull' @@ -45,11 +57,10 @@ export async function rehomeFlattenedForkResources(params: { now: Date requestId?: string }): Promise { - const { tx, edge, sourceWorkspaceId, targetWorkspaceId, direction, userId, now } = params + const { tx, mappingRows, sourceWorkspaceId, targetWorkspaceId, direction, userId, now } = params const folderIdMap = new Map() - const rehomed = { file: 0, table: 0, knowledge_base: 0 } + const rehomed: Record = { file: 0, table: 0, knowledge_base: 0 } - const mappingRows = await getEdgeMappingRows(tx, edge.childWorkspaceId) // A pull copies parent -> child, so the parent side is the source; a push reverses it. const sourceIsParent = direction === 'pull' @@ -91,7 +102,7 @@ export async function rehomeFlattenedForkResources(params: { }) for (const [source, target] of familyFolderIdMap) folderIdMap.set(source, target) - // Group by destination so each folder is one UPDATE regardless of how many rows land in it. + // Group by destination so each folder costs one UPDATE regardless of how many rows land in it. const targetIdsByFolder = new Map() for (const move of moves) { const targetFolderId = familyFolderIdMap.get(move.sourceFolderId) @@ -102,25 +113,34 @@ export async function rehomeFlattenedForkResources(params: { } for (const [targetFolderId, targetIds] of targetIdsByFolder) { - if (family === 'file') { - await tx - .update(workspaceFiles) - .set({ folderId: targetFolderId }) - .where(and(inArray(workspaceFiles.id, targetIds), isNull(workspaceFiles.folderId))) - } else if (family === 'table') { - await tx - .update(userTableDefinitions) - .set({ folderId: targetFolderId, updatedAt: now }) - .where( - and(inArray(userTableDefinitions.id, targetIds), isNull(userTableDefinitions.folderId)) - ) - } else { - await tx - .update(knowledgeBase) - .set({ folderId: targetFolderId, updatedAt: now }) - .where(and(inArray(knowledgeBase.id, targetIds), isNull(knowledgeBase.folderId))) + for (const page of chunkArray(targetIds, REHOME_PAGE)) { + // The `folder_id IS NULL` guard is re-asserted on the write: the read above is only a + // plan, and this keeps the move idempotent under a concurrent placement. + const moved = + family === 'file' + ? await tx + .update(workspaceFiles) + .set({ folderId: targetFolderId }) + .where(and(inArray(workspaceFiles.id, page), isNull(workspaceFiles.folderId))) + .returning({ id: workspaceFiles.id }) + : family === 'table' + ? await tx + .update(userTableDefinitions) + .set({ folderId: targetFolderId, updatedAt: now }) + .where( + and( + inArray(userTableDefinitions.id, page), + isNull(userTableDefinitions.folderId) + ) + ) + .returning({ id: userTableDefinitions.id }) + : await tx + .update(knowledgeBase) + .set({ folderId: targetFolderId, updatedAt: now }) + .where(and(inArray(knowledgeBase.id, page), isNull(knowledgeBase.folderId))) + .returning({ id: knowledgeBase.id }) + rehomed[family] += moved.length } - rehomed[family] += targetIds.length } } @@ -152,20 +172,23 @@ async function planFileRehome( targetWorkspaceId: string, sourceToTarget: Map ): Promise { - const targetRows = await tx - .select({ id: workspaceFiles.id, key: workspaceFiles.key }) - .from(workspaceFiles) - .where( - and( - inArray(workspaceFiles.key, Array.from(sourceToTarget.values())), - eq(workspaceFiles.workspaceId, targetWorkspaceId), - eq(workspaceFiles.context, 'workspace'), - isNull(workspaceFiles.folderId), - isNull(workspaceFiles.deletedAt) + const targetIdByKey = new Map() + for (const page of chunkArray(Array.from(sourceToTarget.values()), REHOME_PAGE)) { + const rows = await tx + .select({ id: workspaceFiles.id, key: workspaceFiles.key }) + .from(workspaceFiles) + .where( + and( + inArray(workspaceFiles.key, page), + eq(workspaceFiles.workspaceId, targetWorkspaceId), + eq(workspaceFiles.context, 'workspace'), + isNull(workspaceFiles.folderId), + isNull(workspaceFiles.deletedAt) + ) ) - ) - if (targetRows.length === 0) return [] - const targetIdByKey = new Map(targetRows.map((row) => [row.key, row.id])) + for (const row of rows) targetIdByKey.set(row.key, row.id) + } + if (targetIdByKey.size === 0) return [] // Only the sources whose target is actually still flattened need their folder read. const wantedSourceKeys = Array.from(sourceToTarget) @@ -173,24 +196,25 @@ async function planFileRehome( .map(([sourceKey]) => sourceKey) if (wantedSourceKeys.length === 0) return [] - const sourceRows = await tx - .select({ key: workspaceFiles.key, folderId: workspaceFiles.folderId }) - .from(workspaceFiles) - .where( - and( - inArray(workspaceFiles.key, wantedSourceKeys), - eq(workspaceFiles.workspaceId, sourceWorkspaceId), - eq(workspaceFiles.context, 'workspace'), - isNull(workspaceFiles.deletedAt) - ) - ) - const moves: RehomeMove[] = [] - for (const source of sourceRows) { - if (!source.folderId) continue - const targetKey = sourceToTarget.get(source.key) - const targetId = targetKey ? targetIdByKey.get(targetKey) : undefined - if (targetId) moves.push({ targetId, sourceFolderId: source.folderId }) + for (const page of chunkArray(wantedSourceKeys, REHOME_PAGE)) { + const rows = await tx + .select({ key: workspaceFiles.key, folderId: workspaceFiles.folderId }) + .from(workspaceFiles) + .where( + and( + inArray(workspaceFiles.key, page), + eq(workspaceFiles.workspaceId, sourceWorkspaceId), + eq(workspaceFiles.context, 'workspace'), + isNull(workspaceFiles.deletedAt) + ) + ) + for (const source of rows) { + if (!source.folderId) continue + const targetKey = sourceToTarget.get(source.key) + const targetId = targetKey ? targetIdByKey.get(targetKey) : undefined + if (targetId) moves.push({ targetId, sourceFolderId: source.folderId }) + } } return moves } @@ -203,67 +227,71 @@ async function planContainerRehome( targetWorkspaceId: string, sourceToTarget: Map ): Promise { - const targetIds = Array.from(sourceToTarget.values()) - const sourceIds = Array.from(sourceToTarget.keys()) - - const [targetRows, sourceRows] = - family === 'table' - ? await Promise.all([ - tx + const flattenedTargetIds = new Set() + for (const page of chunkArray(Array.from(sourceToTarget.values()), REHOME_PAGE)) { + const rows = + family === 'table' + ? await tx .select({ id: userTableDefinitions.id }) .from(userTableDefinitions) .where( and( - inArray(userTableDefinitions.id, targetIds), + inArray(userTableDefinitions.id, page), eq(userTableDefinitions.workspaceId, targetWorkspaceId), isNull(userTableDefinitions.folderId), isNull(userTableDefinitions.archivedAt) ) - ), - tx - .select({ id: userTableDefinitions.id, folderId: userTableDefinitions.folderId }) - .from(userTableDefinitions) - .where( - and( - inArray(userTableDefinitions.id, sourceIds), - eq(userTableDefinitions.workspaceId, sourceWorkspaceId), - isNull(userTableDefinitions.archivedAt) - ) - ), - ]) - : await Promise.all([ - tx + ) + : await tx .select({ id: knowledgeBase.id }) .from(knowledgeBase) .where( and( - inArray(knowledgeBase.id, targetIds), + inArray(knowledgeBase.id, page), eq(knowledgeBase.workspaceId, targetWorkspaceId), isNull(knowledgeBase.folderId), isNull(knowledgeBase.deletedAt) ) - ), - tx + ) + for (const row of rows) flattenedTargetIds.add(row.id) + } + if (flattenedTargetIds.size === 0) return [] + + const wantedSourceIds = Array.from(sourceToTarget) + .filter(([, targetId]) => flattenedTargetIds.has(targetId)) + .map(([sourceId]) => sourceId) + if (wantedSourceIds.length === 0) return [] + + const moves: RehomeMove[] = [] + for (const page of chunkArray(wantedSourceIds, REHOME_PAGE)) { + const rows = + family === 'table' + ? await tx + .select({ id: userTableDefinitions.id, folderId: userTableDefinitions.folderId }) + .from(userTableDefinitions) + .where( + and( + inArray(userTableDefinitions.id, page), + eq(userTableDefinitions.workspaceId, sourceWorkspaceId), + isNull(userTableDefinitions.archivedAt) + ) + ) + : await tx .select({ id: knowledgeBase.id, folderId: knowledgeBase.folderId }) .from(knowledgeBase) .where( and( - inArray(knowledgeBase.id, sourceIds), + inArray(knowledgeBase.id, page), eq(knowledgeBase.workspaceId, sourceWorkspaceId), isNull(knowledgeBase.deletedAt) ) - ), - ]) - - const flattenedTargetIds = new Set(targetRows.map((row) => row.id)) - if (flattenedTargetIds.size === 0) return [] - - const moves: RehomeMove[] = [] - for (const source of sourceRows) { - if (!source.folderId) continue - const targetId = sourceToTarget.get(source.id) - if (targetId && flattenedTargetIds.has(targetId)) { - moves.push({ targetId, sourceFolderId: source.folderId }) + ) + for (const source of rows) { + if (!source.folderId) continue + const targetId = sourceToTarget.get(source.id) + if (targetId && flattenedTargetIds.has(targetId)) { + moves.push({ targetId, sourceFolderId: source.folderId }) + } } } return moves From a07e14bf9af159e4fa741c40fd77d3b689ad6bed Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 15 Aug 2026 17:32:09 -0700 Subject: [PATCH 3/3] fix(fork): drop the sync-time re-home pass, keep folder transit forward-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../lib/promote/promote-plan.ts | 8 - .../lib/promote/promote.test.ts | 8 - .../workspace-forking/lib/promote/promote.ts | 16 - .../lib/promote/rehome-mapped.test.ts | 221 ------------- .../lib/promote/rehome-mapped.ts | 298 ------------------ 5 files changed, 551 deletions(-) delete mode 100644 apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.test.ts delete mode 100644 apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.ts diff --git a/apps/sim/ee/workspace-forking/lib/promote/promote-plan.ts b/apps/sim/ee/workspace-forking/lib/promote/promote-plan.ts index 1db9f7a8d07..c666c345c30 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/promote-plan.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/promote-plan.ts @@ -8,7 +8,6 @@ import type { ForkEdge } from '@/ee/workspace-forking/lib/lineage/lineage' import { detectForkCascadeReferences } from '@/ee/workspace-forking/lib/mapping/cascade' import { buildForkResolver, - type ForkMappingRow, getEdgeMappingRows, resourceTypeToForkKind, } from '@/ee/workspace-forking/lib/mapping/mapping-store' @@ -90,12 +89,6 @@ export interface ForkPromotePlan { willUpdate: number willCreate: number willArchive: number - /** - * The edge's persisted identity rows, already read to build {@link ForkPromotePlan.resolver}. - * Exposed so later stages of the same transaction reuse them instead of re-reading the whole - * edge mapping, which for a large fork is a second full load of the same rows. - */ - mappingRows: ForkMappingRow[] } /** @@ -558,6 +551,5 @@ export async function computeForkPromotePlan(params: { willUpdate, willCreate, willArchive: archivedTargetIds.length, - mappingRows, } } diff --git a/apps/sim/ee/workspace-forking/lib/promote/promote.test.ts b/apps/sim/ee/workspace-forking/lib/promote/promote.test.ts index 1670968af3c..c44dd9c9e2a 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/promote.test.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/promote.test.ts @@ -107,14 +107,6 @@ vi.mock('@/ee/workspace-forking/lib/mapping/mapping-store', () => ({ deleteWorkflowIdentityByIds: vi.fn(), upsertEdgeMappings: vi.fn(), })) -// Re-homing has its own suite (`rehome-mapped.test.ts`); stub it here so these promote -// orchestration tests are not coupled to its queries. -vi.mock('@/ee/workspace-forking/lib/promote/rehome-mapped', () => ({ - rehomeFlattenedForkResources: vi.fn(async () => ({ - folderIdMap: new Map(), - rehomed: { file: 0, table: 0, knowledge_base: 0 }, - })), -})) vi.mock('@/ee/workspace-forking/lib/promote/cleared-refs', () => ({ collectForkSyncBlockers: mockCollectBlockers, verifyForkDropAcknowledgments: mockVerifyDrops, diff --git a/apps/sim/ee/workspace-forking/lib/promote/promote.ts b/apps/sim/ee/workspace-forking/lib/promote/promote.ts index 0edaa888fb0..ea92f08c4e8 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/promote.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/promote.ts @@ -82,7 +82,6 @@ import { type PromoteRunWorkflowSnapshot, upsertPromoteRun, } from '@/ee/workspace-forking/lib/promote/promote-run-store' -import { rehomeFlattenedForkResources } from '@/ee/workspace-forking/lib/promote/rehome-mapped' import { buildForkTriggerPlan, type ForkTriggerMappingInput, @@ -562,21 +561,6 @@ export async function promoteFork(params: PromoteForkParams): Promise item.sourceMeta.folderId), }) - // Heal earlier syncs that landed mapped files/tables/KBs at the target root before folder - // structure transited a fork edge. Touches only still-flattened rows, so it converges to a - // no-op and never overrides a placement chosen in the target. - const rehomeResult = await rehomeFlattenedForkResources({ - tx, - mappingRows: plan.mappingRows, - sourceWorkspaceId, - targetWorkspaceId, - direction, - userId, - now, - requestId, - }) - for (const [source, target] of rehomeResult.folderIdMap) folderIdMap.set(source, target) - let resolver = plan.resolver let copyContentPlan: ForkContentPlan | null = null let copyContentRefMaps: SerializableForkContentRefMaps | null = null diff --git a/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.test.ts b/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.test.ts deleted file mode 100644 index 0c6c17758ba..00000000000 --- a/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.test.ts +++ /dev/null @@ -1,221 +0,0 @@ -/** - * @vitest-environment node - */ -import { folder as folderTable, knowledgeBase, workspaceFiles } from '@sim/db/schema' -import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { DbOrTx } from '@/lib/db/types' -import type { ForkMappingRow } from '@/ee/workspace-forking/lib/mapping/mapping-store' -import { rehomeFlattenedForkResources } from '@/ee/workspace-forking/lib/promote/rehome-mapped' - -interface UpdateCall { - table: unknown - values: Record -} - -/** - * Table-dispatched tx mock. Reads are keyed by table (and, for the two-phase folder mapping, - * by call order: source tree first, then target tree) so a test never has to count queries. - */ -function makeTx(rows: { - files?: Record[] - knowledgeBases?: Record[] - sourceFolders?: Record[] -}) { - const updates: UpdateCall[] = [] - const insertedFolders: Record[] = [] - let folderCall = 0 - const tx = { - select: () => ({ - from: (table: unknown) => ({ - where: () => { - if (table === folderTable) { - return Promise.resolve(folderCall++ === 0 ? (rows.sourceFolders ?? []) : []) - } - if (table === workspaceFiles) return Promise.resolve(rows.files ?? []) - if (table === knowledgeBase) return Promise.resolve(rows.knowledgeBases ?? []) - return Promise.resolve([]) - }, - }), - }), - insert: () => ({ - values: (values: Record[]) => { - insertedFolders.push(...values) - return Promise.resolve() - }, - }), - update: (table: unknown) => ({ - set: (values: Record) => ({ - where: () => { - updates.push({ table, values }) - // The real update reports the rows it actually moved; echo one id back so the - // caller's tally reflects a genuine write rather than the planned batch size. - return { returning: () => Promise.resolve([{ id: 'moved' }]) } - }, - }), - }), - } - return { tx: tx as unknown as DbOrTx, updates, insertedFolders } -} - -const fileMapping: ForkMappingRow[] = [ - { - id: 'map-1', - childWorkspaceId: 'child-ws', - resourceType: 'file', - parentResourceId: 'workspace/parent-ws/a.png', - childResourceId: 'workspace/child-ws/a.png', - }, -] - -const kbMapping: ForkMappingRow[] = [ - { - id: 'map-2', - childWorkspaceId: 'child-ws', - resourceType: 'knowledge_base', - parentResourceId: 'kb-target', - childResourceId: 'kb-source', - }, -] - -const baseParams = { - mappingRows: [] as ForkMappingRow[], - sourceWorkspaceId: 'child-ws', - targetWorkspaceId: 'parent-ws', - direction: 'push' as const, - userId: 'user-1', - now: new Date('2026-08-15T00:00:00.000Z'), -} - -describe('rehomeFlattenedForkResources', () => { - beforeEach(() => { - vi.clearAllMocks() - }) - - it('mirrors the source folder and moves a root-flattened mapped file into it', async () => { - // Push: the child is the source, so the mapping row's child side is the source key. - const { tx, updates, insertedFolders } = makeTx({ - // Both the target lookup (flattened row) and the source lookup read this table; the - // rows carry the fields each phase needs. - files: [ - { id: 'file-target', key: 'workspace/parent-ws/a.png', folderId: null }, - { id: 'file-source', key: 'workspace/child-ws/a.png', folderId: 'src-folder' }, - ], - sourceFolders: [ - { - id: 'src-folder', - name: 'Contracts', - parentId: null, - workspaceId: 'child-ws', - resourceType: 'file', - deletedAt: null, - }, - ], - }) - - const result = await rehomeFlattenedForkResources({ - ...baseParams, - mappingRows: fileMapping, - tx, - }) - - expect(insertedFolders).toHaveLength(1) - expect(insertedFolders[0]).toMatchObject({ - name: 'Contracts', - workspaceId: 'parent-ws', - resourceType: 'file', - }) - const newFolderId = insertedFolders[0].id as string - expect(updates).toHaveLength(1) - expect(updates[0].table).toBe(workspaceFiles) - expect(updates[0].values).toEqual({ folderId: newFolderId }) - expect(result.rehomed.file).toBe(1) - expect(result.folderIdMap.get('src-folder')).toBe(newFolderId) - }) - - it('leaves a resource alone when the source itself sits at the root', async () => { - const { tx, updates, insertedFolders } = makeTx({ - files: [ - { id: 'file-target', key: 'workspace/parent-ws/a.png', folderId: null }, - { id: 'file-source', key: 'workspace/child-ws/a.png', folderId: null }, - ], - }) - - const result = await rehomeFlattenedForkResources({ - ...baseParams, - mappingRows: fileMapping, - tx, - }) - - expect(insertedFolders).toHaveLength(0) - expect(updates).toHaveLength(0) - expect(result.rehomed.file).toBe(0) - }) - - it('never touches a target already placed in a folder, so a deliberate move survives a re-sync', async () => { - // The target read filters on `folderId IS NULL`, so an already-placed row is simply absent. - const { tx, updates } = makeTx({ - knowledgeBases: [], - sourceFolders: [ - { - id: 'src-folder', - name: 'Policies', - parentId: null, - workspaceId: 'child-ws', - resourceType: 'knowledge_base', - deletedAt: null, - }, - ], - }) - - const result = await rehomeFlattenedForkResources({ - ...baseParams, - mappingRows: kbMapping, - tx, - }) - - expect(updates).toHaveLength(0) - expect(result.rehomed.knowledge_base).toBe(0) - }) - - it('no-ops entirely when the edge has no mapped resources', async () => { - const { tx, updates, insertedFolders } = makeTx({}) - - const result = await rehomeFlattenedForkResources({ ...baseParams, tx }) - - expect(updates).toHaveLength(0) - expect(insertedFolders).toHaveLength(0) - expect(result.rehomed).toEqual({ file: 0, table: 0, knowledge_base: 0 }) - }) - - it('orients pull the other way: the parent side is the source', async () => { - const { tx, updates } = makeTx({ - files: [ - // On a pull the CHILD is the target, so its key is the one that must still be flattened. - { id: 'file-target', key: 'workspace/child-ws/a.png', folderId: null }, - { id: 'file-source', key: 'workspace/parent-ws/a.png', folderId: 'src-folder' }, - ], - sourceFolders: [ - { - id: 'src-folder', - name: 'Contracts', - parentId: null, - workspaceId: 'parent-ws', - resourceType: 'file', - deletedAt: null, - }, - ], - }) - - const result = await rehomeFlattenedForkResources({ - ...baseParams, - mappingRows: fileMapping, - tx, - direction: 'pull', - sourceWorkspaceId: 'parent-ws', - targetWorkspaceId: 'child-ws', - }) - - expect(updates).toHaveLength(1) - expect(result.rehomed.file).toBe(1) - }) -}) diff --git a/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.ts b/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.ts deleted file mode 100644 index 13f744490f8..00000000000 --- a/apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.ts +++ /dev/null @@ -1,298 +0,0 @@ -import { knowledgeBase, userTableDefinitions, workspaceFiles } from '@sim/db/schema' -import { createLogger } from '@sim/logger' -import { chunkArray } from '@sim/utils/helpers' -import { and, eq, inArray, isNull } from 'drizzle-orm' -import type { DbOrTx } from '@/lib/db/types' -import { resolveForkFolderMapping } from '@/ee/workspace-forking/lib/copy/copy-workflows' -import type { ForkMappingRow } from '@/ee/workspace-forking/lib/mapping/mapping-store' - -const logger = createLogger('WorkspaceForkRehomeMapped') - -/** - * Identity rows arrive in one shot from the promote plan, but the resource lookups they drive are - * `IN (...)` filters whose length scales with the whole edge. Page them so a large fork can never - * approach the bind-parameter ceiling or hand the planner a pathological list; matches the paging - * the rest of the fork copy already uses. - */ -const REHOME_PAGE = 500 - -/** - * The families whose copies predate folder-structure transit. Workflows are absent on purpose: - * their folder placement has always been remapped, and a workflow sync rewrites `folder_id` on - * every replace, so they are already self-healing. - */ -const REHOMED_FAMILIES = ['file', 'table', 'knowledge_base'] as const - -type RehomedFamily = (typeof REHOMED_FAMILIES)[number] - -export interface RehomeMappedResult { - /** source folder id -> target folder id for any subtree mirrored while re-homing. */ - folderIdMap: Map - /** Rows actually moved per family, for the promote log line. */ - rehomed: Record -} - -/** - * Re-home already-mapped files / tables / knowledge bases that an earlier sync flattened to the - * target root, mirroring the source folder subtree they belong to and moving them into it. - * - * Deliberately narrow: only rows whose target `folder_id` is currently NULL are touched. Copies - * made before folder structure transited a fork edge all landed at the root, so NULL is the exact - * signature of the damage - and skipping non-NULL rows means a placement the user chose in the - * target is never overwritten by a later sync. A resource that is legitimately at the source root - * has nothing to map and stays put, so the pass converges to a no-op once healed. - * - * Runs inside the promote transaction, after the sync-blocker gate, so a blocked sync moves - * nothing and a refusal from the folder-ceiling check rolls the whole sync back rather than - * leaving a half-moved tree. - */ -export async function rehomeFlattenedForkResources(params: { - tx: DbOrTx - /** The edge's identity rows, reused from the promote plan rather than re-read. */ - mappingRows: readonly ForkMappingRow[] - sourceWorkspaceId: string - targetWorkspaceId: string - direction: 'push' | 'pull' - userId: string - now: Date - requestId?: string -}): Promise { - const { tx, mappingRows, sourceWorkspaceId, targetWorkspaceId, direction, userId, now } = params - const folderIdMap = new Map() - const rehomed: Record = { file: 0, table: 0, knowledge_base: 0 } - - // A pull copies parent -> child, so the parent side is the source; a push reverses it. - const sourceIsParent = direction === 'pull' - - for (const family of REHOMED_FAMILIES) { - /** - * Source resource key -> target resource key for this family. Files map by STORAGE KEY (that - * is what `file-upload` subblocks reference and what the mapping rows store); tables and - * knowledge bases map by row id. - */ - const sourceToTarget = new Map() - for (const row of mappingRows) { - if (row.resourceType !== family || !row.childResourceId) continue - const source = sourceIsParent ? row.parentResourceId : row.childResourceId - const target = sourceIsParent ? row.childResourceId : row.parentResourceId - sourceToTarget.set(source, target) - } - if (sourceToTarget.size === 0) continue - - const moves = - family === 'file' - ? await planFileRehome(tx, sourceWorkspaceId, targetWorkspaceId, sourceToTarget) - : await planContainerRehome( - tx, - family, - sourceWorkspaceId, - targetWorkspaceId, - sourceToTarget - ) - if (moves.length === 0) continue - - const familyFolderIdMap = await resolveForkFolderMapping({ - tx, - sourceWorkspaceId, - targetWorkspaceId, - userId, - now, - resourceType: family, - contentFolderIds: moves.map((move) => move.sourceFolderId), - }) - for (const [source, target] of familyFolderIdMap) folderIdMap.set(source, target) - - // Group by destination so each folder costs one UPDATE regardless of how many rows land in it. - const targetIdsByFolder = new Map() - for (const move of moves) { - const targetFolderId = familyFolderIdMap.get(move.sourceFolderId) - if (!targetFolderId) continue - const bucket = targetIdsByFolder.get(targetFolderId) - if (bucket) bucket.push(move.targetId) - else targetIdsByFolder.set(targetFolderId, [move.targetId]) - } - - for (const [targetFolderId, targetIds] of targetIdsByFolder) { - for (const page of chunkArray(targetIds, REHOME_PAGE)) { - // The `folder_id IS NULL` guard is re-asserted on the write: the read above is only a - // plan, and this keeps the move idempotent under a concurrent placement. - const moved = - family === 'file' - ? await tx - .update(workspaceFiles) - .set({ folderId: targetFolderId }) - .where(and(inArray(workspaceFiles.id, page), isNull(workspaceFiles.folderId))) - .returning({ id: workspaceFiles.id }) - : family === 'table' - ? await tx - .update(userTableDefinitions) - .set({ folderId: targetFolderId, updatedAt: now }) - .where( - and( - inArray(userTableDefinitions.id, page), - isNull(userTableDefinitions.folderId) - ) - ) - .returning({ id: userTableDefinitions.id }) - : await tx - .update(knowledgeBase) - .set({ folderId: targetFolderId, updatedAt: now }) - .where(and(inArray(knowledgeBase.id, page), isNull(knowledgeBase.folderId))) - .returning({ id: knowledgeBase.id }) - rehomed[family] += moved.length - } - } - } - - const total = rehomed.file + rehomed.table + rehomed.knowledge_base - if (total > 0) { - logger.info(`[${params.requestId ?? 'unknown'}] Re-homed root-flattened fork resources`, { - targetWorkspaceId, - direction, - ...rehomed, - }) - } - - return { folderIdMap, rehomed } -} - -interface RehomeMove { - targetId: string - sourceFolderId: string -} - -/** - * Files map by storage key, so the source lookup keys on `workspace_files.key` while the target - * update keys on `workspace_files.id`. Only durable `workspace`-context rows participate, matching - * what the copy is willing to duplicate in the first place. - */ -async function planFileRehome( - tx: DbOrTx, - sourceWorkspaceId: string, - targetWorkspaceId: string, - sourceToTarget: Map -): Promise { - const targetIdByKey = new Map() - for (const page of chunkArray(Array.from(sourceToTarget.values()), REHOME_PAGE)) { - const rows = await tx - .select({ id: workspaceFiles.id, key: workspaceFiles.key }) - .from(workspaceFiles) - .where( - and( - inArray(workspaceFiles.key, page), - eq(workspaceFiles.workspaceId, targetWorkspaceId), - eq(workspaceFiles.context, 'workspace'), - isNull(workspaceFiles.folderId), - isNull(workspaceFiles.deletedAt) - ) - ) - for (const row of rows) targetIdByKey.set(row.key, row.id) - } - if (targetIdByKey.size === 0) return [] - - // Only the sources whose target is actually still flattened need their folder read. - const wantedSourceKeys = Array.from(sourceToTarget) - .filter(([, targetKey]) => targetIdByKey.has(targetKey)) - .map(([sourceKey]) => sourceKey) - if (wantedSourceKeys.length === 0) return [] - - const moves: RehomeMove[] = [] - for (const page of chunkArray(wantedSourceKeys, REHOME_PAGE)) { - const rows = await tx - .select({ key: workspaceFiles.key, folderId: workspaceFiles.folderId }) - .from(workspaceFiles) - .where( - and( - inArray(workspaceFiles.key, page), - eq(workspaceFiles.workspaceId, sourceWorkspaceId), - eq(workspaceFiles.context, 'workspace'), - isNull(workspaceFiles.deletedAt) - ) - ) - for (const source of rows) { - if (!source.folderId) continue - const targetKey = sourceToTarget.get(source.key) - const targetId = targetKey ? targetIdByKey.get(targetKey) : undefined - if (targetId) moves.push({ targetId, sourceFolderId: source.folderId }) - } - } - return moves -} - -/** Tables and knowledge bases both map by row id and differ only in table + soft-delete column. */ -async function planContainerRehome( - tx: DbOrTx, - family: 'table' | 'knowledge_base', - sourceWorkspaceId: string, - targetWorkspaceId: string, - sourceToTarget: Map -): Promise { - const flattenedTargetIds = new Set() - for (const page of chunkArray(Array.from(sourceToTarget.values()), REHOME_PAGE)) { - const rows = - family === 'table' - ? await tx - .select({ id: userTableDefinitions.id }) - .from(userTableDefinitions) - .where( - and( - inArray(userTableDefinitions.id, page), - eq(userTableDefinitions.workspaceId, targetWorkspaceId), - isNull(userTableDefinitions.folderId), - isNull(userTableDefinitions.archivedAt) - ) - ) - : await tx - .select({ id: knowledgeBase.id }) - .from(knowledgeBase) - .where( - and( - inArray(knowledgeBase.id, page), - eq(knowledgeBase.workspaceId, targetWorkspaceId), - isNull(knowledgeBase.folderId), - isNull(knowledgeBase.deletedAt) - ) - ) - for (const row of rows) flattenedTargetIds.add(row.id) - } - if (flattenedTargetIds.size === 0) return [] - - const wantedSourceIds = Array.from(sourceToTarget) - .filter(([, targetId]) => flattenedTargetIds.has(targetId)) - .map(([sourceId]) => sourceId) - if (wantedSourceIds.length === 0) return [] - - const moves: RehomeMove[] = [] - for (const page of chunkArray(wantedSourceIds, REHOME_PAGE)) { - const rows = - family === 'table' - ? await tx - .select({ id: userTableDefinitions.id, folderId: userTableDefinitions.folderId }) - .from(userTableDefinitions) - .where( - and( - inArray(userTableDefinitions.id, page), - eq(userTableDefinitions.workspaceId, sourceWorkspaceId), - isNull(userTableDefinitions.archivedAt) - ) - ) - : await tx - .select({ id: knowledgeBase.id, folderId: knowledgeBase.folderId }) - .from(knowledgeBase) - .where( - and( - inArray(knowledgeBase.id, page), - eq(knowledgeBase.workspaceId, sourceWorkspaceId), - isNull(knowledgeBase.deletedAt) - ) - ) - for (const source of rows) { - if (!source.folderId) continue - const targetId = sourceToTarget.get(source.id) - if (targetId && flattenedTargetIds.has(targetId)) { - moves.push({ targetId, sourceFolderId: source.folderId }) - } - } - } - return moves -}