Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 26 additions & 6 deletions sdk/src/authored-flow-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,16 +197,36 @@ export async function executeAuthoredFlow<Input = undefined>(
}
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}`, ':');
Expand Down
21 changes: 9 additions & 12 deletions sdk/tests/authored-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading