From 6f6ba65ab410a22174212e2a2ca30c341a0f6fcd Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 09:30:21 +0000 Subject: [PATCH] fix: prevent duplicate completion actions when setFailed() is called after setComplete() When a NonDeterminismError occurs during replay after the orchestrator generator has already finished (and setComplete was called), setFailed() now removes the previously added completeOrchestration action before adding the FAILED action. This ensures exactly one completion action is returned to the sidecar. Previously, setFailed() would unconditionally add a second completion action, resulting in an invalid response with both COMPLETED and FAILED terminal actions. Fixes #237 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../worker/runtime-orchestration-context.ts | 11 +++++ .../test/orchestration_executor.spec.ts | 45 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/packages/durabletask-js/src/worker/runtime-orchestration-context.ts b/packages/durabletask-js/src/worker/runtime-orchestration-context.ts index 2170af7..e63d32a 100644 --- a/packages/durabletask-js/src/worker/runtime-orchestration-context.ts +++ b/packages/durabletask-js/src/worker/runtime-orchestration-context.ts @@ -224,6 +224,17 @@ export class RuntimeOrchestrationContext extends OrchestrationContext { } setFailed(e: Error) { + // If already complete, remove the previous completion action to prevent + // duplicate completion actions in the response + if (this._isComplete) { + for (const [id, action] of Object.entries(this._pendingActions)) { + if (action.hasCompleteorchestration()) { + delete this._pendingActions[Number(id)]; + break; + } + } + } + this._isComplete = true; this._completionStatus = pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED; // Note: Do NOT clear pending actions here - fire-and-forget actions like sendEvent diff --git a/packages/durabletask-js/test/orchestration_executor.spec.ts b/packages/durabletask-js/test/orchestration_executor.spec.ts index 599453f..c095c47 100644 --- a/packages/durabletask-js/test/orchestration_executor.spec.ts +++ b/packages/durabletask-js/test/orchestration_executor.spec.ts @@ -2117,6 +2117,51 @@ describe("Orchestration Executor", () => { expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_COMPLETED); expect(completeAction?.getResult()?.getValue()).toEqual(JSON.stringify("recovered")); }); + + it("should not produce duplicate completion actions when setFailed is called after setComplete", async () => { + // Scenario: An orchestrator completes immediately (returns without yielding). + // During replay, the history has a TaskScheduled event that the orchestrator + // did NOT produce (simulating a non-determinism situation). The executor + // calls setComplete when the generator finishes, then the subsequent + // TaskScheduled event validation fails with a NonDeterminismError. + // Without the _isComplete guard in setFailed(), this would produce two + // completion actions (one COMPLETED, one FAILED). + const immediateOrchestrator: TOrchestrator = async (_ctx: OrchestrationContext) => { + return "done"; + }; + + const registry = new Registry(); + const name = registry.addOrchestrator(immediateOrchestrator); + + // Old events simulate a previous execution that scheduled an activity, + // but the current code no longer does that (non-determinism). + const oldEvents = [ + newOrchestratorStartedEvent(new Date()), + newExecutionStartedEvent(name, TEST_INSTANCE_ID, undefined), + // This TaskScheduled event has no matching pending action after the + // orchestrator completes immediately, triggering a NonDeterminismError. + // Use taskId=99 to avoid colliding with the completeOrchestration action's + // sequence number (which would be 1). + newTaskScheduledEvent(99, "SomeActivity"), + ]; + + const newEvents = [newOrchestratorStartedEvent(new Date())]; + + const executor = new OrchestrationExecutor(registry, testLogger); + const result = await executor.execute(TEST_INSTANCE_ID, oldEvents, newEvents); + + // The key assertion: there should be exactly ONE completion action, not two. + // The NonDeterminismError that occurs after the generator completes should + // override the completion status to FAILED (not produce duplicates). + const completeActions = result.actions.filter((a) => a.hasCompleteorchestration()); + expect(completeActions.length).toEqual(1); + + const completeAction = completeActions[0].getCompleteorchestration()!; + expect(completeAction.getOrchestrationstatus()).toEqual( + pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED, + ); + expect(completeAction.getFailuredetails()?.getErrortype()).toEqual("NonDeterminismError"); + }); }); function getAndValidateSingleCompleteOrchestrationAction(