fix(service-automation): a try_catch whose catch region itself fails keeps both regions' step record

[claude]

Fixes #14222

A try_catch whose catch region itself fails now returns the step record of both regions instead of discarding everything.

try_catch returns failure from three sites. #13803 taught the engine to fold a dying container's carried steps off the THROW channel; #14184 taught the returned-failure branch (if (!result.success)) the same and taught the first producer — a try_catch with no catch region — to supply them. The second producer was left unfolded. When a catch region is present and the handler itself fails, the return dropped childSteps entirely, so the run log kept a step for neither region and the #4354 summary reported acted: 0 over writes from two regions that had genuinely landed. acted: 0 on a failed run reads as "nothing happened, safe to re-run".

The half that was genuinely missing was the catch region's own steps: the failed try attempts were already in scope, but the catch region ran without a partialSteps sink, so when the handler threw its completed steps unwound with the stack. The catch region now receives the same sink the try region already had (runRegion's fifth argument), and the failing return carries [...failedTryAttempts, ...catchAttempts] — failed try attempts first, the ordering the successful-catch return has always used.

Ruling of record

issuecomment-5503054554 (triage, 2026-09-02 01:40:34Z). Comments read to the last page: 2 — the triage ruling and the dispatching seat's claim comment. The ruling settles the design the card asked for, as a restore-invariant: "the run log must carry every step that ran, whichever region ran it." This PR implements exactly that shape — the sink as runRegion's fifth argument, [...failedAttemptSteps, ...catchAttemptSteps] with failed try attempts first, regionKind supplied by runRegion's existing tagger, and no engine.ts change. The standing fence on engine.ts was not approached: the #14184 fold that reads these steps is already in place at engine.ts:7141.

The pre-existing pin "a failing catch region still fails with the catch error and carries no try steps" is inverted in place with its comment rewritten, not deleted — what it used to assert is written out in the new comment, per the ruling.

Ablation — two legs, both red, restore proven

Subject resolution stated up front: the pin suite lives in packages/services/service-automation/src/ and imports its subject relatively (../engine.js, ./try-catch-node.js), so vitest resolves it to source, never dist/. The package's only vitest alias is for @objectstack/platform-objects, unrelated. No rebuild of service-automation is in the ablation's path, and the mutation is live in the next run; it is proven on disk by blob hash and anchor-occurrence counts, never by a comment marker.

All four runs below are at 20aa3b0d9, with HEAD blob of try-catch-node.ts = 66eac7ad9.

leg	mutation	on-disk proof	result
baseline	none	blob 66eac7ad9 == HEAD	Test Files 1 passed (1) / Tests 17 passed (17)
A	drop childSteps from the failing-catch return	anchor 1 → 0; blob 43360a513 != HEAD	Test Files 1 failed (1) / Tests 2 failed | 15 passed (17)
B	drop only the catch-region sink argument (keep childSteps)	sink-arg 1 → 0; blob a2b6b440b != HEAD	Test Files 1 failed (1) / Tests 2 failed | 15 passed (17)
restore	git checkout HEAD -- ABSOLUTE_PATH	blob back to 66eac7ad9 == HEAD; git diff HEAD empty	Test Files 1 passed (1) / Tests 17 passed (17)

The mutation script carries trap RESTORE_FN EXIT INT TERM with absolute paths derived from git rev-parse --show-toplevel; restore is proven by blob equality against the HEAD blob (an empty hash is treated as failure), not by the trap having fired and not by an exit code.

The two legs discriminate the two halves, which is the point. Leg A removes the whole record: acted=0, grouped steps []. Leg B keeps childSteps but removes the sink — this is precisely the one-liner #14184 could have written — and it lands at acted=2 over 3 writes that happened, with catch:cw1:success / catch:handler:failure missing from the log:

A: AssertionError: expected [] to deeply equal [ 'try:w1:success', …(3) ]
A: reported acted=0 is LOWER than the 3 writes that actually happened

B: AssertionError: expected [ 'try:w1:success', …(2) ] to deeply equal [ 'try:w1:success', …(3) ]
   - "catch:handler:failure"
B: reported acted=2 is LOWER than the 3 writes that actually happened

Leg B is the one that matters for scope: it proves the sink — the genuinely new seam, the part the card said was not a mechanical mirror of #14184 — is load-bearing and not decoration.

In both legs exactly 2 tests fail and 15 pass. The 15 include every control that must not move: "still fails the run, with the same error text and step code", "still routes down the fault edge, with the same $error contents", the contained-path controls, the honest-zero reverse controls, and the three nesting/double-count pins. That is the measured evidence that this change is additive to the record only.

Adversarial reading of the 48-line diff

Asked directly whether the diff does only what the card asks. It does. Specifically:

• Success path is untouched. runRegion pushes into partialSteps only inside its own catch arm (engine.ts: tag(); partialSteps?.push(...regionSteps); then rethrow). On success the sink is never written and regionSteps is returned as before. The successful-catch return still uses the returned catchSteps, so passing the fifth argument changes nothing a caller observes when the handler succeeds — and no step can reach the log twice by this route. The suite pins that independently (expect(new Set(steps).size).toBe(steps.length)).
• Error propagation is unchanged. runRegion still rethrows, catchErr is still caught at the same place, and the error string is byte-identical (try_catch '${node.id}': catch region failed — ${catchMsg}). success: false is unchanged.
• Ordering is [...failedAttemptSteps, ...catchAttemptSteps] — the ruling's rule, and the same one the successful-catch return above it already used.
• Nothing is tagged at this site. regionKind: 'try' / 'catch' and parentNodeId come from runRegion's existing tagger, which runs on its failure path as well as its success path.
• Most of the 48 lines are comment and re-indentation. Reformatting the runRegion call from 3 lines to 8 to take a fifth argument, plus a ~22-line comment block, accounts for the bulk. The behavioural delta is two things: one new const catchAttemptSteps: StepLogEntry[] = [], passed as the fifth argument, and one new childSteps: key on the failing return.
• No new exported symbol (git diff | grep '^+.*export' → none), and engine.ts is not in the diff at all.

I found nothing that reaches beyond the card. The one behavioural consequence worth naming explicitly is intended and is the fix itself: the run summary's acted for this path changes from an under-count to the true count (0→3 in the new pin). That is a corrected value in an existing field, not a new field.

Clause ② — no

No consumer-visible payload key is added and no accept/reject behaviour moves. childSteps?: StepLogEntry[] is a pre-existing optional field on NodeExecutionResult (engine.ts:349), already produced by both success returns in this same executor and by the no-catch failing return since #14184; this return starts populating it, it does not introduce it. The element type StepLogEntry gains no field — engine.ts is untouched, which is the mechanical check the dispatch asked for. Accept/reject is unmoved: same success: false, byte-identical error text, same NODE_FAILURE step, same $error write, same fault-edge routing — all of which live in engine.ts and are pinned green through both ablation legs.

Changeset

.changeset/try-catch-failing-catch-step-record.md → "@objectstack/service-automation": patch. Confirmed rather than assumed: no exported symbol is added, no type is widened, no package.json/exports moves, and the only change is that an existing optional field is now populated on a path where it was absent. The direct precedent agrees — #14184, the same defect one path over in the same file, landed as patch (.changeset/trycatch-returned-failure-step-record.md on main at 7d3b1b79c).

Gates

Re-derived on the final diff with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack CHANGED_PATHS → 35 commands. Exit codes captured by redirecting first (cmd > log 2>&1; EXIT=$?), never across a pipe. 32 green, 3 NOT MEASURED — each of the three exits 3, and each prints its own verdict text declaring the prerequisite unmet:

• node scripts/check-test-completeness.mjs — "PREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named… ⛔ It is not a red." CI tees the log and passes the path.
• pnpm check:dual-build-cjs-loads — "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/… ⛔ This is NOT a pass: nothing was measured." Needs a full pnpm build. My diff touches no package.json, no exports, and no build config, so the dual-build surface is untouched by construction.
• pnpm check:type-check-debt — "PREREQUISITE NOT MET… ⛔ This is NOT a pass and NOT a finding." Needs the built dependency closure. Its self-test passed (48+68+43+28+19+18 cases) and the non-re-measure pnpm check:type-check-coverage ran green.

Also run and green outside the derived set: pnpm check:nul-bytes (self-test 75 assertions; 8071 files scanned, clean), plus a direct control-byte scan of the three changed files (grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]' → no match).

Package verification, all at 20aa3b0d9:

• pnpm --filter '@objectstack/service-automation^...' build — dependency closure built first.
• pnpm --filter @objectstack/service-automation build — green; DTS emit succeeds (2/2 declared declaration file(s) present), which type-checks the source.
• pnpm --filter @objectstack/service-automation test — Test Files 101 passed (101) / Tests 1203 passed (1203).

ESLint — a declared narrowing, not a skip. Ran eslint --no-inline-config on the changed paths rather than the whole repo, with all three pieces of evidence:

1. Population from eslint's own config, not my guess — eslint itself reports the changeset as File ignored because no matching configuration was supplied, i.e. it places 2 of the 3 changed files in the lint population.
2. Count from --format json — 3 entries reported, 2 linted, 0 errors, 0 real warnings.
3. Invariance for untouched files — eslint.config.mjs:326-329 states this repo "runs one eslint.config.mjs, which never enables type-aware linting (no parserOptions.project, no typed @typescript-eslint rules) for ANY file, test or not", corroborated by every parserOptions in the config carrying only ecmaVersion/sourceType. With no cross-file type program there is no mechanism by which an edit under service-automation/src/builtin/ can move any untouched file's verdict. pnpm lint is eslint . --no-inline-config — the same binary, flag and config I invoked.

The repo-wide pnpm lint run itself is CI's, and CI runs the farm exactly once regardless.

Not encountered

The known repo-wide flake at packages/cli/test/run-dev-unbuilt-workspace.e2e.test.ts:317 did not appear — nothing in this card's verification touches packages/cli.

Provenance

This branch was pushed by an earlier agent that died before opening a PR or reporting. Its commit 860269286 was verified rather than trusted: the diff was re-read line by line, the ablation above was authored and run fresh, and every number here was measured in this session. origin/main was merged twice during verification; the final tree is 20aa3b0d9.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

1 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

• the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 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 5a5336b399db2ef18dd4700f97d579a328197dda → packageMentionDocs.

Which tree this was computed on

This run read content/docs from d47c1bb46f8b7c97c8cd341481fa762e8ffeaf7e — the merge of head 20aa3b0d963eb00258967370ad7b493295da9d8c into base 5a5336b399db2ef18dd4700f97d579a328197dda, 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 d47c1bb46f8b7c97c8cd341481fa762e8ffeaf7e && git checkout d47c1bb46f8b7c97c8cd341481fa762e8ffeaf7e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5a5336b399db2ef18dd4700f97d579a328197dda 20aa3b0d963eb00258967370ad7b493295da9d8c && git checkout -B drift-repro 5a5336b399db2ef18dd4700f97d579a328197dda && git merge --no-ff 20aa3b0d963eb00258967370ad7b493295da9d8c

node scripts/docs-audit/affected-docs.mjs --json 5a5336b399db2ef18dd4700f97d579a328197dda

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

[github-actions]

⛔ merge queue 构建失败 — 先分诊，再决定要不要重排

队列构建 33716742745 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集），
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job（日志抽取，best effort）：

• Test Core (1/6) — 失败步骤: Run this shard's tests

  @objectstack/cli:test:  FAIL   integration  test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1801
  

↳ 失败原因 是判读的关键：超时（Test timed out in … / Hook timed out in …）多半是负载/时序，不是本 PR 的回归；
断言（AssertionError: …）才指向真实的行为改变。两者的 FAIL 行长得一模一样，只有这一行能区分。

⚠️ 断言这一侧有一类例外，判据是断言在测什么，不是它是不是 AssertionError。 断言的对象是产品行为（一个值、一个形状、一次拒收）⇒ 照上面读：真实的行为改变，去查，⛔ 不要重排掉；
断言的对象是这次实验自身的有效性前提（跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件）⇒ 它跟超时是同一类，同样对负载敏感，重排一次是合法的判别手段。
识别是机械的：断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为（真回归）；
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提：它守护的那条不变式当时是绿的，同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量，仍然是时间测量。（⛔ 这只改「怎么读一次红」，不改「哪些测试可以重排」——后者由别处管。）

跨 PR 相同签名（24h，按失败测试文件聚合）：

• test/run-dev-unbuilt-workspace.e2e.test.ts — 24h 内已弹出 26 个 PR（fix(plugin-sharing): let system writes materialize sharing rules — drop the isSystem skips in bindRuleHooks (#13533) #14528、test(cli): make the unread-reader ceiling a load-independent constant at RUN_TIMEOUT_MS #14715、test(cli): derive the vitest tier population instead of freezing it in a list #14717、fix(sharing): stamp organization_id on every sys_record_share write, backfill the stranded rows, admit the object to the tenancy ledger (#14484) #14726、fix(devx): partition os-regen-merge.sh's step-1 conflict message by class #14733、fix(spec): send a top-level flow trigger to the START node config, not to a type rename #14735、fix(deps): take the fix for the four OSV advisories blocking every manifest PR #14736、fix(tooling): a template-literal mode for the vocabulary gate's four shared textual scanners #14739、test(runtime): pin the measured loadMany vs by-name divergence for #14423 (a) #14741、test(plugin-approvals): pin the lock release in the tenant-admin reverse check #14750、fix(lint): consolidate five more private "Did you mean?" copies onto suggestName #14757、fix(metadata-protocol): apply the allowOrgOverride read gate inside getMetaItems, so multi-type sweeps are scoped per type #14767、feat(dispatch-gates): a checkable whole-tree-population channel, so a repo-wide gate is placed by declaration instead of landing near undetermined #14774、feat(spec,platform-objects,service-messaging,plugin-auth): sys_user.locale + per-recipient notification locale (#13881) #14775、skills(data): drop the two bare ADR-0010 citations from the objectstack-data skill #14777、docs(skills): repair five semantic minors in the axis-weighting and batch-presentation encoding #14779、fix(skills): teach {current_user_id}, not $currentUser, in the objectstack-ui Filtering example #14781、fix(deps): take the fix for four fast-uri OSV advisories (3.1.5 -> 3.1.6) #14783、docs(pm-dispatch): timestamped readings are the first construct-shaped fix; record the remediation-shape ranking beside it #14784、docs(ci): correct lint.yml corpus-claim-drift comment for #13745's shipped rows #14790、docs(skills): settle two false claims in skills/README.md and add three cross-skill routing bullets #14795、docs(i18n): name the measurement-widening race in check-i18n-coverage's header #14799、fix(examples): app-multi-package's module is served WITH scope: 'project' — drop the "no scope key" claim #14804、fix(plugin-auth): register the auth service-composition bindings independently of registerRoutes #14810、fix(service-automation): a try_catch whose catch region itself fails keeps both regions' step record #14813、test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle #14815），分属 5 条投机栈 ⇒ 5 次独立命中。⚠️ 同签名的汇总 issue Queue-flake anchor: test/run-dev-unbuilt-workspace.e2e.test.ts #14822 已作为重复关闭，且没有仍然打开的同签名汇总 issue ⇒ 本次不新建（新建只会再被判重）。请到它指向的那张 issue 上谈，或重开 Queue-flake anchor: test/run-dev-unbuilt-workspace.e2e.test.ts #14822。
• ⚠️ 24h 评论账本没读完（超过 5 页仍未读到窗口尽头），所以上面的「不同 PR 数」是下界，不是全量。

历史信号：

• 本 PR 过去 24h 无队列失败记录（首次）。
• 过去 24h 队列共有 71 个失败构建（不含本次）。

分诊清单：

1. 失败测试在本 PR 改动的包里 → 真回归，修 PR。
2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」；已有汇总 issue ⇒ flaky/环境问题实锤，去那张 issue 上谈，修好前重排只会再烧一轮全队列。
3. 两者都不是 → 可能与同组 PR 语义冲突；等前面的 PR 落地或失败出队后再重排一次即可，不要连续重排。

---

Generated by Claude Code · merge-queue-triage workflow (#4859)