diff --git a/sdk/src/authored-flow-executor.ts b/sdk/src/authored-flow-executor.ts index b8582d5f7..bdc8b11df 100644 --- a/sdk/src/authored-flow-executor.ts +++ b/sdk/src/authored-flow-executor.ts @@ -197,16 +197,36 @@ export async function executeAuthoredFlow( } throw bodyFailure; } - try { - await verifyAuthoredOperations(definition.name, authoredSteps, lifecycle); - } finally { - lifecycle.close(); - } + // The completion requirement is checked BEFORE operation verification, + // because without a completion the verification cannot answer its own + // question. `AuthoredFlowLifecycle.isHandled` decides whether an operation + // was consumed by asking whether the COMPLETION depends on it; with no + // completion there is no async id to trace from, so it returns false for + // every operation. Verifying first therefore reported correctly-awaited + // steps as `unawaited_step`, naming the step the author had awaited and + // saying nothing about the `done()` they forgot (#183). + // + // A body that both forgets `done()` and leaves a step unawaited now reports + // the missing completion. That is the honest order: the unawaited-step + // verdict is not computable until there is a completion to compute it + // against, and once the author adds `done()` the verification runs normally + // and will catch it. if (requestedCompletion === undefined) { - throw new AuthoredFlowExecutionError( + const missingCompletion = new AuthoredFlowExecutionError( 'missing_completion', `flow "${definition.name}" returned without done()`, ); + try { + await stopAuthoredOperations(authoredSteps, missingCompletion); + } finally { + lifecycle.close(); + } + throw missingCompletion; + } + try { + await verifyAuthoredOperations(definition.name, authoredSteps, lifecycle); + } finally { + lifecycle.close(); } await lowerDeterministic(`complete-${nextStep}`, ':'); diff --git a/sdk/tests/authored-flow.test.ts b/sdk/tests/authored-flow.test.ts index b2bcc9edd..985ee93a6 100644 --- a/sdk/tests/authored-flow.test.ts +++ b/sdk/tests/authored-flow.test.ts @@ -465,20 +465,17 @@ describe('authored flow journal executor', () => { }); const client = await connectedClient('authored-flow-missing-completion-test'); try { - // The PROPERTY is what this pins: a body that runs a journal-backed step - // and never calls done() must be refused, not silently completed. + // Back to the code the original test asserted before #140 dropped it. // - // The original version of this test (dropped in #140, restored here) - // asserted `missing_completion`. On current main the refusal arrives as - // `unawaited_step` instead, because `verifyAuthoredOperations` runs - // before the `requestedCompletion === undefined` check in - // `authored-flow-executor.ts` and throws first. The body here DOES await - // its step, so "returned with unawaited steps" is a misleading label for - // it -- see #183. Asserting either code specifically would either fail on - // main or bake in a message that looks wrong, so this asserts the - // refusal and its class instead. + // Between the restoration and now this had to assert only the refusal's + // class, because the executor answered `unawaited_step` -- it verified + // operations before checking for a completion, and `isHandled` returns + // false for every operation when there is no completion to trace from. + // It named the step the author HAD awaited and said nothing about the + // `done()` they forgot (#183). Fixed by checking the completion first, + // so this can pin the code again. await expect(executeAuthoredFlow(handle, client)).rejects.toMatchObject({ - name: 'AuthoredFlowExecutionError', + code: 'missing_completion', }); } finally { client.close();