From ce8fc2a4b6b95a84d80f9f4ec28066d811225661 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ir=C3=A9n=C3=A9e?= Date: Fri, 12 Dec 2025 16:26:58 +0000 Subject: [PATCH] fix: Reset git files when push fails (#23142) --- .../__tests__/source-control.service.test.ts | 134 ++++++++++++++++++ .../source-control.service.ee.ts | 134 +++++++++++------- 2 files changed, 214 insertions(+), 54 deletions(-) diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts index fd1c07aa8bc..ba2c1f0f7aa 100644 --- a/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts @@ -338,6 +338,140 @@ describe('SourceControlService', () => { }); expect(result).toHaveProperty('statusCode', 200); }); + + it('should reset branch to HEAD when export fails', async () => { + // ARRANGE + const user = mock(); + + const mockFile: SourceControlledFile = { + file: 'workflow-1.json', + id: 'wf-1', + name: 'Workflow 1', + type: 'workflow', + status: 'modified', + location: 'local', + conflict: false, + updatedAt: new Date().toISOString(), + }; + + mockStatusService.getStatus.mockResolvedValueOnce([mockFile]); + + (isContainedWithin as jest.Mock).mockReturnValue(true); + + // Mock workflow export to fail + const exportError = new Error('Failed to export workflows'); + sourceControlExportService.exportWorkflowsToWorkFolder.mockRejectedValueOnce(exportError); + + // ACT & ASSERT + await expect( + sourceControlService.pushWorkfolder(user, { + fileNames: [mockFile], + commitMessage: 'Test commit', + }), + ).rejects.toThrow(exportError); + + // Verify no git operations were performed + expect(gitService.stage).not.toHaveBeenCalled(); + expect(gitService.commit).not.toHaveBeenCalled(); + expect(gitService.push).not.toHaveBeenCalled(); + + // Verify resetBranch was called with HEAD to clean up any potential state + expect(gitService.resetBranch).toHaveBeenCalledWith({ hard: true, target: 'HEAD' }); + }); + + it('should reset branch to origin/branch when push fails after commit', async () => { + // ARRANGE + const user = mock(); + const mockFile: SourceControlledFile = { + file: 'workflow-1.json', + id: 'wf-1', + name: 'Workflow 1', + type: 'workflow', + status: 'modified', + location: 'local', + conflict: false, + updatedAt: new Date().toISOString(), + }; + + mockStatusService.getStatus.mockResolvedValueOnce([mockFile]); + sourceControlExportService.exportCredentialsToWorkFolder.mockResolvedValueOnce({ + count: 0, + missingIds: [], + folder: '', + files: [], + }); + + (isContainedWithin as jest.Mock).mockReturnValue(true); + + const pushError = new Error( + 'To github.com:test/n8n.git ! refs/heads/test:refs/heads/test [remote rejected] (push declined due to repository rule violations)', + ); + gitService.push.mockRejectedValueOnce(pushError); + + // ACT & ASSERT + await expect( + sourceControlService.pushWorkfolder(user, { + fileNames: [mockFile], + commitMessage: 'Test commit', + }), + ).rejects.toThrow(pushError); + + // Verify git operations were attempted + expect(gitService.stage).toHaveBeenCalled(); + expect(gitService.commit).toHaveBeenCalled(); + expect(gitService.push).toHaveBeenCalled(); + + // Verify resetBranch was called with origin/branch to sync with remote + expect(gitService.resetBranch).toHaveBeenCalledWith({ + hard: true, + target: 'origin/main', + }); + }); + + it('should reset branch to HEAD when commit fails', async () => { + // ARRANGE + const user = mock(); + const mockFile: SourceControlledFile = { + file: 'workflow-1.json', + id: 'wf-1', + name: 'Workflow 1', + type: 'workflow', + status: 'modified', + location: 'local', + conflict: false, + updatedAt: new Date().toISOString(), + }; + + mockStatusService.getStatus.mockResolvedValueOnce([mockFile]); + sourceControlExportService.exportCredentialsToWorkFolder.mockResolvedValueOnce({ + count: 0, + missingIds: [], + folder: '', + files: [], + }); + + (isContainedWithin as jest.Mock).mockReturnValue(true); + + // Mock commit to fail + const commitError = new Error('Git commit failed'); + gitService.commit.mockRejectedValueOnce(commitError); + + // ACT & ASSERT + await expect( + sourceControlService.pushWorkfolder(user, { + fileNames: [mockFile], + commitMessage: 'Test commit', + }), + ).rejects.toThrow(commitError); + + // Verify stage was called but not push + expect(gitService.stage).toHaveBeenCalled(); + expect(gitService.commit).toHaveBeenCalled(); + expect(gitService.push).not.toHaveBeenCalled(); + + // Verify resetBranch was called with HEAD (no commit to undo) + expect(gitService.resetBranch).toHaveBeenCalledWith({ hard: true, target: 'HEAD' }); + }); }); describe('pullWorkfolder', () => { diff --git a/packages/cli/src/environments.ee/source-control/source-control.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control.service.ee.ts index 130aca3e680..15df47c3876 100644 --- a/packages/cli/src/environments.ee/source-control/source-control.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control.service.ee.ts @@ -287,71 +287,97 @@ export class SourceControlService { } } - const filesToBePushed = new Set(); - const filesToBeDeleted = new Set(); + try { + const filesToBePushed = new Set(); + const filesToBeDeleted = new Set(); - /* + /* Exclude tags, variables and folders JSON file from being deleted as we keep track of them in a single file unlike workflows and credentials */ - filesToPush - .filter((f) => ['workflow', 'credential', 'project'].includes(f.type)) - .forEach((e) => { - if (e.status !== 'deleted') { - filesToBePushed.add(e.file); - } else { - filesToBeDeleted.add(e.file); - } - }); + filesToPush + .filter((f) => ['workflow', 'credential', 'project'].includes(f.type)) + .forEach((e) => { + if (e.status !== 'deleted') { + filesToBePushed.add(e.file); + } else { + filesToBeDeleted.add(e.file); + } + }); - this.sourceControlExportService.rmFilesFromExportFolder(filesToBeDeleted); + this.sourceControlExportService.rmFilesFromExportFolder(filesToBeDeleted); - const workflowsToBeExported = getNonDeletedResources(filesToPush, 'workflow'); - await this.sourceControlExportService.exportWorkflowsToWorkFolder(workflowsToBeExported); + const workflowsToBeExported = getNonDeletedResources(filesToPush, 'workflow'); + await this.sourceControlExportService.exportWorkflowsToWorkFolder(workflowsToBeExported); - const credentialsToBeExported = getNonDeletedResources(filesToPush, 'credential'); - const credentialExportResult = - await this.sourceControlExportService.exportCredentialsToWorkFolder(credentialsToBeExported); - if (credentialExportResult.missingIds && credentialExportResult.missingIds.length > 0) { - credentialExportResult.missingIds.forEach((id) => { - filesToBePushed.delete(this.sourceControlExportService.getCredentialsPath(id)); - statusResult = statusResult.filter( - (e) => e.file !== this.sourceControlExportService.getCredentialsPath(id), + const credentialsToBeExported = getNonDeletedResources(filesToPush, 'credential'); + const credentialExportResult = + await this.sourceControlExportService.exportCredentialsToWorkFolder( + credentialsToBeExported, ); + if (credentialExportResult.missingIds && credentialExportResult.missingIds.length > 0) { + credentialExportResult.missingIds.forEach((id) => { + filesToBePushed.delete(this.sourceControlExportService.getCredentialsPath(id)); + statusResult = statusResult.filter( + (e) => e.file !== this.sourceControlExportService.getCredentialsPath(id), + ); + }); + } + + const projectsToBeExported = getNonDeletedResources(filesToPush, 'project'); + await this.sourceControlExportService.exportTeamProjectsToWorkFolder(projectsToBeExported); + + // The tags file is always re-generated and exported to make sure the workflow-tag mappings are up to date + filesToBePushed.add(getTagsPath(this.gitFolder)); + await this.sourceControlExportService.exportTagsToWorkFolder(context); + + const folderChanges = filterByType(filesToPush, 'folders')[0]; + if (folderChanges) { + filesToBePushed.add(folderChanges.file); + await this.sourceControlExportService.exportFoldersToWorkFolder(context); + } + + const variablesChanges = filterByType(filesToPush, 'variables')[0]; + if (variablesChanges) { + filesToBePushed.add(variablesChanges.file); + await this.sourceControlExportService.exportGlobalVariablesToWorkFolder(); + } + + await this.gitService.stage(filesToBePushed, filesToBeDeleted); + + await this.gitService.commit(options.commitMessage ?? 'Updated Workfolder'); + } catch (error) { + this.logger.error('Failed to export or commit changes', { error }); + try { + await this.gitService.resetBranch({ hard: true, target: 'HEAD' }); + } catch (resetError) { + this.logger.error('Failed to reset branch after export/commit error', { + error: resetError, + }); + } + throw error; + } + + const branchName = this.sourceControlPreferencesService.getBranchName(); + let pushResult: PushResult | undefined; + try { + pushResult = await this.gitService.push({ + branch: branchName, + force: options.force ?? false, }); + + // Only mark files as pushed after successful push + statusResult.forEach((result) => (result.pushed = true)); + } catch (error) { + this.logger.error('Failed to push changes', { error }); + try { + await this.gitService.resetBranch({ hard: true, target: `origin/${branchName}` }); + } catch (resetError) { + this.logger.error('Failed to reset branch after push error', { error: resetError }); + } + throw error; } - const projectsToBeExported = getNonDeletedResources(filesToPush, 'project'); - await this.sourceControlExportService.exportTeamProjectsToWorkFolder(projectsToBeExported); - - // The tags file is always re-generated and exported to make sure the workflow-tag mappings are up to date - filesToBePushed.add(getTagsPath(this.gitFolder)); - await this.sourceControlExportService.exportTagsToWorkFolder(context); - - const folderChanges = filterByType(filesToPush, 'folders')[0]; - if (folderChanges) { - filesToBePushed.add(folderChanges.file); - await this.sourceControlExportService.exportFoldersToWorkFolder(context); - } - - const variablesChanges = filterByType(filesToPush, 'variables')[0]; - if (variablesChanges) { - filesToBePushed.add(variablesChanges.file); - await this.sourceControlExportService.exportGlobalVariablesToWorkFolder(); - } - - await this.gitService.stage(filesToBePushed, filesToBeDeleted); - - // Set all results as pushed - statusResult.forEach((result) => (result.pushed = true)); - - await this.gitService.commit(options.commitMessage ?? 'Updated Workfolder'); - - const pushResult = await this.gitService.push({ - branch: this.sourceControlPreferencesService.getBranchName(), - force: options.force ?? false, - }); - // #region Tracking Information this.eventService.emit( 'source-control-user-finished-push-ui',