Repository navigation
fix(automation): tell approval decide() callers when a subflow parent strands - #17908
Conversation
… strands bubbleToParent already resumes a subflow's parent when the child completes, but a parent that then strands downstream was reported only to the engine's own error log -- the decide() caller (an approvals decision) still answered resumed: true with nothing to distinguish it from a healthy composition, and its runId still named the healthy child, never the stranded parent. Fill the already-declared, previously-unfilled ApprovalDecisionResult. resumeFailure slot (#16472 family ruling, decision batch #76, option A): the door's status code does not move, but a decide() whose own run resumed while its subflow parent stranded now also carries resumeFailure (the parent's runId + repairable) and a matching resumeError, sourced from a new internal AutomationEngine.takeSubflowParentStrand(childRunId) read-once channel that bubbleToParent populates on its 'stranded' exit only -- RESUME_IN_PROGRESS / STORE_UNAVAILABLE stay the untouched functional degradation, and the engine's own error log line is unchanged. Updates the #15556 reproduction test to its designed-to-go-red truthful shape, keeping both its healthy-composition and direct-throw controls. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj
📓 Docs Drift CheckThis PR changes 2 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 7 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 0861e20b78bd192d6d41de5f60ae89351a917e9a && git checkout 0861e20b78bd192d6d41de5f60ae89351a917e9a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 84e6b05b6d295f1c744d236921300f447cf7791e 41479de9a4dadd56d8ff686ea083723a1527666a && git checkout -B drift-repro 84e6b05b6d295f1c744d236921300f447cf7791e && git merge --no-ff 41479de9a4dadd56d8ff686ea083723a1527666a
node scripts/docs-audit/affected-docs.mjs --json 84e6b05b6d295f1c744d236921300f447cf7791e
|
|
What is failing
Why — and the timeline matters, because the first red had a different cause
The Clause-② correction behind itThis seat declared The claim-time measurement was right as far as it went — Remedy — route 1 of the two the gate names, and ⛔ not route 2The gate offers two non-interchangeable routes: raise the widened package, or correct the declaration at its producer. Route 2 is measurably closed here — the declaration is right; it is the ⛔ Not dropping the changeset, ⛔ not downgrading anything, ⛔ not adding a A changeset is a repository file, so ⛔ this seat does not edit it: the rework is with the implementing dev and the fix arrives as a push to this branch. No re-run is needed or has been spent — the push re-triggers the gate. ⭐ Recorded because it should not be lost in the correction: the dev's reason for not reusing
Generated by Claude Code |
…correction) The delivered diff adds new public surface -- AutomationEngine. takeSubflowParentStrand() and the exported SubflowParentStrand interface on @objectstack/service-automation, plus a matching optional ApprovalResumeSurface member on @objectstack/plugin-approvals -- so Clause-2 is yes on the mechanical "new exported symbol" floor, independent of the wire-compatibility argument in the PR body. Neither package's changes reach any wire payload; the grade moves because the export surface grew, not because behaviour did. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj
Contract reviewHead reviewed: In-seat Clause-② review, default judgement tier (a non-spec seat's Clause-② review is default-tier self-review plus the gates; the at-tier isolated reviewer is the spec seat's requirement, and this PR touches zero ① Derived judgments — every accept-set and public-surface change, named from the diff
② Semver grading — consistent
③ Boundary flags and
|
|
Blocked on #17909 — a Clause-② provenance (the clear)
What is failing, and why no fix comes from this lane
The guard requires the stamping package to own the row, or a ⛔ And there is no spec-free path.
StateCard #15556 → ⇒ When #17909 lands, no code change is expected on this branch: re-run
Generated by Claude Code |
…cision-door-resume-failure
Contract review — RE-ISSUED for the post-base-merge headHead reviewed: Implemented-by:
① What the merge did and did not change — measured
⇒ the merge brought ② Clause-② — re-derived at THIS head, ⛔ not carried overLimb (a) — a new exported symbol reachable from the published entry. Satisfied, twice:
⇒ Clause-②: ⭐ Recorded because this is the determination this seat got wrong at claim time and corrected: the claim-time ⭐ One asymmetry, deliberate and worth naming so a later reader does not "fix" it: the approvals side declares its member with an inline structural type, ⛔ not by importing Limb (b) — a new key on an already-published payload: none. The changeset states it and the diff agrees — ③ The lane red line — held⛔ Verdict: PASS at
|
|
…ILED (objectstack-ai#17924) Fixes objectstack-ai#17909 Clause-②: yes `packages/spec` declared `ResumeFailureReport.code: ErrorCode` as **required** and prescribed `RESUME_FAILED` in its docblock, but the provenance ledger listed that code only under `@objectstack/rest`. So the only correct implementation stamps a registered code its own owner key does not list, and `check:error-code-provenance` refuses it. This adds the missing provenance row under `@objectstack/plugin-approvals`. ## The measurement the card declared NOT MEASURED The card recorded its causal link as *derived from the guard's stated rule, not from the guard's own output* — a fresh worktree answered `tsx: not found`. Dependencies installed, the guard run, and here is its own text. **Before** — branch base `5741ff10c30`, ledger unchanged, PR objectstack-ai#17908's stamp site injected from its head `109c9ce13`: ``` scanned 2301 files; 336 registered-code stamp site(s): 318 listed, 17 waived FAIL — 1 stamp site(s) of a registered code with no provenance row: @objectstack/plugin-approvals stamps 'RESUME_FAILED' (objlit) at packages/plugins/plugin-approvals/src/approval-service.ts:3370 — not listed under its own owner key ``` `VERDICT guard-exit=1`. Exactly one violation, exactly the site the card names. **After** — this PR's row, same injected stamp site: ``` scanned 2301 files; 336 registered-code stamp site(s): 319 listed, 17 waived OK — every registered-code stamp site is listed under its own owner key or carries a recorded waiver (10 waiver(s), all live) ``` `VERDICT guard-exit=0`, and the listed count moves 318 to 319 — the site is admitted as listed, not waived. ## The card frames this as one code; it is three, and only one is stamped The member's docblock names `RESUME_FAILED`, `RESUME_TARGET_LOST` and `RESUME_IN_PROGRESS`. Measured on the tree, not inherited: | code | ledger owner | who stamps it in a scanned position | |---|---|---| | `RESUME_FAILED` | `@objectstack/rest` (`:287`) | rest's catch maps the message prefix to a status, via a variable — not a literal the scan sees. plugin-approvals stamps the literal, once, at `approval-service.ts:3370` (new in objectstack-ai#17908). **The row this PR adds.** | | `RESUME_TARGET_LOST` | `@objectstack/rest` (`:288`) | nobody, in a scanned position. plugin-approvals throws it as a message prefix (`approval-service.ts:3189`); rest's catch maps that prefix. No row owed. | | `RESUME_IN_PROGRESS` | `@objectstack/service-automation` (`:1044`) — **not** rest, correcting the dispatch's open premise | `service-automation/src/engine.ts:5795` and `:6165`, which its own owner key already lists. plugin-approvals only *compares* it (`:3304`). No row owed. | ## Why a row and not a waiver — the precedent's own predicate, applied The `EXTERNAL_IMPORT_ERROR` waiver states the test plainly: the door's catch *"stamps this code itself for EVERY importObject throw and never reads the producer's declaration — the door names the wire vocabulary."* Both halves fail here, and the second decisively: - `resumeFailure` rides a **success** answer. The REST approvals door serves `decide` / `recall` with `res.json(out)` (`rest-server.ts`), which copies the service's result verbatim — there is no envelope for the door to stamp on that path. - `git grep resumeFailure packages/rest/src` matches **nothing**. The door neither stamps nor reads it. The producer's literal *is* the wire value. The door's `handleApprovalError` catch is a real second moment, and it is the *throw* path — already covered by rest's existing row. This PR does not touch it. ⛔ The gate is not evaded. The literal stays exactly where it is; nothing is hoisted into a constant to dodge the textual scan. ## Ablation and cost direction Each leg mutates on disk, proves the mutation landed by occurrence count, runs the guard, and restores under a `trap` verified by `git hash-object` against the `HEAD` blob plus an empty `git diff HEAD`. The guard is a `tsx` script importing `../src/api/error-code-ledger.zod` — source, no `dist` interposed, so no build sits between the edit and the reading. | leg | mutation | expected | measured | |---|---|---|---| | ablation | remove the row, keep the stamp site | reds, same site | `exit=1`, `@objectstack/plugin-approvals stamps 'RESUME_FAILED' … at …:3370`, listed back to 318 | | cost direction | keep the row, stamp `RESUME_TARGET_LOST` instead | still reds — the row must not over-permit | `exit=1`, names `RESUME_TARGET_LOST` at `:3290`. The row admits exactly the one code measured | | objectstack-ai#15970 coverage | keep the row, stamp `RESUME_FAILED` at **two** sites | green — a row is package-scoped | `exit=0`, 320 listed | ## Does this unblock objectstack-ai#15970? **Yes, conditionally, and the condition is measured rather than asserted.** The guard reconciles on `(package, code)`, not on line number, so any number of `RESUME_FAILED` sites inside `@objectstack/plugin-approvals` are admitted by this one row — the third leg above proves it with two sites. `ApprovalRecallResult.resumeFailure` is the same member type in the same package, so an implementation that stamps `RESUME_FAILED` is covered with no further ledger edit.⚠️ It is **not** covered if objectstack-ai#15970's implementation stamps `RESUME_TARGET_LOST` for a lost run: the cost-direction leg is exactly that case, and it reds. That is the intended narrowness, not a gap — that code's wire emission belongs to rest's catch, and if a producer ever stamps it on a success answer it owes its own reading, not a pre-emptive row. ## Clause ② and the changeset level Declared **`Clause-②: yes`**, measured rather than argued: `node scripts/pm/check-widening-tells.mjs --declaration no` answers `exit=4` with `✗ T4 … a new registration in a registry / catalog` at `error-code-ledger.zod.ts:1084`; with `--declaration yes` it answers `exit=0`. The ledger's own header agrees — *"Registering a code widens this face and is therefore a Clause-② change, door or no door."* The changeset is `minor` accordingly.⚠️ Worth a reviewer's eye, and stated rather than hidden: nothing an implementation may now *emit* actually changes. `RESUME_FAILED` was already in the registered union under rest, so no response can carry a code it could not carry before — the per-package rows are provenance, not identity, as this file's header says. The `yes` is the conservative direction, not a claim that the accept set grew. `needs:contract-review` is hung on this PR. ## Verification | check | verdict | |---|---| | `pnpm --filter @objectstack/spec check:error-code-provenance` | `exit=0` | | `pnpm --filter @objectstack/spec test` (`--project local`) | `exit=0` — 473 files passed, 1 skipped; 13483 tests passed, 1 skipped | | `pnpm --filter @objectstack/spec test:repo` (`--project repo`) | `exit=0` — 31 files, 523 tests passed | | `pnpm --filter @objectstack/spec typecheck` | `exit=0` | | `pnpm check:nul-bytes` | `exit=0` — 8541 text files scanned | | `pnpm check:error-code-casing` | `exit=0` | | `pnpm check:dispatcher-error-vocabulary` | `exit=0` | | spec gate family (`check:liveness`, `check:duration-unit-keys`, `check:empty-state`, `check:variant-docs`, `check:llms-txt`, `check:skill-refs`, `check:strictness-ledger`, `check:yaml-examples`, `check:objectui-pin-citations`, `check:export-origins`) | all `exit=0` | | `pnpm --filter @objectstack/spec build` + `check:api-surface` / `check:authorable-surface` / `check:docs` / `check:exported-any` / `check:dual-source-exports` / `check:entry-nameability` / `check:browser-reachable-entries` / `check:generated` | all `exit=0` — no generated artifact moves | | derived gate sweep (`node scripts/pm/dispatch-gates.mjs --commands`, all 78 families, reconciled with `--ran`) | 75 `exit=0`, 3 `exit=3` **NOT MEASURED** | The three NOT-MEASURED families are `check:doc-formula-expressions`, `check:dual-build-cjs-loads` and `check:lean-entry-closure` — each exits 3 with `PREREQUISITE NOT MET`, wanting a repo-wide `pnpm build` this diff does not otherwise need. CI's *Build Core* supplies it; recorded as not measured, ⛔ not as green. Heavy steps ran through `scripts/pm/os-verify-lock.sh` on a shared box; verdicts are read from each command's own exit code captured before any pipe. ## 维护者速读(草稿) **改了什么。** `packages/spec/src/api/error-code-ledger.zod.ts` 里,在 `'@objectstack/plugin-approvals'` 的 owner key 下加了一行 `'RESUME_FAILED'`,附带记录 wire 路径的注释;外加一份 `minor` changeset。一个数据表条目,没有新导出符号,没有已发布载荷上的新键。 **为什么改。** spec 自己把 `ResumeFailureReport.code` 声明成必填并在 docblock 里点名 `RESUME_FAILED`,却没给会填它的那个包登记 provenance 行 —— 于是"照声明实现"这件事本身会被姊妹门禁 `check:error-code-provenance` 拒收。第一个来填槽的实现者只是第一个发现的人。这一行把声明补齐。 **风险与代价(含回滚)。** 风险低:`RESUME_FAILED` 本来就在已注册并集里(挂在 rest 名下),这一行只改 provenance 归属,不让任何响应能带上它此前带不了的码。代价是一条 `minor` changeset(条款② 的机械判据命中 T4,按保守方向申报 `yes`)。回滚 = 删掉这一行加这份 changeset,单文件 revert,无迁移、无数据、无 wire 影响。 **席位意见。** (留空,待席位定稿) **你要做的。** 确认两件事:(1) 同意这里走"加行"而不是"加 waiver" —— 判据是先例那句可测的话("门自己 stamp 且从不读生产者的声明")在成功路径上两半都不成立,证据在上面的 `res.json(out)` 与 `packages/rest/src` 零命中;(2) 条款② 申报为 `yes` 属于保守方向,而非真的放宽了接受集 —— 若你认为这类"已注册码的第二 emitter 行"不该触发契约复核,那是门禁 T4 的判据问题,值得单独立卡,⛔ 不在本 PR 处理。 ## Unblocks - **PR objectstack-ai#17908 (objectstack-ai#15556)** — blocked on this; its only failing step was *Error-code provenance guard*. `objectstack-ai#15556 is not addressed here.` - **objectstack-ai#15970** — covered for a `RESUME_FAILED` stamp, measured above. `objectstack-ai#15970 remains open.` --- _Generated by [Claude Code](https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH)_ Co-authored-by: Claude <noreply@anthropic.com>
… the run (objectstack-ai#17988) Fixes objectstack-ai#15970 Clause-②: no `recall` resumes its run DIRECTLY rather than through `resumeRecordedOutcome`, so the `resumeCode` / `resumeStatus` the engine already stamps on the error were discarded one line before the result was built. The caller got `resumed: false` plus one sentence of prose and no way to learn the run is repairable — while the identical strand reached through `decide` has carried `{ finalized, decision, runId, repairable: true }` since objectstack-ai#13807. The difference was the door, not the strand. This fills `ApprovalRecallResult.resumeFailure`, a slot `@objectstack/spec` already declared (`contracts/approval-service.ts`) and left without a producer on this door. Zero `packages/spec` files touched. ## The card's table, reproduced BEFORE the change Real `AutomationEngine` + `InMemorySuspendedRunStore` + real `ApprovalService` + real `registerApprovalNode`, one live process, in-memory ObjectQL double for the approvals tables only. `deal_approval` parks at an `approval` node; `mark_rejected` throws the card's own text. Verbatim from the probe's stdout, worktree at `c8a006fc4`: | read | result | |---|---| | the call | no throw — returned normally | | `result.resumed` | `false` | | `result.resumeError` | `"resume of run 'run_a2cb9106…' failed: update_record(crm_leave_request) failed: Record 9SEmlyRfw8D9-J7Z not found"` | | `result.resumeFailure` | **`undefined`** | | `result.request.status` | `"recalled"` | | `strandedDecisionDetails(result)` | `undefined` | | `engine.hasSuspendedRun(runId)` | `false` | | `'resumeFailure' in result` | `false` | | `engine.restoreConsumedSuspension(runId)` | `{"restored":true,"reason":"… the run is resumable again; re-issue the continuation"}` | | `marks` (downstream work) | `[]` | CONTROL in the SAME run — the identical strand through `decide`: | read | result | |---|---| | the call | THREW `"RESUME_FAILED: the reject decision was recorded on request areq_7b2f3a61…, but its flow run 'run_f4601c41…' could not be resumed and is now stranded: …"` | | `strandedDecisionDetails(err)` | `{"finalized":true,"decision":"reject","runId":"run_f4601c41…","repairable":true}` | | `engine.hasSuspendedRun(runId)` | `false` | ⭐ One card row is corrected by measurement and stays corrected: `strandedDecisionDetails(result)` is **`undefined`, not `null`** — and that is not something this PR could have fixed. It is the ERROR-envelope reader for a door that THROWS; against a recall RESULT object it can only ever answer `undefined`, whatever the door does. The ruling put the success-answer carrier on `resumeFailure` for exactly that reason (the spec's own "⛔ Not `StrandedDecisionDetails`" note). ## The same table AFTER, same harness, same lever | read | result | |---|---| | the call | no throw — returned normally (unchanged, and asserted so it stays unchanged) | | `result.resumed` | `false` | | `result.resumeError` | `"resume of run 'run_d0bad676…' failed: update_record(crm_leave_request) failed: Record 9SEmlyRfw8D9-J7Z not found"` | | `result.resumeFailure` | **`{"code":"RESUME_FAILED","runId":"run_d0bad676…","status":"stranded","repairable":true}`** | | `result.request.status` | `"recalled"` | | `strandedDecisionDetails(result)` | `undefined` (see above — correct, not unfixed) | | `engine.hasSuspendedRun(runId)` | `false` | | `'resumeFailure' in result` | **`true`** | | `engine.restoreConsumedSuspension(runId)` | `{"restored":true,…}` — the repair `repairable: true` promises, cashed in the pin | | `marks` (downstream work) | `[]` | The `decide` control is byte-identical in the after run, which is the point: the harness discriminates the two doors rather than passing vacuously. ## What changed `recall`'s non-revise-window arm, and nothing else: - the `catch` now hoists objectstack-ai#13807's own derivation — `const status = resumeStatusOf(err); const repairable = status === 'stranded'` — instead of testing it inline, and on the repairable exit stamps `resumeFailure: { code: 'RESUME_FAILED', runId, status: 'stranded', repairable: true }` beside the existing `journalStrandedContinuation` stash; - the SUCCESS path now reads the value `serviceResume` already returned to it and discarded. objectstack-ai#17908 landed `takeSubflowParentStrand` and wired it into `resumeRecordedOutcome` (which serves `decide` / `sendBack` / `resubmit`) and said in its own body that this door "would trivially gain the same fix … but that fold-in belongs to objectstack-ai#15970". The spec declares this door's answer for that shape already — `ApprovalRecallResult.resumed`: *"a resume that completed and then stranded a run further up (a subflow's parent, objectstack-ai#15556) still answers `true`, with the strand told on `resumeFailure`"* — so leaving it out would have left a declared contract with no producer; - the result spreads `resumeFailure` conditionally, omitted rather than `undefined`, the same spelling `decide` uses, so `'resumeFailure' in result` reads presence correctly. ## Fences, each one measured rather than assumed - ⛔ **The no-throw stays.** PIN 1 asserts the call returns normally; a fix that made `recall` fail would be the wrong fix, not a stricter one. - 🔴 **`RESUME_FAILED` and only that code.** PIN 1 asserts the literal. Two exits report; every other exit deliberately reports NOTHING, and PIN 3 pins that: a lost run's honest code is `RESUME_TARGET_LOST` and the tolerated duplicate's is `RESUME_IN_PROGRESS`, and this package's ledger row admits neither — its own comment spells out that this package stamps neither in a scanned position. Stamping `RESUME_FAILED` there would make the discriminator lie about WHICH failure it was, which is the defect being fixed, one field over. Absence is declared legitimate by the member's docblock ("An absent member means no report was made, never that no run is stranded"). - ⛔ **Nothing stopped at the `packages/spec` fence and nothing needed to.** The honest code for both exits this PR fills is `RESUME_FAILED`, which the ledger already admits. See Acceptance notes for the one adjacent observation. - ⛔ **The log line is untouched** — same message, same `error` level, same three context keys, asserted in PIN 1. The ruling left logging alone; the report is a sibling of that line, not a replacement. - ⛔ `content/docs/releases/**` untouched. `packages/spec` untouched (0 files). ## Tests New `packages/plugins/plugin-approvals/src/recall-strand-discriminator.test.ts` — PIN 1 (the card's table row for row), PIN 2 (a healthy recall, so PIN 1's presence is a reading and not the fixture), PIN 3 (the non-stranded exit that reports no code, with its reason), CONTROL (the identical strand through `decide`). `subflow-hosted-approval-strand.test.ts` gains the recall case for the objectstack-ai#15556 shape. **Ablation — the pins go red on the defect.** Mutation = restoring the pre-fix production file from `c8a006fc4`, hash-verified on disk both ways, under a `trap … EXIT INT TERM` with an absolute repo root. ``` HEAD blob fa932e3 / BASE blob 9a68d36 LEG 0 clean on-disk fa932e3 · marker 1 · pre-fix guard line 0 · 8 passed LEG 1 mutated on-disk 9a68d36 (== BASE) · marker 0 · pre-fix guard line 1 vitest exit=1 — Test Files 2 failed (2), Tests 2 failed | 6 passed (8) AssertionError: expected undefined to deeply equal { code: 'RESUME_FAILED', …(3) } x2 LEG 2 restored on-disk fa932e3 (== HEAD) · marker 1 · `git diff HEAD` empty · `git status --porcelain` empty vitest exit=0 — Test Files 2 passed (2), Tests 8 passed (8) ``` Predicted direction was RED and RED is what it did; exactly the two new `resumeFailure` assertions failed and the four controls stayed green. `node scripts/ablation-dist-preflight.mjs @objectstack/plugin-approvals '…' --absent` reported `no dist/ in this package`, which is the correct reading here rather than a void run: both test files import the mutated module RELATIVELY (`./approval-service.js`), so vitest resolves it from `src` and no `dist` sits on its resolution path. The hazard that script exists to stop is a false GREEN from a stale build; this ablation's observed colour is RED, which a stale build cannot produce. Commands, all under the shared verify lock, `VERDICT command-exit` read rather than a bare `$?`: - `pnpm --workspace-concurrency=2 --filter '@objectstack/plugin-approvals^...' build` — exit 0 (dependency closure, built before any reading). - `pnpm --filter @objectstack/plugin-approvals test` — exit 0, **45 files / 738 tests pass** (was 44 / 733 on `main`). - `pnpm --filter @objectstack/plugin-approvals typecheck` — exit 0; `check:test-typecheck: OK — 8 file(s) / 324 error(s) / 27 pinned signature(s)`, the shrink-only ledger unchanged (the new test file is NOT in it; its two initial TS7006 were fixed by typing the recorder, not by expanding the ledger). - `pnpm exec turbo run build --filter=!@objectstack/docs --concurrency=2` — exit 0, 73/73 tasks. - `pnpm lint` (`eslint . --no-inline-config`, the whole repo, no narrowing) — exit 0. **Gate families: 71 derived, 71 run, 0 NOT-MEASURED, 0 UNRUN**, reconciled mechanically at `76ee9926e` with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran FILE`, every family recorded with its exit code: ``` ✓ dispatch-gates --ran: 71 derived famil(ies) accounted for — 71 run, 0 NOT-MEASURED (a DERIVED zero — all 71 recorded an exit code and none of them is 3). ``` Three of them answered **exit 3 (PREREQUISITE NOT MET)** on the first pass — `check:dual-build-cjs-loads`, `check:i18n`, `check:type-check-debt` — which is a statement about the tree and not a pass. The closure they name was built (the `turbo run build` line above) and all three were re-run to a real verdict: **exit 0** each. `check:engine-double-contract` answered exit 1 with its own remedy (a new pinned engine double the ledger had not learned about); regenerated with `--write`, 0 seam rows added or grown, 0 lost, gate now exit 0. The gate this lane was warned about, verbatim: ``` $ pnpm --filter @objectstack/spec check:error-code-provenance > tsx scripts/check-error-code-provenance.ts --self-test && tsx scripts/check-error-code-provenance.ts self-test OK — 3 patterns and every waiver direction pinned bounds: packages/**/src non-test .ts/.tsx; ledger file excluded (mention, not stamping); bounds: patterns = objlit, assign, constdef — blind to non-*_CODE constants, bounds: helper/shorthand indirections, templates and class fields (see the header; sibling-gate shapes). scanned 2304 files; 338 registered-code stamp site(s): 321 listed, 17 waived OK — every registered-code stamp site is listed under its own owner key or carries a recorded waiver (10 waiver(s), all live) exit=0 ``` ## Clause-② — re-derived from the DELIVERED diff, not predicted **`no`.** Five files: the production change, two test files, the changeset, and the gate ledger JSON. 1. **New exported symbols reachable from the published entry: zero.** Every added line in the whole diff was searched for the word `export` — `git diff c8a006f..HEAD | grep '^+' | grep -w export` returns nothing at all. `src/index.ts` is untouched (0 hits in the changed-file list) and so is `package.json`, whose `exports` map is a single `.` pointing at `./dist/index.*`; the two test files are outside the `src/index.ts` build graph and outside the published `files: ["dist", "README.md", "CHANGELOG.md"]`. 2. **New key on an already-published payload: none.** `resumeFailure` was already declared, exported and type-pinned on `ApprovalRecallResult` in `@objectstack/spec` before this PR — `resume-failure-report.pin.test.ts` carries a type-level `Eq` assertion (named `RecallCarriesTheReport`) holding `ApprovalRecallResult`'s `resumeFailure` member equal to `ResumeFailureReport` or undefined. This PR supplies its first producer. Per the dispatch skill's own boundary test, 「条款②只指已发布契约面,拉回已声明契约不触它」 — bringing an already-declared contract into effect does not touch Clause-②. Neither the "an existing export's behaviour changed" reading nor wire compatibility is the floor, so neither was used. Changeset graded **`patch`** accordingly (`@objectstack/plugin-approvals` only). ## Acceptance notes Noted while working, not filed and not fixed here: - **The `RESUME_TARGET_LOST` boundary is real but is not a defect.** A recall whose run is already gone is still told in prose and carries no machine-readable half, because the honest code for it is one this package's ledger row does not admit. That is the row working as designed — its comment says so in as many words — and PIN 3 records the boundary so the next reader does not "fix" it by stamping the wrong code. If a consumer ever needs to branch on that exit, it is a `packages/spec` ledger event and belongs to the `domain:spec` seat, exactly as objectstack-ai#17909 was routed. Nothing in this PR stopped at that fence. - **`recall`'s revise-window arm** (`cancelRun`, ADR-0044) tolerates its own failure with prose and no report. A cancel is not a resume, so `ResumeFailureReport` has no referent there; left exactly as it was. - **The `missingRunCapability` exit** already spells `[RESUME_FAILED]` inside its prose for a composition with no engine, and `resumeRecordedOutcome` does not stamp a report there either. Left symmetric with the sibling door rather than diverging on this card. --- _Generated by [Claude Code](https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
The paragraph conflated three outcomes of a paused subflow chain into one
prose block, and its closing sentence ("an ancestor is never stranded,
because resuming it is not what moves it") was written for the child-strand
case only. #15556 (PR #17908, merged c8a006f) shipped a third case the
paragraph never described: the child completes, bubbleToParent resumes the
parent, and the parent's own downstream node throws — the parent itself
lands on the engine's 'stranded' exit and is reported on resumeFailure.
Measured against origin/main a88a1bb:
- packages/services/service-automation/src/engine.ts:1737 (SubflowParentStrand),
:7434 (the 'stranded' exit bubbleToParent records it on)
- packages/plugins/plugin-approvals/src/approval-service.ts:3476 (the
resumeFailure shape: RESUME_FAILED / stranded / repairable)
…bjectstack-ai#20377) Fixes objectstack-ai#17940 Clause-②: no ## What was wrong `content/docs/automation/flows.mdx`'s subflow-chain repair paragraph conflated three distinct outcomes into one block of prose, and its closing sentence — "an ancestor is never *stranded*, because resuming it is not what moves it" — was written for exactly one of them. objectstack-ai#15556 (PR objectstack-ai#17908, merged `c8a006fc41`) shipped a case the paragraph never described, where that sentence is false: the child completes, `bubbleToParent` resumes the parent, and the parent's own downstream node throws. There the bubble is exactly what moves the parent, and the parent itself lands on the engine's `'stranded'` exit. ## Before > A child that fails terminally after the pause fails every waiting ancestor, so > no run is stranded as resumable-forever. **When the child's failure is a > strand** — its resume consumed the pause and a downstream node threw — each > ancestor's consumed pause is recorded too, and the repair verb puts the whole > chain back in one call: `POST …/runs/{runId}/restore-suspension` on **any** > member re-arms every member, deepest first, so the continuation re-issued on > the run you named flows back up through the ancestors instead of completing a > leaf into a parent that never continues. Re-arming an ancestor is all the verb > does — an ancestor is never *stranded*, because resuming it is not what moves > it. A cascade from a child that is **not** repairable records no ancestor > snapshot, deliberately, so the verb never promises a chain repair it could not > finish. ## After (revised per PM review — no self-reference, repair verb named) > A child that fails terminally after the pause fails every waiting ancestor, so > none of them is left stranded as resumable-forever. **When the child's failure > is a strand** — its resume consumed the pause and a downstream node threw — > each ancestor's consumed pause is recorded too, and the repair verb puts the > whole chain back in one call: `POST …/runs/{runId}/restore-suspension` on > **any** member re-arms every member, deepest first, so the continuation > re-issued on the run you named flows back up through the ancestors instead of > completing a leaf into a parent that never continues. Re-arming an ancestor is > all the verb does in this case — no ancestor is stranded here, because > resuming it is not what moves it. A cascade from a child that is **not** > repairable records no ancestor snapshot, deliberately, so the verb never > promises a chain repair it could not finish. > > **A third case is the opposite: the bubble itself is what strands an > ancestor.** The child completes cleanly, `bubbleToParent` resumes the parent > on the child's behalf, and the parent's own downstream node throws. There the > bubble — not a resume the caller issued — is exactly what moves the parent, > and it is the parent, not the child, that lands on the engine's `'stranded'` > exit, terminal. The child's own resume genuinely succeeded, so its resumer > (an approvals decision door, a wait timer) is told the resume succeeded; as > of objectstack-ai#15556 an approval `decide()` also reports the stranded parent on > `resumeFailure` (`{ code: 'RESUME_FAILED', runId: '<parent>', status: > 'stranded', repairable: true }`). Repair it the same way: the same > `restore-suspension` verb (`restoreConsumedSuspension` underneath), issued on > the parent's run id — the `runId` `resumeFailure` names, not the child's. ## Code measured on `origin/main` `a88a1bb39` (not copied from the card) - `packages/services/service-automation/src/engine.ts:1737` — the `SubflowParentStrand` interface (`runId`, `repairable: true`, `error`), recorded only on the arm `AutomationResult.status` calls `'stranded'`. - `packages/services/service-automation/src/engine.ts:7434` — `bubbleToParent`: `if (parentRes.status === 'stranded')` records the `SubflowParentStrand` under the **child's** own run id. - `packages/services/service-automation/src/engine.ts:7511` — `takeSubflowParentStrand(childRunId)`, the delete-on-read hand-off. - `packages/plugins/plugin-approvals/src/approval-service.ts:3476` — the approvals decision door's `resumeFailure` on a `bubbleStrand`: `{ code: 'RESUME_FAILED', runId: bubbleStrand.runId, status: 'stranded', repairable: bubbleStrand.repairable }` — matches the card's claimed shape. - `packages/services/service-automation/src/engine.ts:7994` — `restoreConsumedSuspension`, the repair verb for the parent strand. - `packages/runtime/src/domains/automation.ts:2752` — the REST door, `POST /:name/runs/:runId/restore-suspension`, routes `parts[2]` (the `:runId` path segment) straight into `automationService.restoreConsumedSuspension(parts[2], …)` — so the same `restore-suspension` verb an operator calls in case (a)/(b) is what a third-case operator calls too, on the parent's run id (the `runId` `resumeFailure` names). Binding honoured (thread comment `5697194225`): objectstack-ai#17541 owns the naming of any new `AutomationResult.status` member. This PR coins none — `'stranded'` is the status the engine and the approvals door already use today. ## PM review addendum PM review verified all code anchors and asked for two prose fixes, applied in commit `28c110796`: 1. Dropped the two sentences that referred to the page's own prose ("that sentence is scoped to…" / "the claim above does not hold for it") and restated the scoping as behaviour. 2. Named the third case's repair verb the same way the other two cases do — the `restore-suspension` REST verb (`restoreConsumedSuspension` underneath), issued on the parent's run id — instead of only the engine method name. ## Gates run (docs-only change, no changeset — `content/docs/**` is not a published package surface) `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands content/docs/automation/flows.mdx` derived 40 command(s), same 40 before and after the revision. All 40 ran green both times (reconciled with `--ran`: `40 derived, 40 run, 0 NOT-MEASURED, 0 UNRUN`). One-time prerequisite builds these gates needed (`@objectstack/formula` + `@objectstack/lint`, and `@objectstack/client` + `@objectstack/client-react`) — neither package's source was touched by this diff. Full command list and outputs are in the report comment on objectstack-ai#17940. Serial neighbour: draft PR objectstack-ai#20344 edits the same file at `:1357` and below; this diff's hunk sits at `:1104`–`:1129`, 245+ lines above it — no overlap. --- _Generated by [Claude Code](https://claude.ai/code/session_01VDtqoecgES7ScQYGbFVDRv)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #15556
Update (2026-09-13)
The review seat corrected this PR's contract classification after reading the delivered diff: it adds a new public method (
AutomationEngine.takeSubflowParentStrand) and a new exported interface (SubflowParentStrand) to@objectstack/service-automation, and a matching optional member on@objectstack/plugin-approvals' already-exportedApprovalResumeSurface— new public surface, whatever the wire does. The changeset now grades both packagesminorinstead ofpatch, andneeds:contract-reviewis attached. Nothing else about the delivery changed — see the changeset for exactly what grew and why.What changed
bubbleToParent(packages/services/service-automation/src/engine.ts) already resumes a subflow's PARENT when its child completes. When that parent consumes its own suspension and then fails downstream — the engine's'stranded'exit — the failure was reported only to the engine's ownerrorlog. The caller who resumed the child (an approvalsdecide()) still answeredresumed: truewith nothing to distinguish it from a fully healthy composition, and itsrunIdstill named the healthy CHILD, never the stranded PARENT.This fills the slot the #16472 family ruling (maintainer 2026-09-07, decision batch #76, option A) already declared and left unfilled:
ApprovalDecisionResult.resumeFailure?: ResumeFailureReportin@objectstack/spec(packages/spec/src/contracts/approval-service.ts:785). The door's status code does not move —decide()still never throws for this shape — but the answer now carries the strand behind it.Plumbing — and why it is new public surface, not a wire change.
bubbleToParent's'stranded'exit now also records aSubflowParentStrand({ runId, repairable: true, error }) in a new bounded, per-processMap<childRunId, SubflowParentStrand>onAutomationEngine, read once (and cleared) via a new publictakeSubflowParentStrand(childRunId).plugin-approvals'sserviceResumecalls it right after its own resume reports success, andresumeRecordedOutcometurns a hit intoresumeFailure+resumeErroron thedecide()result.RESUME_IN_PROGRESS/STORE_UNAVAILABLEbubble outcomes record nothing — they stay the untouched functional degradation — andbubbleToParent's ownerrorlog line is byte-for-byte unchanged (the ruling explicitly leaves logging alone).I deliberately did not reuse the generic
AutomationEngine.resume()/AutomationResultreturn value as the carrier: that object is served verbatim to a raw RESTPOST …/resumecaller too (deps.success(result)inpackages/runtime/src/domains/automation.ts, out of my file surface), so adding a field there would leak an undeclared key onto that wire path for every subflow resume, not just an approvals-mediated one. The alternative — a new engine method mirroring the existinginspectConsumedSuspension/hasSuspendedRun/listSuspendedRunsDurablepattern already onApprovalResumeSurface— avoids that leak, but it is itself new exported surface on both packages (see the changeset), which is the thing declared here rather than argued away.Updated the #15556 reproduction test (
subflow-hosted-approval-strand.test.ts) to its designed-to-go-red truthful shape (its own header said the fix must turn it red on purpose), keeping both controls: CONTROL A (healthy composition — still the sharedFULL_SUCCESSliteral, now asserted to diverge from the stranded case) and CONTROL B (the #13807 direct-throw shape, unaffected). Also refreshed two now-stale "#15556 open decision" doc comments inservice-automationandplugin-approvalsthat this PR itself closes.Scope discipline
packages/spectouched. Verified before starting:ApprovalDecisionResult.resumeFailure, its docblock's absence rule, and the widenedresumeErrordocblocks are already landed and pinned (resume-failure-report.pin.test.ts, re-run below, green, untouched). This PR only produces a value for the already-declared slot.recall()(approvals: arecallwhose resume strands reports it as an ordinary non-failure — norepairablediscriminator, where the identical strand throughdecidecarries one #15970's door) not touched — same file, deliberately left alone per the sibling fence. It callsserviceResumetoo and would trivially gain the same fix (its return value is just discarded today), but that fold-in belongs to approvals: arecallwhose resume strands reports it as an ordinary non-failure — norepairablediscriminator, where the identical strand throughdecidecarries one #15970, not this PR.errorfor'stranded',warnfor the two tolerated arms) is untouched, per the ruling.Tests
pnpm --filter @objectstack/plugin-approvals exec vitest run src/subflow-hosted-approval-strand.test.ts— 3/3 pass (the fixed reproduction + both controls).pnpm --filter @objectstack/service-automation exec vitest run src/subflow-bubble-strand-log-level.test.ts src/nested-strand-chain-restore.test.ts src/engine-residual-log-cause.test.ts— 38/38 pass (log-level pins unaffected, both directions).pnpm --filter @objectstack/spec exec vitest run src/contracts/resume-failure-report.pin.test.ts— 6/6 pass (spec side untouched, still green).pnpm --filter @objectstack/service-automation test— 132 files / 1564 tests pass.pnpm --filter @objectstack/plugin-approvals test— 44 files / 733 tests pass.pnpm --filter @objectstack/service-automation typecheckandpnpm --filter @objectstack/plugin-approvals typecheck— both exit 0 (plugin-approvals' pre-existing 324-error test-typecheck debt ledger is unchanged, not mine).pnpm --filter @objectstack/service-automation build/pnpm --filter @objectstack/plugin-approvals build(with their dependency closures) — both exit 0, DTS emitted clean.node scripts/pm/dispatch-gates.mjs --commands(re-derived after the changeset landed) named 63 families; all 63 run. 60 exit 0. 3 exit3(PREREQUISITE NOT MET), reported as NOT MEASURED, never as a pass:check:dual-build-cjs-loads,check:i18n,check:type-check-debt— all three refuse because they read a full-monorepodist/this local run never built (out of the ①②③ local-verification scope; CI builds the whole tree). None is in a family my diff plausibly affects.Changeset
.changeset/15556-subflow-parent-strand-on-decide.md—minoron both@objectstack/service-automationand@objectstack/plugin-approvals(corrected from an initialpatch— see Update above): the fix is additive with no migration, but it adds genuinely new public surface (AutomationEngine.takeSubflowParentStrand,SubflowParentStrand, and a matching optionalApprovalResumeSurfacemember), which is what aminorgrade is for.Generated by Claude Code