diff --git a/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-requests.controller.integration.test.ts b/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-requests.controller.integration.test.ts index b2b6b7e48cf..1631efbf436 100644 --- a/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-requests.controller.integration.test.ts +++ b/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-requests.controller.integration.test.ts @@ -25,6 +25,8 @@ import { v4 as uuid } from 'uuid'; import { ActiveWorkflowManager } from '@/active-workflow-manager'; import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service'; import { WorkflowValidationService } from '@/workflows/workflow-validation.service'; +import { WorkflowService } from '@/workflows/workflow.service'; +import { EnterpriseWorkflowService } from '@/workflows/workflow.service.ee'; import { createAdmin, createMember, createOwner, createUser } from '@test-integration/db/users'; import { createWorkflowHistoryItem } from '@test-integration/db/workflow-history'; import type { SuperAgentTest } from '@test-integration/types'; @@ -932,6 +934,147 @@ describe('publishing a workflow under review', () => { }); }); +/** + * The approval commits under the review lock, but auto-publish runs after it is + * released. Workflow mutations are deliberately not serialized behind that lock, + * so they can land in the gap. These pin the accepted outcomes: the approval + * stands and the publish that lost the race is reported as a failure. + */ +describe('a workflow mutation racing the auto-publish of an approval', () => { + /** Run `raceAction` in the gap between the committed approval and auto-publish. */ + function raceBeforeAutoPublish(raceAction: () => Promise) { + const workflowService = Container.get(WorkflowService); + const activate = workflowService.activateWorkflow.bind(workflowService); + vi.spyOn(workflowService, 'activateWorkflow').mockImplementationOnce(async (...args) => { + await raceAction(); + return await activate(...args); + }); + } + + test('an archive that lands in the gap leaves an approved review and a failed auto-publish', async () => { + const { workflow, versionId } = await createReviewableWorkflow(); + const request = await createOpenReview(workflow.id, versionId); + + raceBeforeAutoPublish( + async () => await ownerAgent.post(`/workflows/${workflow.id}/archive`).expect(200), + ); + + const response = await ownerAgent + .post(`/workflow-review-requests/${request.id}/decision`) + .send({ decision: 'approved' }) + .expect(200); + + expect(response.body.data).toMatchObject({ + state: 'closed', + decision: 'approved', + autoPublish: { status: 'failed', message: 'Cannot activate an archived workflow.' }, + }); + + // The archive found the review already closed, so it left the approval alone. + const closed = await requestRepository.findById(request.id, {}); + expect(closed).toMatchObject({ state: 'closed', decision: 'approved' }); + + const archived = await workflowEntityRepository.findOneByOrFail({ id: workflow.id }); + expect(archived.isArchived).toBe(true); + expect(archived.activeVersionId).toBeNull(); + + // Publishing stays blocked by the archival itself, not by the closed review. + await ownerAgent.post(`/workflows/${workflow.id}/activate`).send({ versionId }).expect(400); + }); + + test('a transfer that lands in the gap leaves an approved review and a failed auto-publish', async () => { + const versionId = uuid(); + const workflow = await createWorkflow({}, teamProject); + await createWorkflowHistoryItem(workflow.id, { versionId }); + + // The requester publishes on approval, so it must be someone who loses access + // when the workflow moves — the deciding owner never does. + const request = await requestRepository.createRequest( + { projectId: teamProject.id, title: 'Review before publishing', createdById: member.id }, + {}, + ); + await workflowRepository.createWorkflowRow( + { + workflowReviewRequestId: request.id, + workflowId: workflow.id, + workflowVersionId: versionId, + }, + {}, + ); + await authorRepository.addAuthor( + { workflowReviewRequestId: request.id, userId: member.id }, + {}, + ); + + const destination = await createTeamProject('Elsewhere', await createMember()); + raceBeforeAutoPublish( + async () => + await Container.get(EnterpriseWorkflowService).transferWorkflow( + owner, + workflow.id, + destination.id, + ), + ); + + const response = await ownerAgent + .post(`/workflow-review-requests/${request.id}/decision`) + .send({ decision: 'approved' }) + .expect(200); + + expect(response.body.data).toMatchObject({ + state: 'closed', + decision: 'approved', + autoPublish: { + status: 'failed', + message: + 'You do not have permission to activate this workflow. Ask the owner to share it with you.', + }, + }); + + expect(await requestRepository.findById(request.id, {})).toMatchObject({ + state: 'closed', + decision: 'approved', + }); + expect( + (await workflowEntityRepository.findOneByOrFail({ id: workflow.id })).activeVersionId, + ).toBeNull(); + }); + + test('a review created in the gap blocks the auto-publish without failing the decision', async () => { + const { workflow, versionId } = await createReviewableWorkflow(); + const request = await createOpenReview(workflow.id, versionId); + + let racingRequestId = ''; + raceBeforeAutoPublish(async () => { + racingRequestId = (await createOpenReview(workflow.id, versionId)).id; + }); + + const response = await ownerAgent + .post(`/workflow-review-requests/${request.id}/decision`) + .send({ decision: 'approved' }) + .expect(200); + + expect(response.body.data).toMatchObject({ + state: 'closed', + decision: 'approved', + autoPublish: { + status: 'failed', + message: + "Workflow can't be published while its review is open. Submit this version to the review, or wait for the review to close.", + }, + }); + + // The new review is untouched and still guards the workflow. + expect(await requestRepository.findOpenRequestForWorkflow(workflow.id, {})).toMatchObject({ + id: racingRequestId, + state: 'open', + }); + expect( + (await workflowEntityRepository.findOneByOrFail({ id: workflow.id })).activeVersionId, + ).toBeNull(); + }); +}); + describe('POST /workflow-review-requests/:workflowReviewRequestId/update-version', () => { /** Seed an open review request pinned to `versionId`, authored by `author`. */ async function seedOpenRequest( diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index 4afc80a0dbc..ba637ab08e7 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -886,6 +886,13 @@ export class WorkflowService { // re-applying the already-published version (e.g. a settings-only update) // publishes no new version, so the review gate must not block it. + // + // This check is deliberately not serialized with review mutations: putting + // publishing behind the review feature's global lock would slow down a core + // workflow operation for every instance. A review opened just after this + // passes, or just before an approval's auto-publish reaches it, therefore + // races — accepted, because both outcomes degrade gracefully (the approval + // stands and auto-publish reports `failed`).`. if (versionIdToActivate !== previousActiveVersionId) { await this.workflowPublishGuard.assertCanPublish(workflowId); }