Uh oh!
There was an error while loading. Please reload this page.
fix(service-automation): a no-catch try_catch keeps the record of the writes its try region already made - #14224
Conversation
…cards its try-region step record Measured on origin/main e621291: 11 of 16 pins RED. The run log keeps zero try-region steps and the #4354 summary reports acted: 0 over 2 writes that really landed. The 5 GREEN pins are the controls that must not move: failure text, NODE_FAILURE step code, $error contents, the honest-zero reverse control, and the contained (with-catch) path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
…ep record (#14184) Attach childSteps to try_catch's no-catch failing return, and fold result.childSteps in the engine's if (!result.success) branch — the returned-failure half of #13803's throw-path fold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
📓 Docs Drift Check2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9ad2c7e4754d36da51915b96ac4c537dd32dc1ad && git checkout 9ad2c7e4754d36da51915b96ac4c537dd32dc1ad
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 33dea61f579288c6f1b6b7e9a4789b932ddfa3e2 020552e3f34d9490d00f67976bf24d107216d675 && git checkout -B drift-repro 33dea61f579288c6f1b6b7e9a4789b932ddfa3e2 && git merge --no-ff 020552e3f34d9490d00f67976bf24d107216d675
node scripts/docs-audit/affected-docs.mjs --json 33dea61f579288c6f1b6b7e9a4789b932ddfa3e2 |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#14184
The returned-failure half of the engine's
childStepsasymmetry. #13803 (PR #14185) closed the throw half — a dyingloopbrands its thrown error with the body steps it completed and the engine'scatcharm folds them into the run log — and deliberately left theif (!result.success)branch alone, because at that moment no executor returnedchildStepson a failing result and a fold for zero producers is speculative.try_catchis the producer that makes it real. It does not throw: it catches the try region's failure and RETURNS it, so it never reached the success-only splice. On that return it withheld itschildStepson purpose — reasoning that was correct while the engine spliced them only after a successful result, and stale the moment the failing branch learned to fold.Measurement first — the card asked, and it reproduces
The card was explicit that it had not been reproduced. It was, before any fix, on the merge base
e62129153, by the test file this PR adds: 11 of 16 pins RED.catchtry_catchwhose try region writes two rows then fails reports{ selected: 3, acted: 0 }over 2 writes that really landed.steps.filter(s => s.regionKind === 'try')is[].acted: 0on a failed run reads as "nothing happened, safe to re-run", so the summary was wrong in the one direction that invites double-execution of a non-idempotent region. Same shape as #13803, one construct over.The 5 pins that were GREEN before are the controls that must not move — see "Failure propagation" below.
The change
Two halves, mirroring #13803:
builtin/try-catch-node.ts— the no-catchfailing return now carrieschildSteps: failedAttemptSteps.engine.ts— theif (!result.success)branch foldsresult.childSteps, in the same position the throw arm and the success path use: right behind the container's own step, ahead of anyfaulthandler's steps.Both halves are load-bearing. Ablated separately, each one alone reproduces the pre-change measurement exactly — 11 red / 5 green, the same split as the merge base.
Failure propagation is untouched — proved, not asserted
This is the entire basis on which the change can be judged record-only, so it is measured the way PR #14185 measured its own: these assertions were green before the change and green after it.
try_catchstill returns failure; the run still endsfailed.Node 'guard' failed: try_catch 'guard': try region failed — Node 'bang' failed: boom: at least one recipient is requiredstatus: 'failure'witherror.code: 'NODE_FAILURE'and the same message.fault-edge routing unchanged, and$errorbyte-identical:{ nodeId: 'guard', message: <the text above>, output: undefined }.catch) path, the all-succeeding path and the failing-catchpath are unchanged.Unlike the shape
partial-steps.tsexplicitly rejected forloop(swallow the throw and return a failure instead), nothing here converts a throw into a return: this return already reported failure, already produced aNODE_FAILUREstep, already set$errorand already was routable by afaultedge. Only the record changes.Reverse control
A try region that fails before writing anything still reports
acted: 0— there 0 is the honest answer. Without this pin the fix would be indistinguishable from copyingselectedintoacted;selectedis 3 andactedis 2 in the positive case, so the two also differ there.Zero-producer / no-double-fold
Once the engine folds on
if (!result.success), every failing executor passes through it. Surveyed repo-wide (childSteps, all of., excludingnode_modules/dist): all 9 carrying files live inpackages/services/service-automation/, and the only producers areloop(success return),parallel(success return) andtry_catch.loop's one failing return (themaxIterationsguard) carries nochildSteps. Sotry_catchis the only producer on the failing path, before and after.Nesting is pinned three ways — no-
catchinside no-catch, a dyingloopinside a no-catchtry region, and a no-catchinside atry_catchwith acatch. Each assertsactedreconciles exactly to the real write count (a step folded twice would push it above) and thatnew Set(steps).size === steps.length.Every folded step carries a
parentNodeIdset byrunRegion's tagger, so the ADR-0044 runaway guard, which counts only top-level visits, cannot see them.Verification
Union run on the final commit
020552e3f.pnpm --filter @objectstack/service-automation testpnpm lint(eslint . --no-inline-config, whole repo)pnpm --filter @objectstack/service-automation exec tsc --noEmitnested-region-parity.test.ts— equal to the DEBT ledger's recordederrors: 3Gate families derived live with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands(30 path-derived + convention-triggered), exit codes captured before any pipe.Two families are NOT MEASURED locally (exit 3, which is neither a pass nor a finding) because their prerequisites are CI's to provide, and both say so themselves:
check:dual-build-cjs-loads—PREREQUISITE NOT MET, reads built output, 49 packages have nodist/.check:type-check-debt—--re-measurerefuses without the whole workspace closure built.For the ratchet, the narrowing is proved rather than assumed: the only ledger entry this diff can move is
@objectstack/service-automation, and it was measured directly with its dependency closure built (pnpm --filter '@objectstack/service-automation^...' buildgreen first) at exactly 3 — the recorded number, all three in a file this PR does not touch.tsc --listFilesconfirms all three touched files are in that program, so the reading is measured and not a phantom green. No other entry can move either: the package's emitteddist/index.d.tsis byte-identical across the change (d4df83c4216e5b74e0196c73d5b5b9c8b05063bbboth with and without the fix, each built from a disk state proved by marker count), so no downstream tsc program can see this diff. The structural half of the coverage gate ran and passed.Ablation
Each half removed separately, and in both legs the mutation was proved on disk before the run (HEAD blob hash compared, marker count driven to 0) and the restore proved by state after it (
git diff HEADempty and the blob back to its HEAD hash) — never by an editor's exit code. Both legs carried anEXIT INT TERMrestore trap with absolute paths.910c8074…tob5e4b3f5…, marker 1 to 0childStepsc94cba0f…tof5b5cb8c…, marker 1 to 0Both reproduce the merge-base split exactly, and the 5 controls stay green in both legs. No rebuild is in the loop: the pins reach the subject through relative source imports (
../engine.js,./try-catch-node.js), not through the package'sexports, and the mutation ofsrc/*.tsflipping the result is itself the proof that vitest compiles the source rather thandist/.Out of scope, filed
#14222 —
try_catchwhosecatchregion itself fails still discards the whole record. Same defect class, but not a mechanical mirror: the catch region is run with nopartialStepssink, so closing it needs a new seam plus an ordering decision. Current behaviour is pinned here as an explicit boundary marker (not an endorsement), with a comment pointing at #14222.Scope
The card offered a second shape — a lint / authoring diagnostic for
try_catchwith nocatch. That is #13681's ruled B-branch, owned there; not touched. #13803 / PR #14185 not reopened. Nocontent/docs/releases/edit.Changeset:
.changeset/trycatch-returned-failure-step-record.md(@objectstack/service-automation: patch).Generated by Claude Code