fix(automation): hold a signal-less resume to the screen-input contract (#13648) - #14388
Conversation
`AutomationEngine.refuseInvalidScreenInput` opened with a bare
`if (!signal) return null;`, so `resume(runId)` with no signal object
skipped the `required` screen-field check that `resume(runId, {})` is
held to. The engine already names its one legitimate exemption through
`ENGINE_BUILT_SIGNAL`; the early return was a second, unnamed spelling.
The public `resume` door now normalises an absent signal to `{}` (the
shape the HTTP route has always assembled for an empty body), and
`resumeInternal` / `refuseInvalidScreenInput` / `applyResumeSignal` take
a non-optional signal, so a falsy-signal branch cannot grow back. Pauses
that declare no input contract are unaffected.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
📓 Docs Drift CheckThis PR changes 1 package(s): ⛔ 2 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 3c073c20b0dbed1807aba1af65f85bf24ce515bc && git checkout 3c073c20b0dbed1807aba1af65f85bf24ce515bc
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1dcb995f23fc6f54c38d5e38068800e7513e14f0 8ed175039405c4f9409e676723c1ae6c4044f371 && git checkout -B drift-repro 1dcb995f23fc6f54c38d5e38068800e7513e14f0 && git merge --no-ff 8ed175039405c4f9409e676723c1ae6c4044f371
node scripts/docs-audit/affected-docs.mjs --json 1dcb995f23fc6f54c38d5e38068800e7513e14f0
|
|
Landing provenance (PM seat,
Generated by Claude Code |
Fixes #13648
What
AutomationEngine.refuseInvalidScreenInputopened with a bareif (!signal) return null;, soresume(runId)with no signal object skipped therequiredscreen-field check thatresume(runId, {})is held to (INVALID_SCREEN_INPUT) — the run proceeded with the screen's variables unbound. The engine already names its one legitimate exemption, its own continuations, through theENGINE_BUILT_SIGNALflag; the early return was a second, unnamed spelling of an exemption nobody had asked for.Ruling applied (triage comment on #13648, quoted verbatim, untranslated):
Route: the public
resumedoor normalises an absent signal to{}— the shape the HTTP routePOST /automation/:name/runs/:runId/resumehas always assembled for an empty body — andresumeInternal,refuseInvalidScreenInputandapplyResumeSignalnow take a non-optionalResumeSignal, so both falsy-signal early returns are deleted and a falsy-signal branch cannot grow back. The only exemption left is the engine-built flag, spelled once, at the one place the engine builds a signal (engineBuilt(...)).Unaffected by construction — any pause that declares no screen contract:
wait(its timer wake and restart re-arm callengine.resume(runId)with no signal and keep working,wait-node.test.tsgreen),approval, message-only and object-form screens, and screens whose fields are all optional or hidden byvisibleWhen.Premise (the triage stop condition) — HELD
Sweep of every
resume(caller on the engine /IAutomationServiceacrosspackages/**,examples/**,apps/**(git grep, non-test sources; the positive control is the signal-carrying REST call, which the same sweep hit):packages/runtime/src/domains/automation.ts—automationService.resume(parts[2], signal)(thePOST …/runs/:runId/resumehandler;client.automation.resumeand the hono adapter route both land here)signal = {}field by field, so an empty body is{}packages/plugins/plugin-approvals/src/approval-service.ts—this.automation.resume(runId, signal)viaserviceResumeoutput/branchLabelplus the service-authority marker)packages/services/service-automation/src/builtin/wait-node.tslines 118 and 401 —engine.resume(runId)waitpause, which declares no screen contract ⇒ unaffectedengine.tsresumeInternal(childRunId, signal, true)(subflow delegation) andresumeInternal(parentRunId, sig, …)(bubbleToParent)packages/mcp,packages/rest,packages/plugins/plugin-hono-server,packages/client*resumecaller (the client and the hono route go through the runtime handler above)examples/**,apps/**examples/app-showcase/src/automation/flows/index.ts)The only documented signal-less gesture is the wait node's operator note ("resume it externally via resume(runId)"), and that pause has no screen contract, so its behaviour is unchanged. No external caller relies on a signal-less resume of a screen-paused run ⇒
premise_still_valid: true, no fork.PM hypotheses
refuseInvalidScreenInputhas exactly one call site (resumeInternal, beforeapplyResumeSignal); nothing else read itsnull-for-missing-signal answer. The subflow-delegation branch may replace the caller's signal with an engine-built one before that call, which is the flag path, not the bare one.engineBuilt(...)), and the subflow /mapcontinuations build flagged signals. But the wait node's timer and re-arm paths build NO signal at all — they callengine.resume(runId). They are unaffected for a different reason: awaitpause has noscreen, soscreenDeclaresInputContractis false and an empty submission is conformant. Pinned by the package suite (wait-node.test.ts, 1166/1166 green).git show 80948340 -- engine.tshas hunks at old lines 1622, 1788, 1831, 4521 (loadSuspendedRunStrict,cacheOnlySuspensions, prose); zero mentions ofrefuseInvalidScreenInputorENGINE_BUILT_SIGNAL.Tests (head
8ed17503)New
packages/services/service-automation/src/builtin/screen-resume-signal-less.test.ts(9 pins):requiredfield ⇒success: false,code: 'INVALID_SCREEN_INPUT', first sentenceInvalid screen input: …"kind"… required, and the run is still paused (hasSuspendedRuntrue,getSuspendedScreenstillask); plus: the signal-less and the{ variables: {} }refusals are byte-equal envelopes; plus: a signal-less resume of a delegated CHILD is refused and both runs stay paused.success: true, no code, pause consumed).visibleWhen: 'false'.Runs (all through
scripts/pm/os-verify-lock.sh, exit captured before any pipe, verdict lines quoted):pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2 src/builtin/screen-resume-signal-less.test.ts src/builtin/screen-resume-validation.test.ts→Test Files 2 passed (2) · Tests 24 passed (24)·VERDICT command-exit 0pnpm --filter @objectstack/service-automation test→Test Files 98 passed (98) · Tests 1166 passed (1166)·VERDICT command-exit 0— every existing signal-less caller in the suite pauses on a node with no screen contract; no fixture needed re-triage.pnpm --filter '@objectstack/plugin-approvals^...' buildthenpnpm --filter @objectstack/plugin-approvals test(downstream consumer; resolves@objectstack/service-automationthrough its rebuiltdist) →Test Files 35 passed (35) · Tests 652 passed (652)·VERDICT command-exit 0. (A first run before the closure build had one file fail to resolve@objectstack/trigger-record-change— build state, not this change; 650/650 tests already green.)pnpm --filter @objectstack/service-automation exec tsc --noEmit -p tsconfig.json --listFiles→ the new test file andengine.tsare in the program (1 hit each); zero errors in either. The package has notypecheckscript (DEBT ledger); the only 3 errors reported are pre-existing insrc/nested-region-parity.test.ts(privateflows), untouched here.pnpm exec eslint . --no-inline-config --format json(full repo, foreground) → 5681 files, 0 errors, 0 warnings; both changed files in the population.pnpm --filter @objectstack/service-automation build→check-dts-emitted: 2/2;dist/index.jscarriessignal ?? {}(1) and noif (!signal) return null;(0).Reverse verification (committed tree, restoring trap, absolute paths)
git restore --source=80948340 -- engine.ts(the pre-fix file: bare early return back, normalisation gone). Proven on disk before any reading:if (!signal) return null;count 0 → 1,signal ?? {}count 1 → 0, disk blobbb6ca937…= BASE blob. No dist rebuild needed for this leg: the suite imports../engine.jsfrom source (noexports-resolved dist on the path). Observed direction on the mutated tree: 4 red / 5 green — the three (a)-family pins and (b) (whose precondition re-asserts the refusal) went red; (c) and the four (d) pins stayed green. Restore:git checkout HEAD -- engine.ts; disk blobfd009382…= HEAD blob,git diff HEADempty, counts back to 0 / 1.Gates (derived at head
8ed17503withnode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, 35 families +check:nul-bytes)Green (exit 0, each gate's own pass line read):
check-adr-0087-registration,check-changeset-no-major,check-ci-filter-parity,check-comment-mask-adoption,check-cross-package-test-inputs(both spellings),check-empty-changeset,check-keyed-text-bounds,check-plugin-teardown-shape,check-shard-attestation,check-tenant-audit-census,check-undeclared-dep-imports,docs-audit/check-affected-docs,docs-audit/check-drift-comment,pm/check-half-states,pm/release-rehearsal-clone --self-test,check:changeset-gate-self-tests,check:dispatcher-error-vocabulary,check:doc-authoring,check:engine-double-contract,check:logger-receiver-detach,check:objectql-double-limit,check:objectui-changeset,check:page-declaration-shape,check:pm-half-states,check:published-files,check:query-options-erasure,check:slot-lookup,check:test-source-alias,check:type-check-coverage,check:type-source-resolution,check:where-matcher,check:nul-bytes.NOT MEASURED locally (each exited 3 with its own
PREREQUISITE NOT METtext; CI owns them):check-test-completeness(grades a savedturbo run testlog),check:type-check-debt(--re-measureneeds the whole ledgered closure built),check:dual-build-cjs-loads(needs every package'sdist).Contract review (Clause ②)
git diff -U0 80948340..8ed17503 -- engine.ts | grep export): none.resume(runId, signal?)keeps its public signature (theIAutomationServicecontract inpackages/specis untouched — zero spec writes); the narrowed parameters are private / module-private.AutomationEngine.resume(runId)with no signal on a run paused at ascreennode with an unconditionalrequiredfield — before: accepted (run continued, variable unbound); after: rejected with the existingINVALID_SCREEN_INPUTenvelope, pause not consumed, corrected resubmission accepted. Every other input shape is unchanged; the HTTP door never exposed the accepting shape.patchfor@objectstack/service-automation— no new exported symbol, a tightened refusal on a published API using an existing error code, and the wire behaviour of the REST door is unchanged.objectstack-ai/hotcrm#1173's end-to-end pin covers the signal-carrying refusal, which this PR does not change; if it reds, that is the expected early warning and is not to be "fixed green" here.Out of scope
INVALID_SCREEN_INPUTrefusal as a terminal child failure (parent failed, still-paused child orphaned, corrected retry answersRUN_NOT_FOUND). Pre-existing on the signal-carrying path; this PR does not touch that block. service-automation: a subflow parent resume treats the delegated child screen's RETRYABLE refusal (INVALID_SCREEN_INPUT) as a terminal child failure — the parent is failed, the still-paused child is orphaned, and the corrected retry answers RUN_NOT_FOUND #14379 remains open.Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
Generated by Claude Code
🤖 Generated with Claude Code
https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
Generated by Claude Code