diff --git a/packages/cli/src/modules/promotions.ee/__tests__/branch-placement.test.ts b/packages/cli/src/modules/promotions.ee/__tests__/branch-placement.test.ts index 0ae5d29c4b7..c54da8de126 100644 --- a/packages/cli/src/modules/promotions.ee/__tests__/branch-placement.test.ts +++ b/packages/cli/src/modules/promotions.ee/__tests__/branch-placement.test.ts @@ -1,6 +1,6 @@ import type { PackageManifest } from '@/modules/n8n-packages/spec/manifest.schema'; -import { containerPlacement, pinPath, staleWorkflowTargets } from '../branch-placement'; +import { containerMoves, remapPath, staleWorkflowTargets } from '../branch-placement'; const baseMetadata = { packageFormatVersion: '1' as const, @@ -20,8 +20,8 @@ function entry(id: string, name = `name-${id}`, target = `target/${id}`) { const P = 'projects/acme'; const acme = entry('p1', 'Acme', P); -describe('containerPlacement', () => { - it('pins a renamed project so the selection lands in the directory the branch holds', () => { +describe('containerMoves', () => { + it('moves a renamed project to the path the export uses, so the branch content rides along', () => { const existing = makeManifest({ projects: [acme], folders: [entry('f1', 'Sales', `${P}/folders/sales`)], @@ -31,13 +31,12 @@ describe('containerPlacement', () => { workflows: [entry('w1', 'W1', 'projects/acme-corp/workflows/w1')], }); - const placement = containerPlacement(existing, staging); + const moves = containerMoves(existing, staging); - expect(pinPath('projects/acme-corp/workflows/w1', placement.pins)).toBe(`${P}/workflows/w1`); - expect(placement.keptFiles.has('projects/acme-corp/project.json')).toBe(true); + expect(moves).toEqual([{ kind: 'projects', from: P, to: 'projects/acme-corp' }]); }); - it('applies the deepest pin when a parent and a child were both renamed', () => { + it('orders the deepest container first when a parent and a child were both renamed', () => { const existing = makeManifest({ folders: [entry('f1', 'A', `${P}/folders/a`), entry('f2', 'C', `${P}/folders/a/c`)], }); @@ -46,14 +45,14 @@ describe('containerPlacement', () => { workflows: [entry('w1', 'W1', `${P}/folders/b/d/workflows/w1`)], }); - const placement = containerPlacement(existing, staging); - - expect(pinPath(`${P}/folders/b/d/workflows/w1`, placement.pins)).toBe( - `${P}/folders/a/c/workflows/w1`, - ); + // The order is the parking order: the child must leave before the parent moves. + expect(containerMoves(existing, staging)).toEqual([ + { kind: 'folders', from: `${P}/folders/a/c`, to: `${P}/folders/b/d` }, + { kind: 'folders', from: `${P}/folders/a`, to: `${P}/folders/b` }, + ]); }); - it('creates no pin for a folder the branch lacks', () => { + it('creates no move for a container the branch lacks', () => { const existing = makeManifest({ folders: [entry('f1', 'A', `${P}/folders/a`)], }); @@ -61,11 +60,37 @@ describe('containerPlacement', () => { folders: [entry('f1', 'B', `${P}/folders/b`), entry('f2', 'New', `${P}/folders/b/new`)], }); - const placement = containerPlacement(existing, staging); + const moves = containerMoves(existing, staging); - expect(pinPath(`${P}/folders/b/new/workflows/w1`, placement.pins)).toBe( - `${P}/folders/a/new/workflows/w1`, - ); + expect(moves).toEqual([{ kind: 'folders', from: `${P}/folders/a`, to: `${P}/folders/b` }]); + }); + + it('creates no move for a container whose path is unchanged', () => { + const existing = makeManifest({ projects: [acme] }); + const staging = makeManifest({ + projects: [entry('p1', 'Acme', P)], + workflows: [entry('w1', 'W1', `${P}/workflows/w1`)], + }); + + expect(containerMoves(existing, staging)).toEqual([]); + }); +}); + +describe('remapPath', () => { + const moves = [ + { kind: 'folders' as const, from: `${P}/folders/a`, to: `${P}/folders/b` }, + { kind: 'folders' as const, from: `${P}/folders/a/c`, to: `${P}/folders/b/d` }, + ]; + + it('rewrites through the deepest covering move regardless of move order', () => { + const target = `${P}/folders/a/c/workflows/w1`; + expect(remapPath(target, moves)).toBe(`${P}/folders/b/d/workflows/w1`); + expect(remapPath(target, [...moves].reverse())).toBe(`${P}/folders/b/d/workflows/w1`); + }); + + it('leaves a path no move covers, and does not match a sibling by prefix', () => { + expect(remapPath(`${P}/folders/ab/workflows/w1`, moves)).toBe(`${P}/folders/ab/workflows/w1`); + expect(remapPath(`${P}/variables/v1`, moves)).toBe(`${P}/variables/v1`); }); }); @@ -95,7 +120,7 @@ describe('staleWorkflowTargets', () => { ]); }); - it('leaves a renamed folder in place and only replaces the selected workflow', () => { + it('reports only the selected workflow old path when its folder was renamed', () => { const before = makeManifest({ folders: [entry('f1', 'sales', 'projects/p/folders/sales')], workflows: [ diff --git a/packages/cli/src/modules/promotions.ee/__tests__/import-manifest-bridge.test.ts b/packages/cli/src/modules/promotions.ee/__tests__/import-manifest-bridge.test.ts index cc0328a68ac..9cf5bf829d7 100644 --- a/packages/cli/src/modules/promotions.ee/__tests__/import-manifest-bridge.test.ts +++ b/packages/cli/src/modules/promotions.ee/__tests__/import-manifest-bridge.test.ts @@ -101,6 +101,37 @@ describe('import-manifest-bridge', () => { ]); }); + it('keeps an unselected variable whose project directory a container move relocated', async () => { + // The leftover manifest still names the old project path; the file is at the new one. + await writeTree({ + 'manifest.json': leftoverManifest({ + variables: [{ id: 'v-old', name: 'API_KEY', target: 'projects/alpha/variables/api-key' }], + }), + 'projects/renamed/project.json': JSON.stringify({ id: 'p1', name: 'Renamed' }), + 'projects/renamed/variables/api-key/variable.json': JSON.stringify({ name: 'API_KEY' }), + }); + + await writeImportManifest({ + exportFolder, + staging: packageManifestSchema.parse({ + packageFormatVersion: '1', + exportedAt: '2026-01-01T00:00:00.000Z', + sourceN8nVersion: '1.0.0', + sourceId: 'inst-1', + projects: [{ id: 'p1', name: 'Renamed', target: 'projects/renamed' }], + }), + sourceId: 'inst-test', + containerMoves: [{ kind: 'projects', from: 'projects/alpha', to: 'projects/renamed' }], + }); + + const written = packageManifestSchema.parse( + JSON.parse(await readFile(path.join(exportFolder, 'manifest.json'), 'utf-8')), + ); + expect(written.variables).toEqual([ + { id: 'v-old', name: 'API_KEY', target: 'projects/renamed/variables/api-key' }, + ]); + }); + it('unions workflow consumers when leftover and staging share a requirement key', async () => { await writeTree({ 'manifest.json': leftoverManifest({ diff --git a/packages/cli/src/modules/promotions.ee/__tests__/promotions-package.integration.test.ts b/packages/cli/src/modules/promotions.ee/__tests__/promotions-package.integration.test.ts index 6018fa716f9..e0241209e2f 100644 --- a/packages/cli/src/modules/promotions.ee/__tests__/promotions-package.integration.test.ts +++ b/packages/cli/src/modules/promotions.ee/__tests__/promotions-package.integration.test.ts @@ -2108,7 +2108,7 @@ describe('Promote a project selection — branch effects', () => { expect(onBranch).toHaveLength(2); }); - it('leaves a renamed folder alone, so unselected workflows keep their place', async () => { + it('moves a renamed folder, so unselected workflows ride along unchanged', async () => { const remote = await createRemote(); const connection = await createInstanceConnection(remote.bareDir); await service.clone(connection.id, 'promote'); @@ -2147,16 +2147,12 @@ describe('Promote a project selection — branch effects', () => { const selectedEntry = workflows.find((w) => w.id === selected.id)!; const unselectedEntry = workflows.find((w) => w.id === unselected.id)!; - expect(folderEntry).toMatchObject({ name: 'Sales', target: folderBefore.target }); - expect(selectedEntry.target).toBe(`${folderEntry.target}/workflows/selected-${selected.id}`); - expect(unselectedEntry.target).toBe( - `${folderEntry.target}/workflows/unselected-${unselected.id}`, - ); - await expect( - stat(path.join(dir, 'n8n-export', unselectedEntry.target, 'workflow.json')), - ).resolves.toBeDefined(); - const renamedTarget = folderEntry.target.replace(/[^/]+$/, `revenue-${folder.id}`); - await expect(stat(path.join(dir, 'n8n-export', renamedTarget))).rejects.toThrow(); + const renamedTarget = folderBefore.target.replace(/[^/]+$/, `revenue-${folder.id}`); + expect(folderEntry).toMatchObject({ name: 'Revenue', target: renamedTarget }); + expect(selectedEntry.target).toBe(`${renamedTarget}/workflows/selected-${selected.id}`); + expect(unselectedEntry.target).toBe(`${renamedTarget}/workflows/unselected-${unselected.id}`); + expect(unselectedEntry).toMatchObject({ name: 'Unselected' }); + await expect(stat(path.join(dir, 'n8n-export', folderBefore.target))).rejects.toThrow(); const workflowRepository = Container.get(WorkflowRepository); await workflowRepository.delete(unselected.id); diff --git a/packages/cli/src/modules/promotions.ee/__tests__/working-copy-updater.test.ts b/packages/cli/src/modules/promotions.ee/__tests__/working-copy-updater.test.ts index 355e227dd71..38ea18719f9 100644 --- a/packages/cli/src/modules/promotions.ee/__tests__/working-copy-updater.test.ts +++ b/packages/cli/src/modules/promotions.ee/__tests__/working-copy-updater.test.ts @@ -432,7 +432,7 @@ describe('WorkingCopyUpdater', () => { ); }); - it('keeps a renamed folder where the branch has it, so unselected workflows stay put', async () => { + it('renames the folder on the branch and rides unselected workflows along', async () => { await apply( { manifest: makeManifest({ @@ -465,16 +465,17 @@ describe('WorkingCopyUpdater', () => { { workflowIds: ['w1'], deletedWorkflowIds: ['w3'] }, ); - await expectAbsent('projects/alpha/folders/revenue'); - await expectAbsent('projects/alpha/folders/sales/workflows/w3'); - // The folder was renamed on the instance, but nobody selected that change. - expect(await readExported('projects/alpha/folders/sales/folder.json')).toBe( - folderFile('f1', 'Sales'), + // The old folder directory is gone; the rename rode along the selection. + await expectAbsent('projects/alpha/folders/sales'); + await expectAbsent('projects/alpha/folders/revenue/workflows/w3'); + expect(await readExported('projects/alpha/folders/revenue/folder.json')).toBe( + folderFile('f1', 'Revenue'), ); - expect(await readExported('projects/alpha/folders/sales/workflows/w1/workflow.json')).toBe( + expect(await readExported('projects/alpha/folders/revenue/workflows/w1/workflow.json')).toBe( workflowFile('w1', { v: 2 }), ); - expect(await readExported('projects/alpha/folders/sales/workflows/w2/workflow.json')).toBe( + // w2 was not selected, but it moves to the new folder path unchanged. + expect(await readExported('projects/alpha/folders/revenue/workflows/w2/workflow.json')).toBe( workflowFile('w2'), ); expect((await readWrittenManifest()).workflows?.map((entry) => entry.id).sort()).toEqual([ @@ -482,13 +483,13 @@ describe('WorkingCopyUpdater', () => { 'w2', ]); const branch = await updater.readBranchLayout(exportFolder); - expect(branch.folders).toEqual([folder('f1', 'Sales', 'sales')]); + expect(branch.folders).toEqual([folder('f1', 'Revenue', 'revenue')]); expect(branch.workflows).toEqual( - expect.arrayContaining([inFolder('w2', 'sales'), inFolder('w1', 'sales')]), + expect.arrayContaining([inFolder('w2', 'revenue'), inFolder('w1', 'revenue')]), ); }); - it('keeps both folders where the branch has them when their names swap', async () => { + it('swaps two folder directories when their names swap', async () => { // f1 is now named B (slug b) and f2 is named A (slug a); w1 and w3 are selected. await apply( { @@ -532,15 +533,15 @@ describe('WorkingCopyUpdater', () => { { workflowIds: ['w1', 'w3'] }, ); - // Each selected workflow lands in the directory its folder has on the - // branch, next to the siblings nobody selected. + // The folders swap: f1 (now B) takes slug b and f2 (now A) takes slug a. + // Each folder's unselected workflow rides along to the new path. const tree: Record = { - 'projects/alpha/folders/a/folder.json': folderFile('f1', 'A'), - 'projects/alpha/folders/b/folder.json': folderFile('f2', 'B'), - 'projects/alpha/folders/a/workflows/w1/workflow.json': workflowFile('w1', { moved: true }), - 'projects/alpha/folders/a/workflows/w2/workflow.json': workflowFile('w2'), - 'projects/alpha/folders/b/workflows/w3/workflow.json': workflowFile('w3', { moved: true }), - 'projects/alpha/folders/b/workflows/w4/workflow.json': workflowFile('w4'), + 'projects/alpha/folders/b/folder.json': folderFile('f1', 'B'), + 'projects/alpha/folders/a/folder.json': folderFile('f2', 'A'), + 'projects/alpha/folders/b/workflows/w1/workflow.json': workflowFile('w1', { moved: true }), + 'projects/alpha/folders/b/workflows/w2/workflow.json': workflowFile('w2'), + 'projects/alpha/folders/a/workflows/w3/workflow.json': workflowFile('w3', { moved: true }), + 'projects/alpha/folders/a/workflows/w4/workflow.json': workflowFile('w4'), }; for (const [file, content] of Object.entries(tree)) { expect(await readExported(file), file).toBe(content); @@ -553,6 +554,167 @@ describe('WorkingCopyUpdater', () => { ]); }); + it('refuses a rename when a manually changed branch path occupies its target', async () => { + await expect( + apply( + { + manifest: makeManifest({ + projects: [alpha], + folders: [folder('f1', 'A', 'a-f1'), folder('f2', 'B', 'b-f1')], + workflows: [inFolder('w1', 'a-f1'), inFolder('w2', 'b-f1')], + }), + files: { + 'projects/alpha/project.json': projectFile, + 'projects/alpha/folders/a-f1/folder.json': folderFile('f1', 'A'), + // f2 was manually moved to the path that a renamed f1 will use. + 'projects/alpha/folders/b-f1/folder.json': folderFile('f2', 'B'), + 'projects/alpha/folders/a-f1/workflows/w1/workflow.json': workflowFile('w1'), + 'projects/alpha/folders/b-f1/workflows/w2/workflow.json': workflowFile('w2'), + }, + }, + { + manifest: makeManifest({ + projects: [alpha], + folders: [folder('f1', 'B', 'b-f1')], + workflows: [inFolder('w1', 'b-f1')], + }), + files: { + 'projects/alpha/folders/b-f1/folder.json': folderFile('f1', 'B'), + 'projects/alpha/folders/b-f1/workflows/w1/workflow.json': workflowFile('w1'), + }, + }, + { workflowIds: ['w1'] }, + ), + ).rejects.toThrow( + 'The branch path "projects/alpha/folders/b-f1" is already in use. Remove it and retry.', + ); + + expect(await readExported('projects/alpha/folders/a-f1/folder.json')).toBe( + folderFile('f1', 'A'), + ); + expect(await readExported('projects/alpha/folders/b-f1/folder.json')).toBe( + folderFile('f2', 'B'), + ); + expect(await readExported('projects/alpha/folders/a-f1/workflows/w1/workflow.json')).toBe( + workflowFile('w1'), + ); + expect(await readExported('projects/alpha/folders/b-f1/workflows/w2/workflow.json')).toBe( + workflowFile('w2'), + ); + }); + + it('keeps an unselected variable in the manifest after a project rename moves its file', async () => { + const renamed = { id: alpha.id, name: 'Renamed', target: 'projects/renamed' }; + await apply( + { + manifest: makeManifest({ + projects: [alpha], + workflows: [wf('w1'), wf('w2')], + variables: [{ id: 'v1', name: 'V1', target: 'projects/alpha/variables/v1' }], + }), + files: { + 'projects/alpha/project.json': projectFile, + 'projects/alpha/workflows/w1/workflow.json': workflowFile('w1'), + 'projects/alpha/workflows/w2/workflow.json': workflowFile('w2'), + 'projects/alpha/variables/v1/variable.json': variableFile('v1', 'V1'), + }, + }, + { + manifest: makeManifest({ + projects: [renamed], + workflows: [{ id: 'w1', name: 'W1', target: 'projects/renamed/workflows/w1' }], + }), + files: { + 'projects/renamed/project.json': JSON.stringify({ id: alpha.id, name: 'Renamed' }), + 'projects/renamed/workflows/w1/workflow.json': workflowFile('w1', { v: 2 }), + }, + }, + { workflowIds: ['w1'] }, + ); + + expect(await readExported('projects/renamed/variables/v1/variable.json')).toBe( + variableFile('v1', 'V1'), + ); + expect((await readWrittenManifest()).variables).toEqual([ + { id: 'v1', name: 'V1', target: 'projects/renamed/variables/v1' }, + ]); + }); + + it('refuses to pull a folder out of another project when only a new workflow is selected', async () => { + // f1 moved from beta to alpha on the instance and gained w-new. w-new is + // not on the branch, so the selected-workflow check cannot see the move. + const beta = { id: 'p2', name: 'Beta', target: 'projects/beta' }; + await expect( + apply( + { + manifest: makeManifest({ + projects: [alpha, beta], + folders: [{ id: 'f1', name: 'F1', target: 'projects/beta/folders/f1' }], + workflows: [ + { + id: 'w-beta', + name: 'WBeta', + target: 'projects/beta/folders/f1/workflows/w-beta', + }, + ], + }), + files: { + 'projects/alpha/project.json': projectFile, + 'projects/beta/project.json': JSON.stringify({ id: beta.id, name: beta.name }), + 'projects/beta/folders/f1/folder.json': folderFile('f1', 'F1'), + 'projects/beta/folders/f1/workflows/w-beta/workflow.json': workflowFile('w-beta'), + }, + }, + { + manifest: makeManifest({ + projects: [alpha], + folders: [folder('f1', 'F1', 'f1')], + workflows: [inFolder('w-new', 'f1')], + }), + files: { + 'projects/alpha/folders/f1/folder.json': folderFile('f1', 'F1'), + 'projects/alpha/folders/f1/workflows/w-new/workflow.json': workflowFile('w-new'), + }, + }, + { workflowIds: ['w-new'] }, + ), + ).rejects.toThrow('belongs to another project on the branch'); + + expect(await readExported('projects/beta/folders/f1/workflows/w-beta/workflow.json')).toBe( + workflowFile('w-beta'), + ); + }); + + it('refuses to move a project whose entity file sits at the export root', async () => { + // A malformed branch scans the root project.json as target `.`. + await expect( + apply( + { + manifest: makeManifest({ + projects: [{ ...alpha, target: '.' }], + workflows: [{ id: 'w1', name: 'W1', target: 'workflows/w1' }], + }), + files: { + 'project.json': projectFile, + 'workflows/w1/workflow.json': workflowFile('w1'), + }, + }, + { + manifest: makeManifest({ projects: [alpha], workflows: [wf('w2')] }), + files: { + 'projects/alpha/project.json': projectFile, + 'projects/alpha/workflows/w2/workflow.json': workflowFile('w2'), + }, + }, + { workflowIds: ['w2'] }, + ), + ).rejects.toThrow('Container target "." is not a managed leaf directory'); + + expect(await readExported('project.json')).toBe(projectFile); + expect(await readExported('workflows/w1/workflow.json')).toBe(workflowFile('w1')); + await expectAbsent('projects/alpha'); + }); + it('does not write leftover requirements a selected workflow no longer uses', async () => { await apply( { diff --git a/packages/cli/src/modules/promotions.ee/branch-placement.ts b/packages/cli/src/modules/promotions.ee/branch-placement.ts index 0758913caf5..6741d107546 100644 --- a/packages/cli/src/modules/promotions.ee/branch-placement.ts +++ b/packages/cli/src/modules/promotions.ee/branch-placement.ts @@ -1,5 +1,4 @@ -import { entityFilePath } from '@/modules/n8n-packages/io/manifest-entry'; -import type { ManifestEntry, PackageManifest } from '@/modules/n8n-packages/spec/manifest.schema'; +import type { PackageManifest } from '@/modules/n8n-packages/spec/manifest.schema'; /** * Where each project, folder and workflow lives on the branch, read from the @@ -11,56 +10,45 @@ export type BranchLayout = Pick; -} - -const entriesOf = ( - state: Partial>, - kind: ContainerKind | 'workflows', -): ManifestEntry[] => state[kind] ?? []; - export const isUnder = (target: string, prefix: string) => target.startsWith(`${prefix}/`); /** - * Where the staging files land. A project or folder the branch holds keeps its - * directory, because renaming it would move every unselected workflow inside. - * Paths under it are pinned back, so a selection lands next to its siblings. + * Containers whose branch path differs from the export path, deepest `from` + * first. The exporter only writes ancestors of selected workflows, so no + * unrelated container moves. */ -export function containerPlacement(existing: BranchLayout, staging: PackageManifest): Placement { - const pins: Pin[] = []; - const keptFiles = new Set(); +export function containerMoves(existing: BranchLayout, staging: PackageManifest): ContainerMove[] { + const moves: ContainerMove[] = []; for (const kind of CONTAINER_KINDS) { - const onBranch = new Map(entriesOf(existing, kind).map((e) => [e.id, e.target])); - for (const entry of entriesOf(staging, kind)) { - const to = onBranch.get(entry.id); - if (to === undefined) continue; - keptFiles.add(entityFilePath(kind, entry.target)); - if (to !== entry.target) pins.push({ from: entry.target, to }); + const onBranch = new Map((existing[kind] ?? []).map((e) => [e.id, e.target])); + for (const entry of staging[kind] ?? []) { + const from = onBranch.get(entry.id); + if (from === undefined || from === entry.target) continue; + moves.push({ kind, from, to: entry.target }); } } - // Longest first, so a pinned folder wins over its pinned project. - return { pins: pins.sort((a, b) => b.from.length - a.from.length), keptFiles }; + return moves.sort((a, b) => b.from.length - a.from.length); } -/** Rewrite a staging path through the longest pin that covers it. */ -export function pinPath(target: string, pins: readonly Pin[]): string { - const pin = pins.find((p) => target === p.from || isUnder(target, p.from)); - return pin ? `${pin.to}${target.slice(pin.from.length)}` : target; +/** Rewrite a branch path through the deepest move that covers it, in any move order. */ +export function remapPath(target: string, moves: readonly ContainerMove[]): string { + let move: ContainerMove | undefined; + for (const m of moves) { + if (target !== m.from && !isUnder(target, m.from)) continue; + if (move === undefined || m.from.length > move.from.length) move = m; + } + return move ? `${move.to}${target.slice(move.from.length)}` : target; } /** diff --git a/packages/cli/src/modules/promotions.ee/import-manifest-bridge.ts b/packages/cli/src/modules/promotions.ee/import-manifest-bridge.ts index ce9f27e709a..b8950b6bb13 100644 --- a/packages/cli/src/modules/promotions.ee/import-manifest-bridge.ts +++ b/packages/cli/src/modules/promotions.ee/import-manifest-bridge.ts @@ -12,6 +12,8 @@ import type { PackageRequirements, } from '@/modules/n8n-packages/spec/requirements.schema'; +import { remapPath, type ContainerMove } from './branch-placement'; + /** * TEMPORARY bridge: import still inventories a directory package from * `manifest.json`. Apply writes that file after overlay. @@ -89,14 +91,22 @@ export async function writeImportManifest(options: { staging: PackageManifest; sourceId: string; selectedWorkflowIds?: readonly string[]; + /** Container renames already applied to the export, so leftover targets resolve. */ + containerMoves?: readonly ContainerMove[]; }): Promise { - const { exportFolder, staging, sourceId, selectedWorkflowIds = [] } = options; + const { + exportFolder, + staging, + sourceId, + selectedWorkflowIds = [], + containerMoves = [], + } = options; const selected = [...selectedWorkflowIds, ...(staging.workflows ?? []).map((entry) => entry.id)]; const leftover = dropSelectedRequirementUsers(await readLeftoverManifest(exportFolder), selected); const collections = await walkSnapshotCollections(exportFolder); const remainingWorkflowIds = new Set((collections.workflows ?? []).map((entry) => entry.id)); const selectedSet = new Set(selected); - const variables = await collectVariables(exportFolder, leftover, staging); + const variables = await collectVariables(exportFolder, leftover, staging, containerMoves); const manifest = packageManifestSchema.parse({ packageFormatVersion: '1', @@ -174,10 +184,11 @@ async function collectVariables( exportFolder: string, leftover: PackageManifest | undefined, staging: PackageManifest, + containerMoves: readonly ContainerMove[], ): Promise { const byId = new Map(); for (const entry of leftover?.variables ?? []) { - byId.set(entry.id, entry); + byId.set(entry.id, { ...entry, target: remapPath(entry.target, containerMoves) }); } for (const entry of staging.variables ?? []) { byId.set(entry.id, entry); diff --git a/packages/cli/src/modules/promotions.ee/promotions.service.ts b/packages/cli/src/modules/promotions.ee/promotions.service.ts index b5f6af02158..b93794b7b6c 100644 --- a/packages/cli/src/modules/promotions.ee/promotions.service.ts +++ b/packages/cli/src/modules/promotions.ee/promotions.service.ts @@ -249,9 +249,9 @@ export class PromotionsService { /** * Selective promote against an already-resolved connection. Unselected workflows - * stay as-is, and so do the projects and folders the branch already holds: a - * selection creates a container, never renames one, so nothing moves that the - * user did not select. A promotion branch is always new, so force never applies. + * keep their content. A renamed project or folder moves its branch directory, + * so the unselected workflows inside it move with it. A promotion branch is + * always new, so force never applies. */ private async promoteSelectionResolved( input: PromotionOperationInput, diff --git a/packages/cli/src/modules/promotions.ee/working-copy-updater.ts b/packages/cli/src/modules/promotions.ee/working-copy-updater.ts index 6bfbc6cb28e..1d42ac0beb3 100644 --- a/packages/cli/src/modules/promotions.ee/working-copy-updater.ts +++ b/packages/cli/src/modules/promotions.ee/working-copy-updater.ts @@ -11,8 +11,8 @@ import { BadRequestError, InternalServerError } from '@n8n/errors'; import { MANIFEST_FILE } from '@/modules/n8n-packages/spec/constants'; import type { ManifestEntry, PackageManifest } from '@/modules/n8n-packages/spec/manifest.schema'; -import { containerPlacement, isUnder, pinPath, staleWorkflowTargets } from './branch-placement'; -import type { BranchLayout, Placement } from './branch-placement'; +import { containerMoves, isUnder, staleWorkflowTargets } from './branch-placement'; +import type { BranchLayout, ContainerMove } from './branch-placement'; import { writeImportManifest } from './import-manifest-bridge'; import { PromotionsWorkflowsMovedCrossProjectError } from './promotions-selective-push.error'; @@ -315,6 +315,28 @@ export class WorkingCopyUpdater { } } + /** + * A container the branch holds under another project moved between projects. + * Moving it would carry that project's workflows along, so a selective push + * refuses it. A project's own directory may move (a rename). + */ + private assertNoCrossProjectContainerMoves( + branch: BranchLayout, + projectId: string, + moves: readonly ContainerMove[], + ): void { + const projectTarget = branch.projects?.find((p) => p.id === projectId)?.target; + const foreign = moves.find( + ({ from }) => + projectTarget === undefined || !(from === projectTarget || isUnder(from, projectTarget)), + ); + if (foreign) { + throw new BadRequestError( + `"${foreign.from}" belongs to another project on the branch. A selective push cannot move it. Push all projects instead.`, + ); + } + } + /** * Overlay the staging export onto `exportFolder`. Pass `branch` from * `assertSelectionFitsBranch` for the same folder and selection. @@ -345,10 +367,11 @@ export class WorkingCopyUpdater { try { const { state: existing, dependencies } = branch; - const placement = containerPlacement(existing, staging); const otherProjectTargets = (existing.projects ?? []) .filter((p) => p.id !== selection.projectId) .map((p) => p.target); + const moves = containerMoves(existing, staging); + this.assertNoCrossProjectContainerMoves(existing, selection.projectId, moves); const remaining: BranchLayout = { ...existing, workflows: (existing.workflows ?? []).filter( @@ -389,19 +412,21 @@ export class WorkingCopyUpdater { otherProjectTargets, ); for (const { collection, target } of renamedInScope) { - const dir = await this.resolveDependencyDir(workFolder, target); + const dir = await this.resolveLeafTarget(workFolder, target, 'Dependency'); await fs.rm(path.join(dir, DEPENDENCY_FILE_BY_COLLECTION[collection]), { force: true }); if ((await fs.readdir(dir)).length === 0) await fs.rmdir(dir); } - for (const target of outOfScopeInStaging) { - placement.keptFiles.add(target); - } - await this.overlayDirectory(stagingFolder, workFolder, placement); + + // After the cleanups above, which address branch paths. + await this.relocateBranchContainers(workFolder, moves); + + await this.overlayDirectory(stagingFolder, workFolder, outOfScopeInStaging); await writeImportManifest({ exportFolder: workFolder, staging, sourceId: this.instanceSettings.instanceId, selectedWorkflowIds: selection.workflowIds, + containerMoves: moves, }); const backupPath = path.join(tempBase, `.${path.basename(exportFolder)}-bak-${randomUUID()}`); @@ -464,19 +489,7 @@ export class WorkingCopyUpdater { remaining: BranchLayout, staging: PackageManifest, ): Promise { - const segments = target.split(/[\\/]/).filter(Boolean); - if (segments.length === 0 || segments.includes('.') || segments.includes('..')) { - throw new BadRequestError( - `Manifest target "${target}" is not a managed leaf directory. Remove it and retry.`, - ); - } - - const resolved = await this.resolveContained(exportFolder, target); - if (resolved === path.resolve(exportFolder)) { - throw new BadRequestError( - `Manifest target "${target}" is not a managed leaf directory. Remove it and retry.`, - ); - } + const resolved = await this.resolveLeafTarget(exportFolder, target, 'Manifest'); const writtenIds = new Set((staging.workflows ?? []).map((entry) => entry.id)); for (const entry of remaining.workflows ?? []) { @@ -524,7 +537,7 @@ export class WorkingCopyUpdater { otherProjectTargets: string[], ): { renamedInScope: Array<{ collection: DependencyCollection; target: string }>; - outOfScopeInStaging: string[]; + outOfScopeInStaging: Set; } { const stagingTargets = new Map(); for (const collection of DEPENDENCY_COLLECTIONS) { @@ -534,11 +547,7 @@ export class WorkingCopyUpdater { } const renamedInScope: Array<{ collection: DependencyCollection; target: string }> = []; - const outOfScopeInStaging: string[] = []; - - if (stagingTargets.size === 0) { - return { renamedInScope, outOfScopeInStaging }; - } + const outOfScopeInStaging = new Set(); for (const dependency of dependencies) { const newTarget = stagingTargets.get(`${dependency.collection}:${dependency.id}`); @@ -546,7 +555,7 @@ export class WorkingCopyUpdater { const underOtherProject = otherProjectTargets.some((pt) => isUnder(dependency.target, pt)); if (underOtherProject) { - outOfScopeInStaging.push(newTarget); + outOfScopeInStaging.add(newTarget); } else { renamedInScope.push({ collection: dependency.collection, target: dependency.target }); } @@ -573,34 +582,78 @@ export class WorkingCopyUpdater { return undefined; } - /** - * Resolve a dependency directory to remove a file from, rejecting a non-leaf - * target, an escape, or a symlinked path (via `resolveContained`). - */ - private async resolveDependencyDir(exportFolder: string, target: string): Promise { - const segments = target.split(/[\\/]/).filter(Boolean); - if (segments.length === 0 || segments.includes('.') || segments.includes('..')) { - throw new BadRequestError( - `Dependency target "${target}" is not a managed leaf directory. Remove it and retry.`, + /** Resolve a target that will be removed or moved. Rejects the export root itself. */ + private async resolveLeafTarget( + exportFolder: string, + target: string, + label: 'Manifest' | 'Dependency' | 'Container', + ): Promise { + const reject = () => + new BadRequestError( + `${label} target "${target}" is not a managed leaf directory. Remove it and retry.`, ); - } + const segments = target.split(/[\\/]/).filter(Boolean); + if (segments.length === 0 || segments.includes('.') || segments.includes('..')) throw reject(); const resolved = await this.resolveContained(exportFolder, target); - if (resolved === path.resolve(exportFolder)) { - throw new BadRequestError( - `Dependency target "${target}" is not a managed leaf directory. Remove it and retry.`, - ); - } - + if (resolved === path.resolve(exportFolder)) throw reject(); return resolved; } /** - * Copy the staging export into `dest`, each file where `placement` puts it. - * A file the branch keeps is skipped, so a rename on the instance leaves the - * `project.json` or `folder.json` the branch holds alone. + * Move each renamed container directory to its export path, so unselected + * workflows inside it ride along. All sources are parked in a holding + * directory before any is placed, so a swap (A<->B) never clobbers a source. + * Park deepest `from` first; place shallowest `to` first. */ - private async overlayDirectory(src: string, dest: string, placement: Placement): Promise { + private async relocateBranchContainers( + workFolder: string, + moves: readonly ContainerMove[], + ): Promise { + if (moves.length === 0) return; + + // A root entity file scans as target `.`, which would park the whole copy. + const sources = await Promise.all( + moves.map(async ({ from }) => await this.resolveLeafTarget(workFolder, from, 'Container')), + ); + const destinations = await Promise.all( + moves.map(async ({ to }) => await this.resolveLeafTarget(workFolder, to, 'Container')), + ); + + const stash = await fs.mkdtemp(path.join(path.dirname(workFolder), '.relocate-')); + try { + const parked: Array<{ holding: string; dest: string; move: ContainerMove }> = []; + for (const [index, move] of moves.entries()) { + const holding = path.join(stash, String(index)); + await fs.rename(sources[index], holding); + parked.push({ holding, dest: destinations[index], move }); + } + + parked.sort((a, b) => a.move.to.length - b.move.to.length); + for (const { holding, dest, move } of parked) { + // After parking, so a case-only rename does not collide with its own source. + if (await fs.lstat(dest).catch(() => null)) { + throw new BadRequestError( + `The branch path "${move.to}" is already in use. Remove it and retry.`, + ); + } + await fs.mkdir(path.dirname(dest), { recursive: true }); + await fs.rename(holding, dest); + } + } finally { + await fs.rm(stash, { recursive: true, force: true }); + } + } + + /** + * Copy the staging export into `dest`. Skips `keptFiles`: out-of-scope + * dependencies the branch owns elsewhere. + */ + private async overlayDirectory( + src: string, + dest: string, + keptFiles: ReadonlySet, + ): Promise { const verified = new Set(); const walk = async (dir: string): Promise => { const entries = await fs.readdir(dir, { withFileTypes: true }); @@ -609,13 +662,9 @@ export class WorkingCopyUpdater { const fullPath = path.join(dir, entry.name); const relative = path.relative(src, fullPath).split(path.sep).join('/'); - if (relative === MANIFEST_FILE || placement.keptFiles.has(relative)) continue; + if (relative === MANIFEST_FILE || keptFiles.has(relative)) continue; - const destPath = await this.resolveContained( - dest, - pinPath(relative, placement.pins), - verified, - ); + const destPath = await this.resolveContained(dest, relative, verified); if (entry.isDirectory()) { await fs.mkdir(destPath, { recursive: true }); await walk(fullPath); diff --git a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/promoteErrorMessage.ts b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/promoteErrorMessage.ts index a651c74d11d..02400b46965 100644 --- a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/promoteErrorMessage.ts +++ b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/promoteErrorMessage.ts @@ -20,14 +20,14 @@ export function getPromoteErrorMessage( return undefined; } - const meta = parsePromotionsWorkflowsMovedCrossProjectMeta(error.meta); - if (!meta) { - return undefined; + const movedMeta = parsePromotionsWorkflowsMovedCrossProjectMeta(error.meta); + if (movedMeta) { + return i18n.baseText('promotions.modal.promoteError.workflowsMovedCrossProject', { + interpolate: { + workflows: resolveWorkflowTitles(movedMeta.workflowIds, changes), + }, + }); } - return i18n.baseText('promotions.modal.promoteError.workflowsMovedCrossProject', { - interpolate: { - workflows: resolveWorkflowTitles(meta.workflowIds, changes), - }, - }); + return undefined; }