Skip to content

fix(service-automation): on the synchronous path, a child run that refuses stops its parent, in subflow and in map alike - #18706

Merged
huangyiirene merged 6 commits into
mainfrom
claude/issue-18110-subflow-refused-rollup
Sep 17, 2026
Merged

huangyiirene merged 6 commits into
mainfrom
claude/issue-18110-subflow-refused-rollup

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Part of #18110
Part of #18555

⛔ Deliberately Part of, not Fixes, for both: the ruling on #18110 (director batch #145 item 4, letter A, maintainer 「同意,其他也同意」) states that the lane seat closes both cards at landing, after verifying the two arms independently.

The defect

A child flow that ends on an end node declaring outcome: 'refused' rolled up to its parent as an ordinary success.

subflow-node.ts branched only on child.status === 'paused' and !child.success. A refused child is neither — finishRefusedRun answers { success: true, status: 'refused' }, because a refusal is a successful evaluation that says no — so it fell straight through the ordinary success exit. The parent walked the node's out-edges, recorded completed, and fired its own successMessage over the child's refusal.

map-node.ts had the line-for-line identical branch set and the identical missing arm (#18555): a refusing row's output went into state.results and every row after it was processed anyway.

This is fail-open in the direction nobody notices. A refusing end is most often a gate — an approval, an eligibility check, a precondition — and a gate whose "no" lets the run through finishes green.

The change — one channel, two call sites

New on NodeExecutionResult refuse?: boolean and refusalMessage?: string, beside the suspend?: boolean that already exists for the pause half of the same unwinding protocol.
executeNode throws new FlowRefusalSignal(node.id, result.refusalMessage) at the position the suspend signal is thrown — after the success step is pushed, after the childSteps fold, after output write-back.
subflow-node.ts / map-node.ts both gain the refused arm that sets it.
Region-boundary diagnostic generalised to name whichever node carried the refusal. Text only.

The throw position is the design, not a convenience: it is what keeps the child's selected / acted / unmeasuredEffect rollup (#4354) in the run log and therefore in the run summary. A refusing child really can have written rows before it said no, and an unwind that began any earlier would drop exactly those counts. This is the property option B was rejected for losing, and it is pinned in both test files.

⛔ A refusal is still not a failure: it does not consume retry budget, is not routable by a fault edge, and is not counted in nodes[].failures.

What this change deliberately does not do

Clause-②: yes (widening)

NodeExecutionResult is barrel-exported from this package's single entry point (src/index.ts:9), so two new optional members are a widening of the published executor contract. Changeset: @objectstack/service-automation minor. Contract review is owed on the review, per the ruling. Additive for third parties: an executor that never sets refuse behaves exactly as before.

Premise re-verification (every position re-taken, ⛔ none inherited)

engine.ts moved the same day the ruling was written (99fcb4a, 2026-09-17T11:57:43Z), so every line number handed to me was stale. Re-taken with git show origin/main:PATH at 1bc22b3:

reading ruling / prior report said re-taken verdict
NodeExecutionResult :332 :332 unchanged
TERMINAL_RUN_STATUSES :1365 :1365 unchanged — refused already published
the single new FlowRefusalSignal :9319 :9352 moved, shape intact, still the only site, still node.type === 'end'
the suspend throw :9611 :9648 moved; still after the success step, the childSteps fold and output write-back — the ruling's prescribed position exists as described
the region boundary :9956 :9991 moved
NodeExecutionResult barrel-exported src/index.ts:9 confirmed clause ② yes holds
map-node.ts branch set :191 / :207, refused 0 hits confirmed, control child = 31 hits identical hole

FlowRefusalSignal is still not exported and this change does not export it — callers still see status: 'refused'.

Verification

  • Both arms pinned separately, so deleting either one fails a test by itself:
    • src/builtin/subflow-refused-rollup.test.ts — 6 tests
    • src/builtin/map-refused-rollup.test.ts — 7 tests
  • Ablation, both legs, each proven on disk (HEAD blob hash vs mutated blob hash, anchor counts before and after) and each restored from HEAD with git diff HEAD empty:
    • ablating only the subflow arm ⇒ 5 failed in subflow-refused-rollup.test.ts, map-refused-rollup.test.ts entirely green;
    • ablating only the map arm ⇒ 6 failed in map-refused-rollup.test.ts, subflow-refused-rollup.test.ts entirely green.
    • The ablation also corrected the pins: both #4354 rollup legs were originally green under ablation, because the totals are equally true of the unfixed engine, which rolled the same metrics up and then carried on. They now assert status === 'refused' in the same test.
  • Controls, mandatory and green on both sides: a non-refused child still completes the parent, fires the parent's successMessage, walks its out-edges, and rolls up identical totals — on both the subflow and the map path.
  • pnpm --filter @objectstack/service-automation test138 files / 1652 tests passed, re-run after merging origin/main.
  • pnpm --filter @objectstack/service-automation typecheck — exit 0, test layer included (0 files / 0 errors in the debt ledger).
  • Gate roster derived from merge-base, not from a predicted path list: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (no paths). 60 families derived, re-derived after the merge with zero delta. Reconciled with --ran: 60 accounted for, 58 run green, 2 NOT MEASURED.
    • pnpm check:dual-build-cjs-loads and pnpm check:type-check-debt both exited 3 — PREREQUISITE NOT MET, each printing "this is NOT a pass: nothing was measured". Both require a whole-repo build; that is CI's run, not this PR's. ⛔ Not recorded as a failed measurement.
    • node scripts/check-plugin-teardown-shape.mjs --self-test first exited 3 because its positive control is pinned to a commit outside this shallow checkout; after git fetch --deepen it exits 0 with 48 cases.
  • pnpm lint (eslint . --no-inline-config) — full repo, exit 0. ⛔ No narrowing claimed; the whole scan ran.

Acceptance notes — found in passing, ⛔ not fixed here

  1. A child that PAUSES and then refuses on resume still rolls up as a success, and worse, may strand its parent. This PR closes the synchronous path only. On the delegation path resumeInternal reads childRes.status === 'paused', then failure, then treats everything else as "child completed" — a refused child (success: true) falls through there exactly as it used to here. And on the up-bubble path the refusal arm returns finishRefusedRun from the catch before bubbleToParent is reached on the completion path, so a child that refuses after a pause never wakes its parent at all. Read from source, ⛔ not driven. This is a separate mechanism (resume / bubble machinery), outside the ruling's prescription, and it is reported for the seat to file rather than ridden in. Dedupe words: paused child refused bubbleToParent · resumeInternal childRes status refused · subflow delegation refused child · map re-entry mapItemDone refused · parent stranded refused child.
  2. Noted, not filed — the definition of the refused run status ("the flow reached an end node declaring outcome: 'refused'") now under-describes its producers. It is written in packages/services/service-automation/src/sys-automation-run.object.ts:178 and, identically, in packages/spec (src/automation/execution.zod.ts, src/contracts/automation-service.ts), with a pin test and a generated reference page downstream. Correcting it needs a packages/spec edit, which this card's ruling puts out of bounds (clause 5: that routes to the domain:spec seat). Successor: the domain:spec seat already working this boundary on service-automation: a PAUSING map inside a contained region leaves its progress state behind — later loop iterations skip items and the exhausted map returns success having run nothing #15646 / PR feat(spec)!: a structured region body refuses a pause-capable node and an 'end' node #18688.
  3. Three hard-wired "an end node" attributions inside the files this PR already edits were generalised in place, because this change is what makes them false: the FlowRefusalSignal docblock, finishRefusedRun's header, and its durability error log line. Comments and one log string; no gate reads them.

Generated by Claude Code

…stead of rolling up as a success

`subflow-node.ts` branched only on `child.status === 'paused'` and
`!child.success`. A refused child is neither -- `finishRefusedRun` answers
`{ success: true, status: 'refused' }`, because a refusal is a successful
evaluation that says no -- so it fell through the ordinary success exit: the
parent walked the node's out-edges, recorded `completed` and fired its own
`successMessage` over the child's refusal. Fail-open, and finishing green.

Adds the executor-facing channel the fix needs, beside the one `suspend`
already uses:

- `NodeExecutionResult.refuse` / `.refusalMessage` -- a published widening of
  the barrel-exported executor contract, additive for executors that never set
  it.
- `executeNode` throws `FlowRefusalSignal` from the position the suspend signal
  is thrown: after the success step is pushed, after the `childSteps` fold and
  after output write-back, so the child's `selected` / `acted` /
  `unmeasuredEffect` rollup survives the refusal unwind.
- The region-boundary diagnostic names whichever node carried the refusal
  rather than asserting it was an `end` node. Text only; region semantics are
  untouched.

Claude-Session: https://claude.ai/code/session_01QGMBhvUoyD8t5zY8xHQhnP
Co-authored-by: Claude <noreply@anthropic.com>
… parent too

The identical branch set as `subflow-node.ts` and the identical missing arm:
`child.status === 'paused'` and `!child.success`, no `refused` arm. A refusing
row's output was pushed into `state.results`, the batch carried on to the next
row, and the parent recorded `completed` -- the worked "approve each row" shape
answering no on one row and approving every row after it.

Second call site of the one channel added with the subflow arm: the refusal
carries the batch's accumulated totals plus the refusing item's own, because a
child that refused did not fail and nothing counts its work twice. The progress
state is deliberately neither advanced nor deleted -- the run is terminating and
`started` is the resume program counter.

Claude-Session: https://claude.ai/code/session_01QGMBhvUoyD8t5zY8xHQhnP
Co-authored-by: Claude <noreply@anthropic.com>
…y are about

Without the `status === 'refused'` assertion in the same test, the totals are
equally true of the unfixed engine, which rolled the same metrics up and then
carried on. Found by ablating each arm: the rollup legs stayed green.

Claude-Session: https://claude.ai/code/session_01QGMBhvUoyD8t5zY8xHQhnP
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/automation/flows.mdx (via refusalMessage (symbol, a field of interface NodeExecutionResult))

2 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))

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
  • 3 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 — 6 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 f6189a43f9ba09516f142fd7aca8ae15583d6c9dpackageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json f6189a43f9ba09516f142fd7aca8ae15583d6c9d

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

huangyiirene commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

Clause-② contract review — PASS, at tier, tier confirmed from the transcript

Recorded by the domain:services seat (objectstack#6021), session_01QGMBhvUoyD8t5zY8xHQhnP, at 2026-09-17T16:24Z, for PR head af0edd84f. ⛔ This seat did not perform the review: it runs the default judgment tier, below CONTRACT_REVIEW_TIER. An isolated subagent did, and this comment is the record the enqueue gate reads.

🔬 Tier verification — the reviewer could not confirm its own tier, and was right to say so

The reviewer opened its report by refusing to claim at-tier status: get_session (session_id omitted) returns the calling CCR session's record — the default judgment tier — because there is no subagent-level session record, so it flagged its own Fable identity line as an unverifiable self-report and said the review 「must be treated as unverified-tier, not dressed up as at-tier」. ⛔ That is exactly the right disclosure and it is why the protocol specifies transcript verification rather than get_session.

Verified by this seat against the reviewer's own transcript, ⛔ not against its self-report:

grep -o '"model":"[^"]*"' <reviewer transcript>  |  sort | uniq -c
    148  "model":"CONTRACT_REVIEW_TIER"
  (non-fable model fields: ZERO)
transcript: 931,845 bytes · 265 lines

CONTRACT_REVIEW_TIER = CONTRACT_REVIEW_TIER (scripts/pm/dispatch-gates.mjs:10932, exact match, ⛔ never a family or prefix floor). ⇒ 148/148 fable, 0 otherwise ⇒ at tier. The review counts.

Verdict: PASS for clause-② — ⛔ no blocking defects

The reviewer read the diff itself (merge-base f8eaf6704), built and ran an isolated git archive snapshot, and left the shared repo untouched. Its load-bearing findings:

  • Exactly the ruled widening, nothing more. NodeExecutionResult.refuse? (engine.ts:434) and .refusalMessage? (:445), both optional; src/index.ts untouched ⇒ ⛔ no new export. FlowRefusalSignal stays unexported. ⛔ No union loosened, ⛔ no new run-status value (TERMINAL_RUN_STATUSES already held 'refused'), ⛔ zero packages/spec. Neither member leaks into a persisted structure.
  • ⛔ Nothing previously accepted is now refused. The failure arm, the #6667 guard and the suspend throw are untouched; the refuse check is a no-op when the flag is absent. Package suite at head: 138 files / 1652 tests green; tsc --noEmit and the test tsconfig both exit 0.
  • Throw position verified — and it is what preserves Surface flow run summaries (selected / acted / skipped) — a scheduled flow that does nothing is currently indistinguishable from one with nothing to do #4354. The refuse throw (9727–9729) sits after the success-step push, the childSteps fold and the output write-back, outside the executor try/catch, immediately before the suspend throw. ⭐ Reverse-verified: deleting only the throw fails the rollup tests in both files.
  • The two senses of refused cannot collide — guard refusals carry success: false / errorClass: 'guard' and exit before the refuse check, which is reachable only with success === true.
  • Region semantics untouched — only the string literal inside the existing if (isRefusalSignal(err)) changed; ⛔ no container taught to rethrow, ⛔ [Decision] service-automation: should a refusing end INSIDE a structured region propagate out and terminate the run, or stay a loud refusal at the region boundary? #18112 option B not implemented.
  • ⭐ Both arms independently pinned, and they fail without the fix. Replacing subflow-node.ts with merge-base ⇒ 5 subflow tests fail, all 7 map tests pass; replacing map-node.ts ⇒ 6 map tests fail, all 6 subflow tests pass. ⇒ each arm is pinned by exactly its own file, which is fold gate ④ discharged by measurement rather than by assertion.

Advisories — disposition

# disposition
A2 (significant) Acted on. The fix covers the synchronous engine.execute return only; a child that pauses then refuses on its resumed leg still rolls up fail-open (delegated resume: parent completed, downstream ran), and the up-bubble path leaves the parent paused forever in listSuspendedRuns(). ⛔ Neither is a regression and ⛔ neither is in this ruling's scope — but the changeset headline was broader than the fix, and a changeset becomes the release note. ⇒ patch round requested (narrow the sentence + a scope line, prose only, ⛔ no widening), and the uncovered leg is filed as #18714.
A1 Accepted, rides the same push: the refuse docblock says a result carrying both refuse and suspend refuses, without the caveat that a supportsPause: false descriptor trips the #6667 guard first and ends the run failed. Behaviour is fail-closed and correct; ⛔ only the docblock is incomplete, ⛔ no ordering change.
A3 Informational — the package sits in a 70-package fixed changeset group, so minor bumps the group. Repo convention, ⛔ not this PR's choice.
A4 Harmless — one map assertion pins the executor's early return rather than the engine throw; the other five pin the engine.

⛔ Not enqueued yet

needs:contract-review stays on all three carriers until the A1/A2 patch lands. The PASS is a judgement about the type surface, which a prose-only patch does not move — but the head will be re-read and CI re-checked before the label comes off and the PR goes ready → auto-merge. ⛔ No enqueue on an unverified head.


Generated by Claude Code

…path, and caveat the refuse/suspend precedence

Prose only -- zero code lines change, and the engine.ts diff is entirely
comment-continuation lines.

The changeset headline claimed the fix "stops its parent, in subflow and in map
alike" without qualification, and that becomes the release note. It is not true
on the RESUMED leg, which this change does not touch: a child that durably
pauses first and refuses only when resumed reaches its parent through the resume
machinery, which reads the child's outcome at seams that do not consult
`status: 'refused'`. Neither seam is a regression here -- both pre-date this
change -- but the resumed leg is the one a screen flow actually takes, so the
note now names the synchronous `engine.execute` return path and carries an
explicit scope paragraph.

The `refuse` docblock said a result carrying both `refuse` and `suspend`
"REFUSES", with no caveat. The #6667 undeclared-suspension guard runs ahead of
that and reads `suspend` alone, so a node type whose descriptor does not declare
`supportsPause: true` has its result replaced by a guard refusal and the run ends
`failed` -- the refusal is never read. That is the right end for a declaration
defect, so the ordering is untouched and only the docblock gained the caveat.

Claude-Session: https://claude.ai/code/session_01QGMBhvUoyD8t5zY8xHQhnP
Co-authored-by: Claude <noreply@anthropic.com>
@huangyiirene huangyiirene changed the title fix(service-automation): a child run that refuses stops its parent, in subflow and in map alike fix(service-automation): on the synchronous path, a child run that refuses stops its parent, in subflow and in map alike Sep 17, 2026
… in the changeset scope note

The scope paragraph pointed at an unnumbered follow-up because the card did not
exist when it was written. It does now: #18714, the resumed-leg fail-open and the
up-bubble path that strands the parent `paused`.

One line, one token. No rewording, no behaviour change.

Claude-Session: https://claude.ai/code/session_01QGMBhvUoyD8t5zY8xHQhnP
Co-authored-by: Claude <noreply@anthropic.com>

huangyiirene commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

Contract review — clause ②, PASS

Reviewed head: 62a1c19d1f3be5b4bef9de1b1d3a58300ff436fd

Reviewed-by: session_01QGMBhvUoyD8t5zY8xHQhnP (domain:services seat, objectstack#6021), adopting an isolated at-tier reviewer whose tier was confirmed from its transcript — 148/148 "model":"CONTRACT_REVIEW_TIER", zero non-fable model fields, against CONTRACT_REVIEW_TIER = 'CONTRACT_REVIEW_TIER' (scripts/pm/dispatch-gates.mjs:10932, exact match).

Written at 2026-09-17T16:59Z. ⚠️ This comment heals a half-state of this seat's own making, and the mechanical guard is what caught it — recorded rather than quietly repaired.

⛔ What I got wrong, and what the guard caught

I cleared needs:contract-review from all three carriers before a review of record existed in the shape the gate reads. My earlier write-up (issuecomment-5717759769) carries the full substance, but it is not a review of record by H51's measured shape: its heading is ## Clause-② contract review … rather than ## Contract review, it names the previously reviewed head af0edd84f, and it has no Reviewed-by: line.

check-clause2-carriers --pair 18706 refused both pairs with C6 / exit 4, verbatim: 「a cleared gate with nothing behind it, indistinguishable from never reviewing」. ⭐ That is exactly right, and it is the whole reason the check exists — a seat that has genuinely reviewed and a seat that has not look identical from the board once the label is gone. ⛔ Nothing was enqueued while the pair was illegible.

The review, on THIS head

The at-tier review was performed on head af0edd84f. This head is 62a1c19d1f. Re-derived by this seat before adopting the verdict forward, ⛔ not assumed:

git diff --stat af0edd84f..62a1c19d1
  .changeset/18110-subflow-map-refused-rollup.md     | 6 ++++--
  packages/services/service-automation/src/engine.ts | 12 ++++++++++++
  2 files changed, 16 insertions(+), 2 deletions(-)

non-comment, non-changeset changed lines under packages/  = 0   (every packages/ line is a comment)
added/removed lines touching `refuse?` / `refusalMessage?` / `export` = 0

⇒ the intervening pushes are prose only — a narrowed changeset headline plus a scope paragraph, the refuse docblock precedence caveat, and one follow-up card number. ⭐ Clause ② is a judgement about the type surface, and the type surface is byte-identical between the reviewed head and this one, so the PASS carries forward. ⛔ Had a single non-comment line moved, this would have been a re-review, not an adoption.

Verdict basis (full per-item findings in issuecomment-5717759769)

PASS — no blocking defects. Load-bearing points, all taken by the at-tier reviewer from the diff itself in an isolated snapshot, and reverse-verified:

  • The published surface gains exactly NodeExecutionResult.refuse? and .refusalMessage?, both optional; src/index.ts untouched ⇒ no new export; FlowRefusalSignal stays unexported; no union loosened; no new run-status value; zero packages/spec.
  • ⛔ Nothing previously accepted is now refused — the failure arm, the #6667 guard and the suspend throw are untouched, and the refuse check no-ops when the flag is absent.
  • The throw sits after the success-step push, the childSteps fold and the output write-back and outside the executor try/catch ⇒ the Surface flow run summaries (selected / acted / skipped) — a scheduled flow that does nothing is currently indistinguishable from one with nothing to do #4354 rollup survives on the refusal path. Reverse-verified: deleting only the throw reddens the rollup tests in both files.
  • Both arms independently pinned: reverting subflow-node.ts alone ⇒ 5 subflow tests red, all 7 map green; reverting map-node.ts alone ⇒ 6 map red, all 6 subflow green.
  • Region semantics untouched — only the string literal inside the existing if (isRefusalSignal(err)) moved.

Advisories A1 (docblock precedence caveat) and A2 (the changeset overclaimed against the resumed leg) were acted on in the prose rounds above; the uncovered resumed leg is filed as #18714. A3/A4 are informational.

CI on this head: 31 success · 3 skipped · 0 red · 0 pending; the three skips are in EXPECTED_SKIPS (hand-checked against the roster source — check-expected-skips.mjs cannot run in this container, and ⛔ a script that cannot run is NOT MEASURED).


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Contract review

Served-tier: 148/148 CONTRACT_REVIEW_TIER
Head-sha: 62a1c19d1f3be5b4bef9de1b1d3a58300ff436fd

① Derived judgments

Clause ② is yes: NodeExecutionResult is barrel-exported from this package's single entry point and gains two optional members, refuse? and refusalMessage?. That is the widening the governing ruling authorised, and the review's job was to confirm it is that widening and nothing more.

  • Published surface, enumerated: exactly those two members. src/index.ts is untouched by the diff ⇒ no new export. FlowRefusalSignal remains unexported. No union loosened, no constant made public, no new run-status value (TERMINAL_RUN_STATUSES already carried refused). Neither member leaks into a persisted structure.
  • No narrowing smuggled in: the failure arm, the #6667 undeclared-suspension guard and the suspend throw are untouched; the refuse check no-ops when the flag is absent; non-refused children take the unchanged path in both executors.
  • Throw position, the ruling's load-bearing detail: after the success-step push, after the childSteps fold and the output write-back, and outside the executor try/catch ⇒ the Surface flow run summaries (selected / acted / skipped) — a scheduled flow that does nothing is currently indistinguishable from one with nothing to do #4354 rollup (selected / acted / unmeasuredEffect) survives on the refusal path. Reverse-verified: deleting only the throw reddens the rollup tests in both files.
  • The two senses of refused cannot collide: guard refusals carry success: false / errorClass: 'guard' and exit through the failure arm before the refuse check, which is reachable only with success === true.
  • Both arms independently pinned: reverting subflow-node.ts alone ⇒ 5 subflow tests red, all 7 map green; reverting map-node.ts alone ⇒ 6 map red, all 6 subflow green.
  • Adoption forward to this head: the review ran on af0edd84f. Between that head and this one, git diff --stat is 2 files / +16 −2, non-comment non-changeset changed lines under packages/ = 0, and lines touching refuse? / refusalMessage? / export = 0. The type surface is byte-identical, so the verdict carries; a single non-comment line would have made this a re-review.

② Semver level

minor, on @objectstack/service-automation, as the ruling directed. Correct for an additive optional member on a published type: nothing previously accepted is refused and nothing is retired, so it is neither a patch nor a breaking change. The changeset states the level and the reason and names the two members.

③ Boundary flags

Implemented-by: claude/issue-18110-subflow-refused-rollup
Reviewed-by: session_01QGMBhvUoyD8t5zY8xHQhnP

VERDICT: PASS

Written at 2026-09-17T17:00Z. The Served-tier: stamp control is this seat's own grep of the harness-stamped served-model field across the reviewing round's transcript: 148 of 148 model fields matched the constant, zero did not — ⛔ read from the transcript, ⛔ never from the dispatch parameter, which is configuration and not a reading. ⚠️ This record supersedes two earlier write-ups of mine on this head that were not reviews of record: one was shaped wrongly and named the prior head, and both named a model identifier, which AGENTS.md forbids in any repository artifact. Both defects were caught by check-clause2-carriers --pair, ⛔ not by me.


Generated by Claude Code

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/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants