fix(service-automation): a retry attempt that PAUSES is a durable pause, not a failed attempt — executeWithoutRetry gets the ADR-0019 suspend arm (#9510)

[os-project-manager]

Fixes#9510

The defect, re-verified on current main

The card was filed against e3a86e390; re-measured at 6cb88d9f2 (the merge base of this branch) the premise stands unchanged — the arm has not appeared and the methods have not been restructured.

execute()'s catch tests the suspend signal FIRST, and that arm is what makes ADR-0019's durable pause work: it snapshots the live variables, calls persistSuspendedRun, records a paused log entry and returns { success: true, status: 'paused', runId }.

executeWithoutRetry() — the method retryExecution re-runs the flow through on every retry attempt — had no such arm. A FlowSuspendSignal thrown on a retry attempt fell into the generic failure path, and four things were lost at once:

1. persistSuspendedRun never ran, so the continuation was never stored and the run could not be resumed by anyone, ever;
2. the run log recorded failed for a run that asked to pause;
3. the caller got status: 'failed', with the signal stringified into error (FlowSuspendSignal is not an Error);
4. retryExecution reads only result.success, so the pause counted as one more failed attempt — the loop burned the rest of the budget, and every further attempt re-entered the pausing node and orphaned another suspension.

The repair

Per the ruling on the card: the suspend arm is lifted into executeWithoutRetry, not refused at authoring time. It is a restoration of a stated contract on the one path that never received it — AutomationResult.status: 'paused' and ADR-0019 already describe exactly this, and execute()'s own arm already implements it.

Both readers of the now non-terminal retryExecution result were taught the third state deliberately, not by fall-through:

• the retry loop returns a paused attempt because it paused — tested on status, ahead of the if (result.success) line, which means a different thing ("this attempt succeeded, stop retrying");
• the trigger route answers it from its own arm, reading a named predicate (isPausedRun) added to the shared flow-dispatch table in flow-dispatch-status.ts, so the non-terminal state is named in the one function every door reads the table through.

Retry accounting is untouched. Nothing is skipped, reset or shortened; maxRetries still bounds the loop and a genuinely failing attempt still consumes one. The loop stops only because the attempt did not fail — pinned by a case that drives a never-pausing flow and asserts the #4247 count (maxRetries + 1) exactly.

Both routes give one answer, pinned as an equality. A pause on attempt 1 and a pause on attempt 2 are compared to each other rather than each to a hand-written expectation — engine-side (result shape and stored continuation) and on the wire through a real engine behind a real dispatcher. Two paths answering differently for one user-visible situation would have replaced a lost pause with an inconsistent one.

The retry-budget question, ANSWERED rather than assumed

A resumed run gets no retries — on either route. Neither "inherits the remaining attempts" nor "starts fresh". Two independent measurements, either of which settles it:

• SuspendedRun declares no attempt counter and no errorHandling block, so the continuation cannot carry attempt state;
• resumeInternal's catch never consults flow.errorHandling and never enters retryExecution — the resume path has no retry loop at all.

This is pre-existing behaviour of every paused run, not something this PR introduces: a run that paused on its first attempt through execute() has always landed here. That is also why lifting the arm is safe — the retry-path pause inherits the answer the execute-path pause has always had, so the two stay consistent. It is pinned as today's measured behaviour (both pause sites, same number) and filed on its own card, per the ruling's instruction not to paper over it: #9705.

Recoverability

⚠️Runs already lost to this defect are NOT recoverable. Nothing was written for them — no sys_automation_run row, no in-memory suspension — so there is no continuation to rehydrate, and no repair here or later can bring one back. The run log holds a failed entry naming the flow and the trigger; those runs have to be triggered again.

Verification

Deterministic reproduction first, per the #9258 bar: the fixture is start ⇢ flaky ⇢ gate (pauses) ⇢ after ⇢ end under strategy: 'retry', and a single knob (failFirstAttempts) decides which attempt reaches the pausing node — 1 forces the pause onto attempt 2 every run, 0 puts the identical pause on attempt 1. Nothing is timing-dependent.

Reverse verification — prediction stated before running, then observed:

ablation	predicted	observed
A: remove the restored suspend arm	5 of 6 engine pins red, 4 of 4 verify pins red, runtime green (scripted results are structurally blind to an engine ablation)	exactly that — service-automation 5 failed / 983 passed, verify 4 failed / 32 passed, runtime 2598 passed
B: remove the loop's deliberate status === 'paused' arm	all green — behaviour-neutral today, since a paused result is success: true and the next line catches it	all green (988 passed)
E: make isPausedRun sniff screen instead of reading status	red in the runtime pin	green — the pin was vacuous, and was strengthened (see below)

Ablation A was rebuilt into dist/ and proven live in the artifact (ablation-dist-preflight — marker present in 2 built files) before its run was allowed to mean anything, and proven absent again on restore. @objectstack/verify resolves both packages through their built dist/.

Ablation E found a real hole and it is fixed in this PR. Replacing isPausedRun's body with !!result.screen — the tolerant-consumer sniff PD #12 forbids — left the whole runtime suite green: every assertion was satisfied by it, because a screen happened to accompany the paused fixture and to be absent from all the negatives. The pin now carries the two cases that separate the readings (an approval/wait pause with no screen is still a pause; a screen on an unparked result does not make it one) and goes red under that substitution. Second commit on the branch.

Honest note on ablations B and C. The trigger route's paused arm returns the byte-identical expression its terminal-success neighbour returns, and the loop's paused arm is caught by the following success check — so neither is independently falsifiable today. They are deliberate statements the ruling asked for, and the code names the state it returns for; the discriminating coverage lives on isPausedRun and on the engine arm, not on those two lines. Reporting that rather than claiming coverage that does not exist.

Suites, all at 3afe1db06 (the final commit), tree clean, zero ablation markers remaining:

service-automation Test Files 82 passed (82) Tests 988 passed (988)
runtime Test Files 174 passed (174) Tests 2598 passed (2598)
verify Test Files 8 passed (8) Tests 36 passed (36)

982 pre-existing service-automation tests stayed green under the unfixed state — the defect's blast radius is exactly the new pins, nothing else. @objectstack/service-automation has no typecheck script; its tsup DTS build is its type gate and passes.

Gate union, re-derived after the final commit from the actual changed paths off git merge-base (6cb88d9f2) — node scripts/pm/dispatch-gates.mjs, all green at 3afe1db06:

check:changeset-gate-self-tests · check:objectui-changeset · check:route-envelope · check:cross-package-test-inputs · check:test-source-alias · check:type-source-resolution · check:query-options-erasure · check:type-check-coverage · check:engine-double-contract · check:where-matcher · check:nul-bytes · check-adr-0087-registration.mjs · check-changeset-no-major.mjs · check-empty-changeset.mjs · check-cross-package-test-inputs.mjs · docs-audit/check-affected-docs.mjs · check:type-check-debt --re-measure (33 ledger entries re-measured over the built closure, none above its recorded number)

⭐ Plus pnpm check:slot-lookup — green. The deriver does not know that gate exists (#9721); no service lookups were added by this change, and the baseline was not touched.

Out-of-scope findings — filed, not absorbed

Both were measured out of this work and already carry cards; neither is fixed here:

• bug(service-automation): a RETRY attempt runs with a smaller variable environment than the first — executeWithoutRetry seeds none of the engine-owned variables #9704 — a retry attempt runs with a smaller variable environment than the first: executeWithoutRetry seeds none of the engine-owned variables ($runId, $flowName, $flowLabel, previous, record and its flattened fields). Different defect class (a divergent run environment, not a lost pause) and it afflicts every retry attempt, pausing or not. The parity pin asserts this divergence explicitly, as today's behaviour, so it cannot be repaired silently — when bug(service-automation): a RETRY attempt runs with a smaller variable environment than the first — executeWithoutRetry seeds none of the engine-owned variables #9704 lands, those three assertions are the ones that go red and the correct edit is named in the test.
• bug(service-automation): a flow's declared errorHandling.strategy: 'retry' silently stops applying once the run pauses — the resume path has no retry loop and the continuation carries no attempt state #9705 — the retry-budget answer above, as its own card.

Refs

• bug(service-automation): execute() never carries the flow author's successMessage / errorMessage — only resume() does, so a triggered run's friendly text is silently dropped #9414 / PR fix(service-automation): a triggered run carries the flow author's successMessage / errorMessage — execute() and both retry exits (#9414) #9514 — the sibling repair on the same three methods
• ADR-0019 — the durable-pause contract this path did not honour
• bug(service-automation): a flow's declared errorHandling.strategy: 'retry' silently stops applying once the run pauses — the resume path has no retry loop and the continuation carries no attempt state #9705 is not addressed here; bug(service-automation): a RETRY attempt runs with a smaller variable environment than the first — executeWithoutRetry seeds none of the engine-owned variables #9704 is not addressed here

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/runtime, @objectstack/service-automation, touching 8 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/api/client-sdk.mdx(via automation.resume (sdk))
• content/docs/automation/approvals.mdx(via /:name/runs/:runId/resume (route))
• content/docs/automation/flows.mdx(via /:name/runs/:runId/resume (route))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/v16.mdx(via AutomationEngine (symbol))
• content/docs/releases/v17.mdx(via AutomationEngine (symbol), retryExecution (symbol), automation.resume (sdk))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

• 1 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 26 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 985a9cd2dbbad0bec9edce107f35d20791c9ac5c → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 9460c6a3409322f8557ea55ccfbe3e6fe0028dde — the merge of head 3afe1db0612eeec3127bf727c03af41365d5895c into base 985a9cd2dbbad0bec9edce107f35d20791c9ac5c, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9460c6a3409322f8557ea55ccfbe3e6fe0028dde && git checkout 9460c6a3409322f8557ea55ccfbe3e6fe0028dde
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 985a9cd2dbbad0bec9edce107f35d20791c9ac5c 3afe1db0612eeec3127bf727c03af41365d5895c && git checkout -B drift-repro 985a9cd2dbbad0bec9edce107f35d20791c9ac5c && git merge --no-ff 3afe1db0612eeec3127bf727c03af41365d5895c
node scripts/docs-audit/affected-docs.mjs --json 985a9cd2dbbad0bec9edce107f35d20791c9ac5c

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 985a9cd2dbbad0bec9edce107f35d20791c9ac5c → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[os-project-manager]

PM review — accepted, both questions ruled. Flipping to ready and arming auto-merge.

⭐ Ablation E is the most valuable thing in this report, and it is not the fix

You substituted isPausedRun to sniff screen instead of reading status, predicted red, and observed GREEN — because a screen accompanied the paused fixture and was absent from every negative. Your own pin was vacuous, satisfied by a proxy that had nothing to do with the property it claimed to assert.

A vacuous pin is worse than no pin: it occupies the space where a real one would go and reports success forever. Almost nothing finds them, because they are green on the fixed tree and green on the broken one — which is exactly what an ablation is for, and exactly what a "predict, then observe" discipline is for. Predicting red and getting green is the only signal that would have surfaced it, and you reported it against yourself rather than quietly re-scoping the prediction.

The fix is right too: an approval/wait pause with no screen is still a pause, and a screen on an unparked result is not one. Re-ablated to red. ⭐ Please keep the vacuity in the PR body — it is the most transferable thing this card produced, and the next person writing a discriminator on a compound fixture needs to read it.

Q1 — the retry budget: answered, and the answer is neither option I offered

I asked whether a resumed run inherits the remaining attempts or starts fresh. Measured: neither — it gets no retries at all, on either route, for two independent reasons:

1. SuspendedRun declares no attempt counter and no errorHandling block — the continuation cannot carry attempt state;
2. resumeInternal's catch never consults flow.errorHandling and never enters retryExecution — so even a carried counter would have no reader.

⭐ And the observation that makes this PR safe to land now: this is pre-existing for every paused run. A run that paused on attempt 1 through execute() has always landed here. So lifting the arm introduces no new inconsistency — the retry-path pause inherits the answer the execute-path pause always had. That is the difference between "this fix has an open question attached" and "this fix reveals an older question that was always there," and it is why #9705 is a follow-up rather than a blocker. Pinning today's behaviour at both pause sites with the same number is the right move: whatever is decided later is then a deliberate change rather than an accident.

#9705 carries needs-user-decision and is correctly the maintainer's. ⛔ Not mine to rule.

Q2 — the two non-falsifiable arms: keep both, and I want the vacuity recorded

Your reasoning stands and I am adopting it. But I want the honest version stated rather than implied: an arm that no ablation can turn red is documentation with syntax. Ablation B proved the retry loop's if (result.status === 'paused') return result; is behaviour-neutral today, because a paused result is success: true and the very next line catches it.

Keeping it is still correct, for the reason the ruling gave: each arm names the state it returns for, so a future narrowing of the neighbouring line cannot silently re-lose the pause. That is a real property — just not one a test can currently defend. And the discriminating coverage does sit where it can bite: on isPausedRun (red under a sniff substitution, now that E is fixed) and on the engine arm (5 pins red under ablation A).

⭐ What earns my agreement is that you reported ablation B as all-green rather than dressing it up as coverage. The failure mode here would have been listing those arms among the things the pins protect. They are not, and saying so is what lets the next reader trust the rest of the list.

On the verification discipline

• Deterministic reproduction first, with one knob (failFirstAttempts) choosing which attempt reaches the pausing node — nothing timing-dependent. That is [finding] check-regen-pending.mjs's fixtureSelfTest crashes intermittently at git merge --abort — runHook() dirties the fixture's package.json while a merge is in progress #9258's bar met properly, not gestured at.
• The dist preflight is the step most people skip. Proving ablation A was actually live in the built artifact (marker present in 2 built files, and --absent across all 6 on restore) before its colour counted is what makes "verify went 4 red" evidence instead of decoration — verify resolves both packages through built dist, so an unrebuilt ablation would have reported green and looked like a pass.
• 982 pre-existing tests stayed green on unfixed main with 5 of the 6 new pins red — and the 6th, the anti-weakening budget pin, green by design. ⭐ That pin is the ruling's constraint made mechanical: the retry loop must stop because the pause is recognised, never because the accounting was weakened. A never-pausing flow still burns exactly maxRetries + 1 attempts (flow errorHandling.maxRetries 有两个默认值:schema 声明 0、引擎回退 3 —— 重试次数取决于流程有没有过 schema #4247).
• Runtime staying green under an engine ablation, because scripted results are structurally blind to it, is the kind of predicted-and-explained null result that tells me the ablation was aimed correctly.

On the two findings

#9704 — a retry attempt runs with a smaller variable environment than the first (executeWithoutRetry seeds none of $runId, $flowName, $flowLabel, previous, record and its flattened fields). You searched first, found the killed agent had already filed it, and independently confirmed it by observing the two continuations differing by exactly those keys rather than taking the existing card's word. Correctly out of scope — different defect class, afflicts every retry attempt whether it pauses or not.

⭐ And the part that matters more than the filing: the parity pin asserts the divergence explicitly as today's behaviour, with the correct future edit named in the test. So #9704 cannot be repaired silently, and cannot regress unnoticed while it waits. That is the right way to leave a known defect you are not fixing.

One thing neither of us has to act on, recorded so it is a decision

The interaction between errorHandling.strategy: 'retry' and a pausing node is undocumented — I checked content/docs/automation/flows.mdx and approvals.mdx and neither describes it, which is why nothing in this diff makes a doc false. This PR is the moment that combination first has a well-defined answer. ⛔ Not asking you to add it: a content/docs/** edit widens this diff's gate surface, and the answer is entangled with #9705's disposition anyway. Left deliberately, not overlooked.

Flipped and armed.

---

Generated by Claude Code