refactor(core): Pass the whole deleted batch to the post-delete workflow hook (no-changelog) (#35884)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

packages/cli/src/modules/source-control.ee/__tests__/source-control-import.service.ee.test.ts

@@ -2980,7 +2980,7 @@ describe('SourceControlImportService', () => {
 				expect(folderRepository.delete).not.toHaveBeenCalled();
 			});
 
-			it('should fire the afterWorkflowDeleted sweep once, after the folder row delete', async () => {
+			it('should fire the afterWorkflowsDeleted sweep once, after the folder row delete', async () => {
 				const candidates = [mock<SourceControlledFile>({ id: 'folder1' })];
 				const straggler = Object.assign(new WorkflowEntity(), { id: 'wf-1', active: false });
 				folderRepository.getAllFolderIdsInHierarchy.mockResolvedValueOnce([]);
@@ -2989,11 +2989,11 @@ describe('SourceControlImportService', () => {
 
 				await service.deleteFoldersNotInWorkfolder(candidates as any);
 
-				expect(workflowMutationHooks.afterWorkflowDeleted).toHaveBeenCalledTimes(1);
-				expect(workflowMutationHooks.afterWorkflowDeleted).toHaveBeenCalledWith('wf-1');
+				expect(workflowMutationHooks.afterWorkflowsDeleted).toHaveBeenCalledTimes(1);
+				expect(workflowMutationHooks.afterWorkflowsDeleted).toHaveBeenCalledWith(['wf-1']);
 				// Only the row delete cascades the workflows away, so the sweep must run after it
 				expect(
-					workflowMutationHooks.afterWorkflowDeleted.mock.invocationCallOrder[0],
+					workflowMutationHooks.afterWorkflowsDeleted.mock.invocationCallOrder[0],
 				).toBeGreaterThan(folderRepository.delete.mock.invocationCallOrder[0]);
 			});
 
@@ -3004,7 +3004,7 @@ describe('SourceControlImportService', () => {
 
 				await service.deleteFoldersNotInWorkfolder(candidates as any);
 
-				expect(workflowMutationHooks.afterWorkflowDeleted).not.toHaveBeenCalled();
+				expect(workflowMutationHooks.afterWorkflowsDeleted).not.toHaveBeenCalled();
 			});
 		});
 	});
@@ -3640,7 +3640,7 @@ describe('SourceControlImportService', () => {
 				expect(projectRepository.delete).not.toHaveBeenCalled();
 			});
 
-			it('should fire the afterWorkflowDeleted sweep once, after the project row delete', async () => {
+			it('should fire the afterWorkflowsDeleted sweep once for the whole batch, after the project row delete', async () => {
 				const candidates = [mock<SourceControlledFile>({ id: 'project-1' })];
 				sharedWorkflowRepository.find.mockResolvedValueOnce([
 					{ workflowId: 'wf-active' },
@@ -3657,10 +3657,13 @@ describe('SourceControlImportService', () => {
 				await service.deleteTeamProjectsNotInWorkfolder(candidates);
 
 				// The sweep searches globally for orphaned requests, so one call covers the batch
-				expect(workflowMutationHooks.afterWorkflowDeleted).toHaveBeenCalledTimes(1);
-				expect(workflowMutationHooks.afterWorkflowDeleted).toHaveBeenCalledWith('wf-active');
+				expect(workflowMutationHooks.afterWorkflowsDeleted).toHaveBeenCalledTimes(1);
+				expect(workflowMutationHooks.afterWorkflowsDeleted).toHaveBeenCalledWith([
+					'wf-active',
+					'wf-inactive',
+				]);
 				expect(
-					workflowMutationHooks.afterWorkflowDeleted.mock.invocationCallOrder[0],
+					workflowMutationHooks.afterWorkflowsDeleted.mock.invocationCallOrder[0],
 				).toBeGreaterThan(projectRepository.delete.mock.invocationCallOrder[0]);
 			});
 
@@ -3670,7 +3673,7 @@ describe('SourceControlImportService', () => {
 
 				await service.deleteTeamProjectsNotInWorkfolder(candidates);
 
-				expect(workflowMutationHooks.afterWorkflowDeleted).not.toHaveBeenCalled();
+				expect(workflowMutationHooks.afterWorkflowsDeleted).not.toHaveBeenCalled();
 			});
 		});
 	});

packages/cli/src/modules/source-control.ee/source-control-import.service.ee.ts

@@ -1858,7 +1858,7 @@ export class SourceControlImportService {
 	 * regardless — so we do the physical cleanup directly beforehand. The
 	 * `beforeWorkflowDeleted` mutation hook fires here so modules can run their
 	 * pre-delete side effects (e.g. closing open review requests), and the
-	 * caller fires `afterWorkflowDeleted` once the cascade has run (see
+	 * caller fires `afterWorkflowsDeleted` once the cascade has run (see
 	 * {@link sweepAfterWorkflowCascade}) — but external hooks
 	 * (`workflow.delete`/`workflow.afterDelete`) and the `workflow-deleted`
 	 * event still don't fire, a pre-existing gap for any cascade-deleted
@@ -1898,15 +1898,14 @@ export class SourceControlImportService {
 	}
 
 	/**
-	 * Fire `afterWorkflowDeleted` once the folder/project row delete has
+	 * Fire `afterWorkflowsDeleted` once the folder/project row delete has
 	 * cascaded the given workflows away, mirroring `WorkflowService.delete`:
 	 * the sweep behind the hook closes review requests opened after the
-	 * pre-delete hooks ran and now left without a workflow. It searches
-	 * globally for such orphans, so one call covers the whole batch.
+	 * pre-delete hooks ran and now left without a workflow.
 	 */
 	private async sweepAfterWorkflowCascade(cascadedWorkflowIds: string[]) {
 		if (cascadedWorkflowIds.length === 0) return;
-		await this.workflowMutationHooks.afterWorkflowDeleted(cascadedWorkflowIds[0]);
+		await this.workflowMutationHooks.afterWorkflowsDeleted(cascadedWorkflowIds);
 	}
 
 	/** Contextual error for a failed deletion during pull, so the operator learns which resource to look at. */

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-auto-close.service.test.ts

@@ -132,23 +132,31 @@ describe('WorkflowReviewAutoCloseService', () => {
 		expect(logger.error).toHaveBeenCalled();
 	});
 
-	describe('afterWorkflowDeleted', () => {
+	describe('afterWorkflowsDeleted', () => {
 		it('closes the requests the delete orphaned, outside any lock', async () => {
 			requestRepository.closeOrphanedOpenRequests.mockResolvedValue(['req-9']);
 
-			await service.afterWorkflowDeleted('wf-1');
+			await service.afterWorkflowsDeleted(['wf-1', 'wf-2']);
 
 			// A single atomic statement pair, so it needs no lock — and must not take one
 			// after a delete, where camping on the create lock would serialize submissions.
+			// The sweep is global, so the batch never reaches the query; it is log context only.
 			expect(requestRepository.closeOrphanedOpenRequests).toHaveBeenCalledExactlyOnceWith({});
 			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
-			expect(logger.info).toHaveBeenCalled();
+			expect(logger.info).toHaveBeenCalledExactlyOnceWith(
+				expect.any(String),
+				expect.objectContaining({
+					reason: 'workflow-deleted',
+					workflowIds: ['wf-1', 'wf-2'],
+					workflowReviewRequestIds: ['req-9'],
+				}),
+			);
 		});
 
 		it('stays quiet when the delete orphaned nothing', async () => {
 			requestRepository.closeOrphanedOpenRequests.mockResolvedValue([]);
 
-			await service.afterWorkflowDeleted('wf-1');
+			await service.afterWorkflowsDeleted(['wf-1']);
 
 			expect(logger.info).not.toHaveBeenCalled();
 		});
@@ -157,9 +165,12 @@ describe('WorkflowReviewAutoCloseService', () => {
 		it('swallows repository errors, unlike the pre-delete hook', async () => {
 			requestRepository.closeOrphanedOpenRequests.mockRejectedValue(new Error('db down'));
 
-			await expect(service.afterWorkflowDeleted('wf-1')).resolves.toBeUndefined();
+			await expect(service.afterWorkflowsDeleted(['wf-1', 'wf-2'])).resolves.toBeUndefined();
 
-			expect(logger.error).toHaveBeenCalled();
+			expect(logger.error).toHaveBeenCalledExactlyOnceWith(
+				expect.any(String),
+				expect.objectContaining({ workflowIds: ['wf-1', 'wf-2'] }),
+			);
 		});
 	});
 

packages/cli/src/modules/workflow-reviews.ee/workflow-review-auto-close.service.ts

@@ -42,11 +42,12 @@ export class WorkflowReviewAutoCloseService implements WorkflowMutationHooks {
 	}
 
 	/**
-	 * Closes reviews whose workflow is gone: the cascade took their link row, so they can
-	 * only be found by "open with nothing linked", not by workflow id. Catches reviews
-	 * opened after the pre-delete hook ran, and any left by a delete that skips the hooks.
+	 * Closes reviews whose workflow is gone: the cascade took their link rows, so they
+	 * can only be found by "open with nothing linked", not by workflow id — the sweep is
+	 * global and one call covers the whole batch. Catches reviews opened after the
+	 * pre-delete hooks ran, and any left by a delete that skips the hooks.
 	 */
-	async afterWorkflowDeleted(workflowId: string): Promise<void> {
+	async afterWorkflowsDeleted(workflowIds: string[]): Promise<void> {
 		try {
 			const closedRequestIds = await this.workflowReviewRequestRepository.closeOrphanedOpenRequests(
 				{},
@@ -56,14 +57,14 @@ export class WorkflowReviewAutoCloseService implements WorkflowMutationHooks {
 
 			this.logger.info('Closed workflow review request(s) left without a workflow', {
 				reason: 'workflow-deleted',
-				workflowId,
+				workflowIds,
 				workflowReviewRequestIds: closedRequestIds,
 			});
 		} catch (error) {
 			// The delete has already committed; failing it now would be worse than a
 			// request that stays open until the next sweep closes it.
 			this.logger.error('Failed to close workflow review request(s) left without a workflow', {
-				workflowId,
+				workflowIds,
 				error,
 			});
 		}

packages/cli/src/workflows/__tests__/workflow-mutation-hooks-proxy.service.test.ts

@@ -12,7 +12,7 @@ describe('WorkflowMutationHooksProxy', () => {
 		await expect(proxy.afterWorkflowArchived('workflow-1')).resolves.toBeUndefined();
 		await expect(proxy.afterWorkflowsTransferred(['workflow-1'])).resolves.toBeUndefined();
 		await expect(proxy.beforeWorkflowDeleted('workflow-1')).resolves.toBeUndefined();
-		await expect(proxy.afterWorkflowDeleted('workflow-1')).resolves.toBeUndefined();
+		await expect(proxy.afterWorkflowsDeleted(['workflow-1'])).resolves.toBeUndefined();
 	});
 
 	test('forwards each hook to the registered provider', async () => {
@@ -23,7 +23,7 @@ describe('WorkflowMutationHooksProxy', () => {
 		await proxy.afterWorkflowArchived('workflow-1');
 		await proxy.afterWorkflowsTransferred(['workflow-1', 'workflow-2']);
 		await proxy.beforeWorkflowDeleted('workflow-3');
-		await proxy.afterWorkflowDeleted('workflow-4');
+		await proxy.afterWorkflowsDeleted(['workflow-4', 'workflow-5']);
 
 		expect(provider.afterWorkflowArchived).toHaveBeenCalledExactlyOnceWith('workflow-1');
 		expect(provider.afterWorkflowsTransferred).toHaveBeenCalledExactlyOnceWith([
@@ -31,6 +31,9 @@ describe('WorkflowMutationHooksProxy', () => {
 			'workflow-2',
 		]);
 		expect(provider.beforeWorkflowDeleted).toHaveBeenCalledExactlyOnceWith('workflow-3');
-		expect(provider.afterWorkflowDeleted).toHaveBeenCalledExactlyOnceWith('workflow-4');
+		expect(provider.afterWorkflowsDeleted).toHaveBeenCalledExactlyOnceWith([
+			'workflow-4',
+			'workflow-5',
+		]);
 	});
 });

packages/cli/src/workflows/__tests__/workflow.service.test.ts

@@ -1847,21 +1847,21 @@ describe('WorkflowService', () => {
 		});
 
 		// It cleans up rows the cascade orphaned, which cannot be found until the row is gone.
-		test('runs the afterWorkflowDeleted lifecycle hook once the row is deleted', async () => {
+		test('runs the afterWorkflowsDeleted lifecycle hook once the row is deleted', async () => {
 			const workflow = makeWorkflowEntity({ isArchived: true, activeVersionId: null });
 			workflowFinderServiceMock.findWorkflowForUser.mockResolvedValue(workflow);
 
 			await workflowService.delete(mock<User>(), WORKFLOW_ID, true);
 
-			expect(workflowMutationHooksMock.afterWorkflowDeleted).toHaveBeenCalledExactlyOnceWith(
+			expect(workflowMutationHooksMock.afterWorkflowsDeleted).toHaveBeenCalledExactlyOnceWith([
 				WORKFLOW_ID,
-			);
+			]);
 			expect(
-				workflowMutationHooksMock.afterWorkflowDeleted.mock.invocationCallOrder[0],
+				workflowMutationHooksMock.afterWorkflowsDeleted.mock.invocationCallOrder[0],
 			).toBeGreaterThan(workflowRepositoryMock.delete.mock.invocationCallOrder[0]);
 		});
 
-		test('does not run the afterWorkflowDeleted lifecycle hook when the deletion is aborted', async () => {
+		test('does not run the afterWorkflowsDeleted lifecycle hook when the deletion is aborted', async () => {
 			const workflow = makeWorkflowEntity({ isArchived: true, activeVersionId: null });
 			workflowFinderServiceMock.findWorkflowForUser.mockResolvedValue(workflow);
 			workflowMutationHooksMock.beforeWorkflowDeleted.mockRejectedValue(new Error('db down'));
@@ -1870,7 +1870,7 @@ describe('WorkflowService', () => {
 				'db down',
 			);
 
-			expect(workflowMutationHooksMock.afterWorkflowDeleted).not.toHaveBeenCalled();
+			expect(workflowMutationHooksMock.afterWorkflowsDeleted).not.toHaveBeenCalled();
 		});
 
 		test('deletes the workflow executions before the workflow itself', async () => {

packages/cli/src/workflows/workflow-mutation-hooks-proxy.service.ts

@@ -25,11 +25,12 @@ export interface WorkflowMutationHooks {
 	beforeWorkflowDeleted(workflowId: string): Promise<void>;
 
 	/**
-	 * Called once the workflow row is gone, for cleanup that can only be done
+	 * Called once the workflow rows are gone, for cleanup that can only be done
 	 * after the delete cascades — rows orphaned by it, which by definition cannot
-	 * be found while the workflow still exists.
+	 * be found while the workflows still exist. One call covers a whole batch
+	 * (e.g. a folder cascade); the ids identify what triggered the cleanup.
 	 */
-	afterWorkflowDeleted(workflowId: string): Promise<void>;
+	afterWorkflowsDeleted(workflowIds: string[]): Promise<void>;
 }
 
 @Service()
@@ -52,7 +53,7 @@ export class WorkflowMutationHooksProxy implements WorkflowMutationHooks {
 		await this.provider?.beforeWorkflowDeleted(workflowId);
 	}
 
-	async afterWorkflowDeleted(workflowId: string): Promise<void> {
-		await this.provider?.afterWorkflowDeleted(workflowId);
+	async afterWorkflowsDeleted(workflowIds: string[]): Promise<void> {
+		await this.provider?.afterWorkflowsDeleted(workflowIds);
 	}
 }

packages/cli/src/workflows/workflow.service.ts

@@ -1198,7 +1198,7 @@ export class WorkflowService {
 
 		// After the cascade, so it can see the rows the delete orphaned. Observes a
 		// committed delete, so it must not throw — the module swallows its own errors.
-		await this.workflowMutationHooks.afterWorkflowDeleted(workflowId);
+		await this.workflowMutationHooks.afterWorkflowsDeleted([workflowId]);
 
 		this.eventService.emit('workflow-deleted', { user, workflowId, publicApi: false });
 		await this.externalHooks.run('workflow.afterDelete', [