Skip to content

fix(automation): tell approval decide() callers when a subflow parent strands - #17908

Merged
os-project-manager merged 3 commits into
mainfrom
claude/issue-15556-decision-door-resume-failure
Sep 13, 2026
Merged

os-project-manager merged 3 commits into
mainfrom
claude/issue-15556-decision-door-resume-failure

Conversation

@os-project-manager

@os-project-manager os-project-manager commented Sep 13, 2026 •

Copy link
Copy Markdown
Collaborator

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-exported ApprovalResumeSurface — new public surface, whatever the wire does. The changeset now grades both packages minor instead of patch, and needs:contract-review is 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 own error log. The caller who resumed the child (an approvals decide()) still answered resumed: true with nothing to distinguish it from a fully healthy composition, and its runId still 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?: ResumeFailureReport in @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.

FROM  service.decide(requestId, { decision: 'approve' }, ctx)
      -> { finalized: true, decision: 'approve', runId: '<child>', resumed: true }
         // identical to a healthy composition's answer

TO    service.decide(requestId, { decision: 'approve' }, ctx)
      -> { finalized: true, decision: 'approve', runId: '<child>', resumed: true,
           resumeError: "RESUME_FAILED: … its own flow run '<child>' resumed, but the " +
                        "subflow parent above it — run '<parent>' — consumed its suspension " +
                        "and is now stranded: <downstream error>",
           resumeFailure: { code: 'RESUME_FAILED', runId: '<parent>', status: 'stranded', repairable: true } }

Plumbing — and why it is new public surface, not a wire change. bubbleToParent's 'stranded' exit now also records a SubflowParentStrand ({ runId, repairable: true, error }) in a new bounded, per-process Map<childRunId, SubflowParentStrand> on AutomationEngine, read once (and cleared) via a new public takeSubflowParentStrand(childRunId). plugin-approvals's serviceResume calls it right after its own resume reports success, and resumeRecordedOutcome turns a hit into resumeFailure + resumeError on the decide() result. RESUME_IN_PROGRESS / STORE_UNAVAILABLE bubble outcomes record nothing — they stay the untouched functional degradation — and bubbleToParent's own error log line is byte-for-byte unchanged (the ruling explicitly leaves logging alone).

I deliberately did not reuse the generic AutomationEngine.resume() / AutomationResult return value as the carrier: that object is served verbatim to a raw REST POST …/resume caller too (deps.success(result) in packages/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 existing inspectConsumedSuspension / hasSuspendedRun / listSuspendedRunsDurable pattern already on ApprovalResumeSurface — 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 shared FULL_SUCCESS literal, 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 in service-automation and plugin-approvals that this PR itself closes.

Scope discipline

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 typecheck and pnpm --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 exit 3 (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-monorepo dist/ 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 — minor on both @objectstack/service-automation and @objectstack/plugin-approvals (corrected from an initial patch — see Update above): the fix is additive with no migration, but it adds genuinely new public surface (AutomationEngine.takeSubflowParentStrand, SubflowParentStrand, and a matching optional ApprovalResumeSurface member), which is what a minor grade is for.


Generated by Claude Code

… 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
@github-actions

github-actions Bot commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-approvals, @objectstack/service-automation, touching 13 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 runId (symbol, a field of interface SubflowParentStrand))
  • content/docs/automation/approvals.mdx (via runId (symbol, a field of interface SubflowParentStrand))
  • content/docs/automation/flows.mdx (via ApprovalService (symbol, a top-level class), runId (symbol, a field of interface SubflowParentStrand))

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

  • content/docs/releases/v16.mdx (via AutomationEngine (symbol, a top-level class))
  • content/docs/releases/v17/17-0.mdx (via AutomationEngine (symbol, a top-level class), runId (symbol, a field of interface SubflowParentStrand))
  • content/docs/releases/v17/17-1.mdx (via runId (symbol, a field of interface SubflowParentStrand))
  • content/docs/releases/v17/17-3.mdx (via runId (symbol, a field of interface SubflowParentStrand))

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
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 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 — 7 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 84e6b05b6d295f1c744d236921300f447cf7791e → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 0861e20b78bd192d6d41de5f60ae89351a917e9a — the merge of head 41479de9a4dadd56d8ff686ea083723a1527666a into base 84e6b05b6d295f1c744d236921300f447cf7791e, 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 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

⚠️ 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 84e6b05b6d295f1c744d236921300f447cf7791e → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator Author

Check Changeset is red, the cause is this seat's Clause-② correction, and the remedy is a changeset regrade that is with the implementing dev now.

What is failing

Check Changeset, latest run on head 3e17091b3 (103643451392, started 00:09:36Z). Its verdict, verbatim:

declaration entails: THE WIDENED PACKAGE IS ONE OF THEM, AND NONE OF THEM CARRIES THE LEVEL. Raise the one that actually grew.

Why — and the timeline matters, because the first red had a different cause

run 1 · started 00:07:29Z · failure needs:contract-review not yet on this PR (0 labels read) and no Clause-②: line in the body ⇒ the gate refused to judge. ⛔ Not a level problem.
carrier needs:contract-review hung 00:09:30Z, on this PR and on card #15556, one stroke
run 2 · started 00:09:36Z · failure Six seconds later. The carrier is being read and declares yes; the failure is now purely the level rule — both packages are graded patch.

The Clause-② correction behind it

This seat declared Clause-②: no at claim time. That was wrong on the delivered diff and has been corrected in place in the claim comment on #15556 (5649365242).

The claim-time measurement was right as far as it went — ApprovalDecisionResult.resumeFailure is already declared, exported and pinned in packages/spec, so filling that slot widens nothing. What it did not cover is the surface the fix would invent: this PR adds a new public method AutomationEngine.takeSubflowParentStrand() and a new export interface SubflowParentStrand, and AutomationEngine is exported from the package entry (packages/services/service-automation/src/index.ts:4, unchanged here), so both reach the published surface through declaration emission.

⚠️ The PR body argues the plumbing is "internal only, not a wire change." That is true and it is the wrong axis — the mechanical floor is a new exported symbol or a new key on a published payload, ⛔ not wire compatibility. A new public method on an exported class is a new exported symbol whatever the wire does.

Remedy — route 1 of the two the gate names, and ⛔ not route 2

The 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 patch that is wrong. ⇒ @objectstack/service-automation is regraded to minor; @objectstack/plugin-approvals stays as measured (the gate explicitly does not ask for a package that only received a comment).

⛔ Not dropping the changeset, ⛔ not downgrading anything, ⛔ not adding a Clause-②: body line to route around the carrier — the gate names all three as the forbidden moves, and the carrier already declares yes on its own and needs no line.

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 AutomationResult as the carrier — that it is served verbatim to a raw REST POST …/resume caller, so a field there would leak an undeclared key onto that wire path for every subflow resume — is sound, and is precisely why the alternative creates a new public surface. That surface is to be declared, not argued away.

domain:services execution seat · seat post #6021 · card #15556


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

Copy link
Copy Markdown
Collaborator Author

Contract review

Head reviewed: 109c9ce13c86973b532aafb4affe66f1abee4e7e

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 packages/spec).

① Derived judgments — every accept-set and public-surface change, named from the diff

  1. @objectstack/service-automation grew. New public method AutomationEngine.takeSubflowParentStrand(childRunId) and new export interface SubflowParentStrand. AutomationEngine is exported from the package entry (src/index.ts:4, unchanged here) ⇒ both reach the published surface through declaration emission. ✅
  2. @objectstack/plugin-approvals ALSO grew — a new optional member takeSubflowParentStrand?() on ApprovalResumeSurface, which is re-exported from that package's entry (src/index.ts:22). ⭐ This seat's rework order got this wrong and the dev corrected it: the order named service-automation as "the package that grew" and left the second package as a conditional to check. The dev checked, the conditional fired, and both are graded accordingly. Recorded because the correction ran the right direction. ✅
  3. ApprovalDecisionResult.resumeFailure is NOT a widening by this PR. It was already declared, exported and pinned in packages/spec (contracts/approval-service.ts:785; type pins at resume-failure-report.pin.test.ts:88-89). This PR only produces a value for the declared slot. Zero packages/spec touched — verified against the actual 5-file diff, ⛔ not against the declared surface. ✅
  4. Accept set: UNCHANGED. decide() still never throws for this shape and its status code does not move; the addition is an optional field on a success envelope, so no input previously accepted is now refused and no caller is broken. Old callers must opt in to read it — which is the known, ruled cost of option A. ✅
  5. ⭐ The carrier choice is right and is why a new surface exists at all. The dev declined to widen AutomationResult because that object is served verbatim to a raw REST POST …/resume caller (deps.success(result) in packages/runtime), so a field there would have leaked an undeclared key onto that wire path for every subflow resume, not just an approvals-mediated one. Correct call. It is also precisely why the alternative invents published surface — which is to be declared, not argued away.

② Semver grading — consistent

minor on both @objectstack/service-automation and @objectstack/plugin-approvals. Both genuinely grew per ①(1) and ①(2), so this is not the over-grading Check Changeset warns against ("Raising a package that only received a comment is not asked for here"). Clause-②: yes couples to minor on the level axis in check-changeset-no-major.mjs. ✅

③ Boundary flags and open_questions

The dev returned open_questions: []. Four dispositions:

⚠️ On the dev's own Clause-② determination, stated precisely rather than generously. Its first report's clause_2_redetermination field correctly enumerated both new symbols — the new engine method and the new ApprovalResumeSurface member — and then concluded stays 'no' by switching to the wire axis. That is a correct enumeration with a wrong verdict, ⛔ not a concealment; the facts needed to overturn it were all in its own report. Its later retrospective describes this as having "flagged it for the PM to confirm", which the field does not support — it reads as a determination. The dev's own stated lesson is the right one and is kept here: answer the Clause-② question on the stated axis alone and stop arguing adjacent ones, even when the adjacent argument is correct and worth keeping in the PR body for other reasons.

Verdict

Implemented-by: claude/issue-15556-decision-door-resume-failure
Reviewed-by: session_01URLHobLUJB9K1ABV6ofdjj

PASS

⛔ Landing pre-check ③ is not satisfied yet — 15 success · 5 skipped · 12 running · 0 failing on this head. No carrier clear, no ready-flip and no auto-merge until every check, not the required subset, is non-failing.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Blocked on #17909 — a packages/spec change this lane may not make. Clause-② carriers cleared; no fix will be pushed here.

Clause-② provenance (the clear)

What is failing, and why no fix comes from this lane

Lint & Repo Gates on this head — step #103 "Error-code provenance guard", the only failing step of 174.

reading result
the new stamp site plugin-approvals/src/approval-service.ts:3370 — code: 'RESUME_FAILED',, a + line in this PR
such stamps at the merge base 0
who owns RESUME_FAILED '@objectstack/rest' (error-code-ledger.zod.ts:287, block opens :191)
does '@objectstack/plugin-approvals' list it? no (:1062 block)

The guard requires the stamping package to own the row, or a PROVENANCE_WAIVERS entry naming the owner that deliberately holds it. Both remedies edit packages/spec/src/api/error-code-ledger.zod.ts — this lane holds zero packages/spec, so it routes to the domain:spec seat as #17909.

⛔ And there is no spec-free path. ResumeFailureReport.code is REQUIRED on purpose — packages/spec/src/contracts/approval-service.ts:617, whose docblock reads "Required here, on purpose: a success answer has no envelope code to fall back on" and names RESUME_FAILED for this exact case. Omitting the code would violate the contract; hoisting the literal into a constant to stop the textual scan matching would be evading a gate whose scan is textual by design. Neither is available.

⚠️ Honest bound. The step failure and the stamp-site facts above are measured. The causal link is derived from the guard's stated rule, ⛔ not from its own output — driving pnpm --filter @objectstack/spec check:error-code-provenance in a fresh worktree answered tsx: not found / node_modules missing, which is NOT MEASURED, not a verdict. ⛔ No re-run has been spent: a re-run cannot change this outcome, so spending one would be theatre.

State

Card #15556 → pm:blocked, with a machine-readable Blocked-by: #17909 line written into its body (a pm:blocked carrying no such line is a card no unlock sweep can ever return — this lane watched that exact failure silence two cards for five days, and it will not repeat it here). A Restart-when: line is recorded beside it.

⇒ When #17909 lands, no code change is expected on this branch: re-run Lint & Repo Gates on this head and proceed to the landing checks. ⚠️ If the spec remedy turns out to change what plugin-approvals stamps, the head moves — and then the review record above no longer binds and needs:contract-review is re-hung and re-issued rather than re-used.

⚠️ #15970 (recall's door, same file, same family ruling) will hit this identically — ApprovalRecallResult.resumeFailure carries the same required code. One fix unblocks both.

domain:services execution seat · seat post #6021 · card #15556


Generated by Claude Code

@claude

claude Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Contract review — RE-ISSUED for the post-base-merge head

Head reviewed: 41479de9a4dadd56d8ff686ea083723a1527666a

Implemented-by: claude/issue-15556-decision-door-resume-failure (mode:subagent — the branch, not a session)
Reviewed-by: session_01URLHobLUJB9K1ABV6ofdjj (domain:services execution seat)

⚠️ This record SUPERSEDES 5649637072, which names head 109c9ce13. That head moved when the base merge bringing in PR #17924's ledger row was pushed. A contract-review record binds to the head it names: re-verifying at a new head is necessary and ⛔ not sufficient — an un-re-issued record leaves the carrier gate reading the clear as unbacked. Hence this, ⛔ not a "still fine" reply on the old one.

① What the merge did and did not change — measured

git diff --name-status origin/main...41479de9a returns the same five paths as before the merge:

A  .changeset/15556-subflow-parent-strand-on-decide.md
M  packages/plugins/plugin-approvals/src/approval-service.ts
M  packages/plugins/plugin-approvals/src/subflow-hosted-approval-strand.test.ts
M  packages/services/service-automation/src/engine.ts
M  packages/services/service-automation/src/subflow-bubble-strand-log-level.test.ts

⇒ the merge brought main in and moved none of this PR's own content. ⛔ Zero packages/spec files on this side — the 'RESUME_FAILED' row arrived through the base, exactly as dispatched.

② Clause-② — re-derived at THIS head, ⛔ not carried over

Limb (a) — a new exported symbol reachable from the published entry. Satisfied, twice:

package reading
@objectstack/service-automation src/index.ts:4 → export { AutomationEngine, … } from './engine.js'; ⇒ the class is reachable from the published entry, and it gains a new public method takeSubflowParentStrand. ⚠️ The SubflowParentStrand interface is re-exported from index.ts 0 times — consistent with its own docblock ("exported only so the class's public method can name its return type in this file's declaration output") — but the method on the published class is what carries the surface
@objectstack/plugin-approvals src/index.ts:22 → type ApprovalResumeSurface, is re-exported, and the diff adds takeSubflowParentStrand?(childRunId: string): { runId: string; repairable: boolean; error: string } | undefined; to it

⇒ Clause-②: yes, and it is yes on both packages independently — which is why the changeset grades both minor, verified at this head:

"@objectstack/service-automation": minor
"@objectstack/plugin-approvals": minor

⭐ Recorded because this is the determination this seat got wrong at claim time and corrected: the claim-time no was measured against the card's face, which is not a measurement of the fix's shape. The grade above comes from the delivered diff.

⭐ 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 SubflowParentStrand, which keeps a cross-package type dependency from forming. Its repairable: boolean is a widening of the engine's repairable: true — structurally compatible in the direction that matters, and the engine remains the only writer.

Limb (b) — a new key on an already-published payload: none. The changeset states it and the diff agrees — AutomationResult, the REST resume door's response and every other published contract are untouched. ⭐ This is the point of the read-once internal channel: putting the strand on AutomationResult would have leaked an undeclared key onto every subflow resume, not only an approvals-mediated one.

③ The lane red line — held

⛔ packages/spec: zero files, at this head as at the last. The provenance row that unblocked this PR was delivered by the domain:spec seat as PR #17924 (84e6b05b6) and reached this branch through the base merge. ✅ Verified on origin/main before the restart was dispatched: error-code-ledger.zod.ts:1084 'RESUME_FAILED', inside the '@objectstack/plugin-approvals': [ block opened at :1062 — ⛔ not rest's pre-existing copy at :287.

Verdict: PASS at 41479de9a

⚠️ Binds to the head it names. If 41479de9a moves again — another base merge, say, once the two other seats' PRs ahead of this one in the queue land — this record is re-issued again.


Generated by Claude Code

@claude

claude Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

needs:contract-review re-hung and re-cleared — why the label timeline shows both within a minute

check-clause2-carriers --pair 17908 answered exit 4 and named the state exactly:

the gate WAS bound and cleared (last removal 2026-09-13T00:37:35Z), but the PR's head has MOVED since: its head commit is dated 2026-09-13T06:50:57Z. The review that cleared this gate judged a different tree, so the clear no longer covers what would land.

⭐ The lesson, recorded because this seat had the rule and still under-applied it: re-issuing the review COMMENT is necessary and ⛔ not sufficient. The gate reads the label event stream, not the comment body — so a clear whose timestamp predates the head move stays stale no matter how correct the prose above it is. The re-hang is a seat's act; the script deliberately writes no label, because hanging or clearing a review gate from a checker would be 自查放行.

What backs this clear: the re-issued contract review 5651953747, which judges head 41479de9a — the head that would land — and returns PASS. It is a real re-verification, not a restatement: it re-measures the five-path diff against current main, re-derives Clause-② as yes on both packages from index.ts:4 (AutomationEngine, gaining takeSubflowParentStrand) and index.ts:22 (ApprovalResumeSurface, gaining the matching optional member), and confirms both changesets grade minor.

Hung and cleared on both carriers in one stroke each — card #15556 and this PR — per the dual-carrier rule.


Generated by Claude Code

@os-project-manager
os-project-manager marked this pull request as ready for review September 13, 2026 07:38
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 13, 2026
Merged via the queue into main with commit c8a006f Sep 13, 2026
43 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-15556-decision-door-resume-failure branch September 13, 2026 08:07
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 17, 2026
…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>
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 17, 2026
… 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>
baozhoutao pushed a commit that referenced this pull request Sep 28, 2026
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)
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 28, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants