fix(automation,approvals): gate the generic run-resume route on the suspended node (#3801) - #3822
Merged
Merged
Conversation
…uspended node (#3801) `POST /api/v1/automation/:name/runs/:runId/resume` forwarded a caller-supplied `{ inputs, output, branchLabel }` straight into `AutomationEngine.resume`, and `resumeInternal` validated machine state only — the concurrent-resume latch, the run exists, the flow exists, the suspended node still exists. Nothing asked who. Approval nodes suspend and resume through exactly that mechanism, so a resume carrying `branchLabel: 'approve'` walked the approve edge with no approver check, no `sys_approval_action` row and no status mirror — leaving the request row and the run permanently disagreeing. The only thing between the route and the approvals rules was convention, spelled out in a showcase comment. Removing the route is not the fix: it is load-bearing for screen flows. So the gate keys on what the run is parked on: - `ActionDescriptor.resumeAuthority` ('any' | 'service', default 'any') — a pausing node declares who may continue it; `approval` declares 'service'. - A suspension records the node type that produced it (`SuspendedRun.nodeType` / `sys_automation_run.node_type`), captured at suspend time so a flow republished mid-pause cannot re-type the node out from under the gate; older rows fall back to the flow definition. A deprecated ADR-0018 alias resolves to its canonical type, so renaming is not an escape hatch. - The engine refuses a 'service' suspension unless the signal carries `RESUME_AUTHORITY_SERVICE` — a symbol, so a JSON body can never mint it. `ApprovalService` stamps it in one place, on the tail of a decision it has already authorized and recorded. - The gate follows a subflow pause down to the child the signal would actually reach, so resuming the parent is no way around it. - Refusal returns `{ success: false, code: 'forbidden' }` and the route answers 403. Nothing is consumed: the request stays pending and the run stays parked, so the real decision still lands. Engine-internal continuations (subflow delegation/up-bubble, `map` re-entry, wait-timer wake) go through the private `resumeInternal` and are not re-gated. `screen` and `wait` pauses are unchanged. Known adjacent gap, deliberately out of scope: ADR-0044's revise window parks the run on an ordinary author-placed `wait` node, which a type-keyed gate cannot tell from any other wait. Recorded in the ADR addendum and filed separately. 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 5 package(s): 116 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…t advisory (#3801) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013gvN32u1EiuvY9uQEMJiMR
os-zhuang
marked this pull request as ready for review
July 28, 2026 08:20
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 #3801.
The hole
POST /api/v1/automation/:name/runs/:runId/resumeforwards a caller-supplied{ inputs, output, branchLabel }straight intoAutomationEngine.resume, andresumeInternalvalidated machine state only — the concurrent-resume latch, the run exists, the flow exists, the suspended node still exists. Nothing asked who.Approval nodes suspend and resume through exactly that mechanism, so a resume carrying
branchLabel: 'approve'walked the approve edge with no approver check, nosys_approval_actionrow and no status mirror — thesys_approval_requestrow and the run then disagreed permanently. The only thing standing between the route and the approvals rules was convention, spelled out in a showcase comment; a comment in an example is not an access control.Why not "remove the route"
It is load-bearing for screen flows — the UI flow-runner posts
{ inputs }there to advance a pausedscreennode. So the gate discriminates by what the run is parked on, not by the route.The gate
ActionDescriptor.resumeAuthority('any'|'service', default'any') — a pausing node declares who may continue it.approvaldeclares'service';screen/waitkeep the default and behave exactly as before.SuspendedRun.nodeType/sys_automation_run.node_type, captured at suspend time rather than re-derived from the live flow, so a flow republished mid-pause cannot re-type the node out from under the gate. Rows written before this fall back to the flow definition. A deprecated ADR-0018 alias resolves throughaliasOfto its canonical type, so renaming is not an escape hatch either.RESUME_AUTHORITY_SERVICE(@objectstack/spec/contracts) is stamped on theResumeSignalby the owning service. The transport builds its signal out of a JSON body and no JSON body can produce a symbol-keyed property, so the route cannot forge it — the gate never has to trust a caller's claim about itself.ApprovalServicestamps it in one place (serviceResume), on the tail of a decision it has already authorized and recorded, so a future outcome path cannot quietly ship a resume the gate then rejects at runtime.subflownode delegates the signal to its suspended child, so the gate resolves the effective suspension first and judges the node the signal actually lands on. Resuming the parent is not a way around it.resumereturns{ success: false, code: 'forbidden' }; the route answers 403 rather than a 200 carryingsuccess: false(which reads as "your resume ran and the flow failed"). Nothing is consumed — the request stays pending and the run stays parked, so the real decision still lands.Engine-internal continuations (subflow delegation and up-bubble,
mapre-entry, the wait-timer wake) go through the privateresumeInternaland are not re-gated — they continue work an already-authorized call started.Consumer impact
client.automation.resume(flow, runId, { branchLabel: 'approve' })to finish an approval → TO:client.approvals.approve(requestId, …)/.reject/.recall. The old call now answers 403 and changes nothing.resumeAuthority: 'service'on its descriptor and stampRESUME_AUTHORITY_SERVICEfrom that service.Changeset carries the same FROM → TO mapping.
Known adjacent gap (deliberately out of scope)
ADR-0044's revise window parks the run on an ordinary author-placed
waitnode (signal flavor), which a type-keyed gate cannot distinguish from any other wait — a raw resume there still forces a resubmit without asys_approval_actionrow. Recorded in the ADR-0019 addendum; closing it needs a per-suspension owner claim rather than a node-type one, so it is filed separately rather than widening this PR.Tests
New
packages/services/service-automation/src/resume-authority-gate.test.ts(11): refusal leaves the run parked and downstream unrun; the marker lets the owner through; ungated (screen/wait) and descriptor-less pauses unchanged; the tag is recorded; the gate survives a restart via the durable store and falls back for pre-tag rows; a deprecated alias of a gated type is still gated; machine-state errors (unknown run) are still machine-state errors; subflow — resuming the parent over a gated child is refused with neither run consumed, while an ungated child still delegates.Plus: end-to-end in
plugin-approvals(real engine + realApprovalService) proving a raw resume leaves the requestpendingwith only thesubmitaction recorded and the later real decision still landing; the 403 mapping (and the non-403 for an ordinary failed resume) inhttp-dispatcher.test.ts;resumeAuthoritydefault/parse in the spec.Verified:
spec(6739),service-automation(381),plugin-approvals(279),runtime(663) all green; eslint clean;check:docs/check:api-surface/check:spec-changes/check:skill-refs/check:skill-docs/check:upgrade-guide/check:skill-examples/ downstream-contract typecheck all in sync (reference docs + api-surface snapshot regenerated, not hand-edited).Generated by Claude Code