From e0fe04084e77d296e2daf1767423a56d66455cee Mon Sep 17 00:00:00 2001 From: Kai Date: Tue, 13 Oct 2026 14:31:19 +0000 Subject: [PATCH] fix(core): Allow project promotion without an instance export (#40939) --- .../__tests__/promotions-git.service.test.ts | 24 ++- .../promotions-package.integration.test.ts | 137 +++++++++++++++++- .../__tests__/promotions.service.test.ts | 17 ++- .../promotions.ee/promotions-git.service.ts | 42 +++++- .../promotions.ee/promotions.service.ts | 52 +++++-- 5 files changed, 235 insertions(+), 37 deletions(-) diff --git a/packages/cli/src/modules/promotions.ee/__tests__/promotions-git.service.test.ts b/packages/cli/src/modules/promotions.ee/__tests__/promotions-git.service.test.ts index a6467d0bdbd..db1a274e3d8 100644 --- a/packages/cli/src/modules/promotions.ee/__tests__/promotions-git.service.test.ts +++ b/packages/cli/src/modules/promotions.ee/__tests__/promotions-git.service.test.ts @@ -462,16 +462,19 @@ describe('PromotionsGitService (git operations)', () => { }); it('restores HEAD and the index when the push fails', async () => { - mockGit.revparse.mockResolvedValueOnce('before\n').mockResolvedValueOnce('after\n'); + mockGit.raw.mockResolvedValueOnce('before\n'); mockGit.push.mockRejectedValueOnce(new Error('push rejected')); await expect(call({ rollbackOnFailure: true })).rejects.toThrow(BadRequestError); - expect(mockGit.revparse.mock.invocationCallOrder[0]).toBeLessThan( + expect(mockGit.raw.mock.calls).toEqual([ + [['for-each-ref', '--format=%(objectname)', 'refs/heads/main']], + [['reset', '--mixed', 'before']], + ]); + expect(mockGit.raw.mock.invocationCallOrder[0]).toBeLessThan( mockGit.add.mock.invocationCallOrder[0], ); - expect(mockGit.raw).toHaveBeenCalledExactlyOnceWith(['reset', '--mixed', 'before']); - expect(mockGit.raw.mock.invocationCallOrder[0]).toBeGreaterThan( + expect(mockGit.raw.mock.invocationCallOrder[1]).toBeGreaterThan( mockGit.push.mock.invocationCallOrder[0], ); }); @@ -479,7 +482,9 @@ describe('PromotionsGitService (git operations)', () => { it('keeps the push error when rollback fails', async () => { const pushError = new ServiceUnavailableError('Push timed out'); mockGit.push.mockRejectedValueOnce(pushError); - mockGit.raw.mockRejectedValueOnce(new Error('reset failed')); + mockGit.raw + .mockResolvedValueOnce('before\n') + .mockRejectedValueOnce(new Error('reset failed')); await expect(call({ rollbackOnFailure: true })).rejects.toBe(pushError); @@ -490,12 +495,17 @@ describe('PromotionsGitService (git operations)', () => { }); it('keeps the new commit when the push succeeds', async () => { - mockGit.revparse.mockResolvedValueOnce('before\n').mockResolvedValueOnce('after\n'); + mockGit.raw.mockResolvedValueOnce('before\n'); + mockGit.revparse.mockResolvedValueOnce('after\n'); await expect(call({ rollbackOnFailure: true })).resolves.toEqual({ commitSha: 'after' }); expect(mockGit.push).toHaveBeenCalledExactlyOnceWith('origin', 'main'); - expect(mockGit.raw).not.toHaveBeenCalled(); + expect(mockGit.raw).toHaveBeenCalledExactlyOnceWith([ + 'for-each-ref', + '--format=%(objectname)', + 'refs/heads/main', + ]); }); it('redacts a push failure and keeps raw git output out of the log', async () => { 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 f9fd437640d..dcbee7a3dba 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 @@ -1934,19 +1934,140 @@ describe('Apply data table changes', () => { }); describe('Promote a project selection — branch effects', () => { - it('refuses a selection when the branch has no package yet', async () => { + it('promotes a first selection to a branch without a package, then adds to it', async () => { const remote = await createRemote(); const connection = await createInstanceConnection(remote.bareDir); await service.clone(connection.id, 'promote'); + const { project, workflows } = await setupProjectWithWorkflows('Orders', ['w1', 'w2']); + await setupProjectWithWorkflows('Billing', ['b1']); + const promoteWorkflow = async (workflowId: string) => + await service.promoteProjectSelection(project.id, owner, { + workflowIds: [workflowId], + commitMessage: 'Promote selection', + canExportVariableValues: true, + }); + + await promoteWorkflow(workflows[0].id); + + const first = await inspectBranch(remote.bareDir); + expect((await readBranchEntities(first.dir, 'workflow.json')).map((w) => w.id)).toEqual([ + workflows[0].id, + ]); + expect( + (await readBranchEntities(first.dir, PACKAGE_ENTITY_LAYOUT.projects.fileName)).map( + (p) => p.id, + ), + ).toEqual([project.id]); + await expect(readFile(path.join(first.dir, 'README.md'), 'utf-8')).resolves.toBe( + '# n8n promotions test\n', + ); + + await promoteWorkflow(workflows[1].id); + + const second = await inspectBranch(remote.bareDir); + expect((await readBranchEntities(second.dir, 'workflow.json')).map((w) => w.id).sort()).toEqual( + workflows.map((w) => w.id).sort(), + ); + }); + + it.each(['n8n-export', 'n8n-export/manifest.json'])( + 'refuses a selection when the local copy lost committed %s, until it connects again', + async (lostPath) => { + const remote = await createRemote(); + const connection = await createInstanceConnection(remote.bareDir); + const { configId } = await service.clone(connection.id, 'promote'); + const { project, workflows } = await setupProjectWithWorkflows('Orders', ['w1', 'w2']); + const promoteWorkflow = async (workflowId: string) => + await service.promoteProjectSelection(project.id, owner, { + workflowIds: [workflowId], + commitMessage: 'Promote selection', + canExportVariableValues: true, + }); + await promoteWorkflow(workflows[0].id); + const remoteGit = simpleGit(remote.bareDir); + const promotedCommit = (await remoteGit.revparse(['main'])).trim(); + + const { repositoryFolder } = workingDirectory.paths(configId); + await rm(path.join(repositoryFolder, lostPath), { recursive: true, force: true }); + + await expect(promoteWorkflow(workflows[1].id)).rejects.toThrow( + 'The local copy of the promote branch is incomplete', + ); + expect((await remoteGit.revparse(['main'])).trim()).toBe(promotedCommit); + + await service.clone(connection.id, 'promote'); + await promoteWorkflow(workflows[1].id); + + const { dir } = await inspectBranch(remote.bareDir); + expect((await readBranchEntities(dir, 'workflow.json')).map((w) => w.id).sort()).toEqual( + workflows.map((w) => w.id).sort(), + ); + }, + ); + + it('pushes a first selection to an empty repository after a failed attempt', async () => { + const bareDir = path.join(testRoot, 'empty-remote.git'); + await simpleGit().raw(['init', '--bare', bareDir]); + const connection = await createInstanceConnection(bareDir); + const { configId } = await service.clone(connection.id, 'promote'); + const { repositoryFolder } = workingDirectory.paths(configId); + const checkoutGit = simpleGit(repositoryFolder); + const { project, workflows } = await setupProjectWithWorkflows('Orders', ['w1']); + const request = { + workflowIds: [workflows[0].id], + commitMessage: 'Add w1', + canExportVariableValues: true, + }; + + await rename(bareDir, `${bareDir}.offline`); + await expect(service.promoteProjectSelection(project.id, owner, request)).rejects.toThrow(); + + // The checkout is back to a branch with no commits and no package. + await expect(stat(path.join(repositoryFolder, 'n8n-export'))).rejects.toMatchObject({ + code: 'ENOENT', + }); + expect( + (await checkoutGit.raw(['for-each-ref', '--format=%(objectname)', 'refs/heads/main'])).trim(), + ).toBe(''); + expect((await checkoutGit.status()).isClean()).toBe(true); + + await rename(`${bareDir}.offline`, bareDir); + await service.promoteProjectSelection(project.id, owner, request); + + expect((await simpleGit(bareDir).raw(['rev-list', '--count', 'main'])).trim()).toBe('1'); + const { dir } = await inspectBranch(bareDir); + expect((await readBranchEntities(dir, 'workflow.json')).map((w) => w.id)).toEqual([ + workflows[0].id, + ]); + }); + + it('pushes a first branched selection off a base without a package', async () => { + const remote = await createRemote(); + const connection = await createInstanceConnection( + remote.bareDir, + { apply: 'main', promote: 'main' }, + true, + ); + const { configId } = await service.clone(connection.id, 'promote'); + const baseCommit = (await remote.git.revparse(['main'])).trim(); const { project, workflows } = await setupProjectWithWorkflows('Orders', ['w1']); - await expect( - service.promoteProjectSelection(project.id, owner, { - workflowIds: [workflows[0].id], - commitMessage: 'Add w1', - canExportVariableValues: true, - }), - ).rejects.toThrow('Promote the instance first'); + const result = await service.promoteProjectSelection(project.id, owner, { + workflowIds: [workflows[0].id], + commitMessage: 'Add w1', + canExportVariableValues: true, + }); + + const remoteGit = simpleGit(remote.bareDir); + expect((await remoteGit.revparse([`${result.git.branchName}^`])).trim()).toBe(baseCommit); + expect((await remoteGit.revparse(['main'])).trim()).toBe(baseCommit); + const { repositoryFolder } = workingDirectory.paths(configId); + expect((await simpleGit(repositoryFolder).revparse(['HEAD'])).trim()).toBe(baseCommit); + + const { dir } = await inspectBranch(remote.bareDir, result.git.branchName); + expect((await readBranchEntities(dir, 'workflow.json')).map((w) => w.id)).toEqual([ + workflows[0].id, + ]); }); it('adds only the selected workflow, leaving existing workflows untouched', async () => { diff --git a/packages/cli/src/modules/promotions.ee/__tests__/promotions.service.test.ts b/packages/cli/src/modules/promotions.ee/__tests__/promotions.service.test.ts index 263fe5c8ef8..792e2697253 100644 --- a/packages/cli/src/modules/promotions.ee/__tests__/promotions.service.test.ts +++ b/packages/cli/src/modules/promotions.ee/__tests__/promotions.service.test.ts @@ -34,7 +34,7 @@ import { WorkingCopyUpdater } from '../working-copy-updater'; vi.mock('node:fs/promises', async (importOriginal) => { const fs = await importOriginal(); - return { ...fs, rm: vi.fn(fs.rm) }; + return { ...fs, rm: vi.fn(fs.rm), stat: vi.fn(fs.stat) }; }); const CONFIG_ID = 'cfg1'; @@ -149,6 +149,7 @@ describe('PromotionsService', () => { afterEach(async () => { vi.mocked(rm).mockReset(); + vi.mocked(stat).mockReset(); await rm(n8nFolder, { recursive: true, force: true }); }); @@ -668,17 +669,23 @@ describe('PromotionsService', () => { ); }); - it('refuses a selection when the branch has no package', async () => { - await mkdir(repositoryFolder, { recursive: true }); + it('fails without exporting when the package folder cannot be read', async () => { + const readError = Object.assign(new Error('permission denied'), { code: 'EACCES' }); + const { stat: realStat } = + await vi.importActual('node:fs/promises'); + vi.mocked(stat).mockImplementation(async (target, options) => { + if (target === packageFolder) throw readError; + return await realStat(target, options); + }); ownedByP1('w1'); await expect( service.promoteProjectSelection('p1', actor, { workflowIds: ['w1'], - commitMessage: 'first selective', + commitMessage: 'm', canExportVariableValues: true, }), - ).rejects.toThrow('Promote the instance first'); + ).rejects.toBe(readError); expect(n8nPackagesService.exportPackageToDirectory).not.toHaveBeenCalled(); expect(gitService.commitAndPush).not.toHaveBeenCalled(); }); diff --git a/packages/cli/src/modules/promotions.ee/promotions-git.service.ts b/packages/cli/src/modules/promotions.ee/promotions-git.service.ts index a48bb740c46..ce8ffbfe58d 100644 --- a/packages/cli/src/modules/promotions.ee/promotions-git.service.ts +++ b/packages/cli/src/modules/promotions.ee/promotions-git.service.ts @@ -261,8 +261,15 @@ export class PromotionsGitService { }); } + // Empty on a repository with no commits yet. const previousHead = rollbackOnFailure - ? (await git.revparse(['HEAD'])).trim() + ? ( + await git.raw([ + 'for-each-ref', + '--format=%(objectname)', + `refs/heads/${branchName}`, + ]) + ).trim() : undefined; try { // Scope staging to the package while including removed entities. @@ -278,10 +285,16 @@ export class PromotionsGitService { return { commitSha }; } catch (error) { - if (previousHead) { + if (previousHead !== undefined) { try { // Restore Git state. The caller restores the package files. - await git.raw(['reset', '--mixed', previousHead]); + if (previousHead) { + await git.raw(['reset', '--mixed', previousHead]); + } else { + // Remove the first commit and clear the index. + await git.raw(['update-ref', '-d', 'HEAD']); + await git.raw(['read-tree', '--empty']); + } } catch { this.logger.warn( 'Failed to restore the Git revision after a failed promotion', @@ -472,6 +485,29 @@ export class PromotionsGitService { } } + /** Reports whether the local branch's last commit holds this path. False when it has no commits. */ + async hasCommittedPath({ + paths, + branchName, + configId, + pathspec, + }: Pick & { + pathspec: string; + }): Promise { + try { + return await this.lockCheckout(paths.repositoryFolder, async () => { + const git = simpleGit({ ...BASE_GIT_OPTIONS, baseDir: paths.repositoryFolder }); + const head = ( + await git.raw(['for-each-ref', '--format=%(objectname)', `refs/heads/${branchName}`]) + ).trim(); + if (!head) return false; + return (await git.raw(['ls-tree', head, '--', pathspec])).trim() !== ''; + }); + } catch (error) { + throw this.mapGitError(error, { configId, branchName }); + } + } + async readFilesAtCommit({ paths, branchName, diff --git a/packages/cli/src/modules/promotions.ee/promotions.service.ts b/packages/cli/src/modules/promotions.ee/promotions.service.ts index c0534b08a4b..9d36fb59f64 100644 --- a/packages/cli/src/modules/promotions.ee/promotions.service.ts +++ b/packages/cli/src/modules/promotions.ee/promotions.service.ts @@ -275,9 +275,20 @@ export class PromotionsService { const { repositoryFolder } = this.workingDirectory.paths(input.configId); const packageFolder = path.join(repositoryFolder, PACKAGE_SUBFOLDER); - if (!(await this.hasExportedPackage(packageFolder))) { + // No package yet means this selection becomes the first export. + const packageExisted = await pathExists(packageFolder); + if ( + !(await pathExists(path.join(packageFolder, MANIFEST_FILE))) && + (await this.gitService.hasCommittedPath({ + paths: this.workingDirectory.paths(input.configId), + branchName, + configId: input.configId, + pathspec: `${PACKAGE_SUBFOLDER}/${MANIFEST_FILE}`, + })) + ) { + // The commit holds a manifest the checkout lost. A push would drop branch content or metadata. throw new BadRequestError( - 'The local checkout has no exported package. Promote the instance first, then promote a selection.', + 'The local copy of the promote branch is incomplete. Disconnect and connect it again in the instance connection settings.', ); } @@ -289,9 +300,11 @@ export class PromotionsService { let keepPrePushBackup = false; try { - await rm(prePushBackup, { recursive: true, force: true }); - await cp(packageFolder, prePushBackup, { recursive: true, verbatimSymlinks: true }); - backedUp = true; + if (packageExisted) { + await rm(prePushBackup, { recursive: true, force: true }); + await cp(packageFolder, prePushBackup, { recursive: true, verbatimSymlinks: true }); + backedUp = true; + } // The exporter does the selecting: it writes the selected workflows and // what they need, so nothing has to be filtered out afterwards. @@ -366,10 +379,18 @@ export class PromotionsService { { packageFolder, backupFolder: prePushBackup, error: restoreError }, ); } + } else if (!packageExisted) { + await rm(packageFolder, { recursive: true, force: true }).catch((cleanupError: unknown) => { + this.logger.warn('Failed to remove the first selection after a failed promote', { + packageFolder, + error: cleanupError, + }); + }); } throw error; } finally { - if (!keepPrePushBackup) { + // A first push has no backup of its own. Keep any backup left by an earlier failure. + if (packageExisted && !keepPrePushBackup) { await rm(prePushBackup, { recursive: true, force: true }).catch((error: unknown) => { this.logger.warn('Failed to remove the selection backup', { prePushBackup, error }); }); @@ -836,14 +857,6 @@ export class PromotionsService { } } - private async hasExportedPackage(packageFolder: string): Promise { - try { - return (await stat(path.join(packageFolder, MANIFEST_FILE))).isFile(); - } catch { - return false; - } - } - /** * A checkout counts as ready only when it exists and was cloned from the * configuration we just resolved. Another process may have changed the remote or @@ -968,6 +981,17 @@ export class PromotionsService { } } +/** Throws on any error except a missing path, so an unreadable package never looks empty. */ +async function pathExists(target: string): Promise { + try { + await stat(target); + return true; + } catch (error) { + if (error instanceof Error && 'code' in error && error.code === 'ENOENT') return false; + throw error; + } +} + async function isDirectory(folder: string): Promise { try { return (await stat(folder)).isDirectory();