feat(flows): a replay that stops early says so - #866
Open
divshekhar wants to merge 4 commits into
Open
Conversation
Replay breaks on the first failing step. That is right: once a consequence did not hold, every
later step would run against a state the flow never described, and continuing turns one wrong
result into several. What was missing was any way for the caller to SEE it — a two-step flow that
halted came back as `steps: [oneResult]`, and a reader who does not already know the halt rule
sees a step that is simply not there.
Reported as "flow replay silently skips a second destructive step". All three observations in
that report were correct — step 1 never dispatched, no request fired, and the identical click
driven by hand worked — and the conclusion was wrong. The destructive-action gate was never
involved. Step 0's expectation failed and the run stopped, which is documented behaviour that the
output gave no way to read.
The cost of that invisibility was a filed defect, an investigation, and very nearly a change to
the destructive-action path that nothing needed.
`halted: { atStep, notAttempted }`, omitted entirely when every step ran so a clean pass stays
token-flat. `notAttempted` counts from the FLOW's step total, not from the results array: the
results array is the short one, so deriving the total from it would report zero skipped on every
halt — the defect restated as its own fix. A failure on the LAST step is not a halt and reports
nothing, because `notAttempted: 0` would send a reader looking for steps that do not exist.
Declared in BOTH places or it arrives as nothing: the core type and `flow-tools.ts`'s
outputSchema. That file's own comment above `unverifiable` says why, and a test now pins the
pair, because a stripped field fails silently.
Part of #697. The other half of that issue — drift naming the expectation's target instead of the
step's anchor — was already fixed by `expectElementDrift` / EXPECT_ELEMENT_NOT_FOUND.
Signed-off-by: Divyanshu Shekhar <imdshekhar@gmail.com>
divshekhar
enabled auto-merge (squash)
September 8, 2026 12:51
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #697.
What the report actually was
Filed as "flow replay silently skips a second destructive step and reports it as testid drift". All three observations were correct — step 1 never dispatched, no request fired, and the identical click driven by hand with
confirmDangerous: trueworked. The conclusion was wrong.The destructive-action gate was never involved.
replayFlowends its loop withif (result.drift !== undefined || !result.ok) break;— step 0's expectation failed and the run stopped, which is documented behaviour. Step 1 not dispatching is the correct outcome.Why it read as a silent skip
Nothing in the envelope says the run stopped. A two-step flow that halted returns
steps: [oneResult], and a reader who does not know the halt rule sees a step that is simply absent.The cost was a filed defect, an investigation, and very nearly a fix to the destructive-action path that nothing needed.
The change
halted: { atStep, notAttempted }, omitted entirely when every step ran so a clean pass stays token-flat.Two decisions worth flagging:
notAttemptedcounts from the FLOW's step total, not from the results array. The results array is the short one, so deriving the total from it would report zero skipped on every halt — the defect restated as its own fix. There is a test for exactly that.notAttempted: 0would send a reader looking for steps that do not exist.Declared in both the core type and
flow-tools.ts'soutputSchema. That file's own comment aboveunverifiableexplains why — "a field the handler sets and the schema omits arrives as nothing, silently" — and a test pins the pair.Not in this PR
The other half of #697, drift naming the expectation's target instead of the step's anchor, was already fixed by
expectElementDrift/EXPECT_ELEMENT_NOT_FOUND. I have not changed the destructive-action path, because nothing in the report shows it misbehaving.Gates
pnpm format:check,pnpm lint,pnpm typecheck,pnpm test:unit(670 files)pnpm test:e2e— 36/36 + soak (tool surface change)