fix(automation): resume gate follows map: too; the route stops accepting engine-internal variables (#3853) - #3860
Merged
Conversation
…pting engine-internal variables (#3853) Two holes in the #3801 gate, both demonstrated with a repro before fixing. 1. The chain walk missed `map:`. `resumeInternal` handles the two linked-run correlations oppositely — `subflow:` DELEGATES the signal to the child, `map:` RE-RUNS the map node — and the gate followed only the first. A run parked on a `map` node was therefore judged on `map` itself (resumeAuthority 'any') and let through even while the item it was waiting on sat on an `approval`. `map` is the batch-approval shape and the map parent's run id is the one a launcher holds. Since `$mapState.started` is advanced past the in-flight item before the suspend, and the re-entry records a result only when `$mapItemDone` is set, an empty-body resume of the parent skipped that item's approval outright and orphaned its still-pending request; a later real decision then bubbled into a parent already waiting on the next item, cascading the misalignment. The walk now follows both prefixes. The unifying rule: a linked-run pause is waiting on a CHILD, so the child's node carries the authority — read the item, not the loop. They differ only in what continuing would mean (lands on the child vs. advances past it), which the refusal message now says. 2. Resume `inputs` could write the engine's `$` namespace. They are applied as bare flow variables, so a caller could set the exact handoff keys bubbleToParent uses (`<nodeId>.$mapItemDone` / `$mapItemOutput`) and have the map record a per-item result for a decision nobody made — the node id is readable from GET /automation/:name. The same reached `$runId`, which approval/wait nodes use to correlate external state back to a run. The route now answers 400 for a reserved name (`$…` or a `.$` segment). Enforced at the transport, not in the engine, so the in-process bubble keeps working — the same trust split the gate itself uses. Refuse rather than silently strip, so a mis-authored screen input fails at the door. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013gvN32u1EiuvY9uQEMJiMR
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 23 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
July 28, 2026 12:50
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3853. Follow-up to #3822 — two holes in the gate that PR shipped, both demonstrated with a repro before fixing, not reasoned.
1. The chain walk missed
map:resumeInternalhandles the two linked-run correlations oppositely (engine.ts:2080):The gate's chain walk followed only
subflow:. A run parked on amapnode was therefore judged onmapitself —resumeAuthority: 'any'— and let through even while the item it was waiting on sat on anapproval.mapis the batch-approval shape (ADR-0037 A2,showcase_release_signoff), and the map parent's run id is the one a launcher holds.$mapState.startedis advanced past the in-flight item before the suspend (map-node.ts:155-156), and the re-entry records a result only when$mapItemDoneis set (map-node.ts:113-118), so an empty-body resume of the parent skipped that item's approval outright:Knock-on: item 1's eventual real decision bubbles into a parent now waiting on item 2, so its output lands under the wrong index and the misalignment cascades.
Fix. The walk follows both prefixes. The unifying rule is that a linked-run pause is waiting on a child, so the child's node carries the authority — the gate reads the item, not the loop. They differ only in what continuing would mean (lands on the child vs. advances past it), which the refusal message now distinguishes.
2. Resume
inputscould write the engine's$namespacesignal.variablesare applied as bare flow variables, so a caller could set the exact handoff keysbubbleToParentuses — with the node id readable fromGET /automation/:name(an SDK route) — and have the map record a per-item result for a decision nobody made:POST .../runs/{parentRunId}/resume { "inputs": { "signoffs.$mapItemDone": true, "signoffs.$mapItemOutput": { "result": "FORGED — never approved" } } } → { started: 2, results: [ { "result": "FORGED — never approved" } ] }The same reached
$runId, whichapproval/waitnodes read to correlate external state back to a run.$is the engine's namespace throughout ($runId,$flowName,$flowLabel,$record,$error,$parentRunId,$parentMapNode,$parentOutputVariable,<nodeId>.$mapState); authors never write there.Fix. The route answers 400 for a reserved name (
$…or a.$segment). Deliberately at the transport and not in the engine:bubbleToParentlegitimately writes those keys in-process, and this is the same trust split the gate itself uses — strict at the untrusted boundary, unrestricted for code that already holds the authority. Refuse rather than silently strip, so a mis-authored screen input fails at the door instead of many nodes downstream.Note (2) is not fixed by (1): a map whose pending item sits on an ungated
screen/waitis still resumable, and the handoff keys were still writable there.Consumer impact
Author-declared variables are unaffected —
{ new_assignee: 'ada' }and dotted names likecollect.notestill pass (covered by a test). If you were driving a batch-approvalmapby resuming the map's own run id, resume the item's run through its owning service instead (client.approvals.approve) — the map advances itself when the item completes.Tests
resume-authority-gate.test.tsgrows athrough a map pauseblock (14 total): the map parent is refused while an item is gated and nothing is consumed ($mapStateunchanged, both runs still suspended); the item completing through its owning service still advances the map and runs the item's downstream (no regression to the bubble); a map over ungated items stays resumable.http-dispatcher.test.tscovers the 400 for the three reserved shapes plus the "ordinary inputs still pass" control, and asserts the engine is never reached on refusal.Verified:
service-automation(35 files),plugin-approvals(12 files),runtime(47 files / 696 tests) all green;eslint --no-inline-configclean;check:doc-authoringandcheck:docsin sync.Docs: the
flows.mdxgate section now describes the linked-run walk in both directions plus the$-namespace rule; the batch-approval section says the map's own run id is refused while an item is open; ADR-0019 gains a#3853addendum and the#3801addendum's chain-walk bullet is corrected.Distinct from #3823 (revise-window
wait), which needs a per-suspension owner claim rather than a chain walk.Generated by Claude Code