Uh oh!
There was an error while loading. Please reload this page.
fix(services): a paused run's variable snapshot is readable on run-detail (#7639) - #7896
Conversation
…tail (#7639) While a run was paused, `GET /api/v1/automation/{flow}/runs/{runId}` carried no `variables` key at all — so a run stopped at an approval, a screen or a wait, the state an operator most often needs to inspect, answered with no variable state. "What did the previous node produce, and why did the next one route the way it did?" was answerable only by inference from what the next node resolved. Structural, not a data gap. `ExecutionLogSchema` has declared `variables` ("Final state of flow variables") since the schema was written, and the engine's own log entry declared it too — with no producer anywhere, so the key the run-detail read publishes was never populated. The same declared-with-no-writer shape as `StepLogEntry.retryAttempt` (#7546). The engine already held the answer: both `status: 'paused'` recordLog sites sit just below the suspend bookkeeping that computes `Object.fromEntries(variables)` for the continuation. Both paused sites now write it — the initial-execution suspend AND the resume-path re-suspend, so a multi-stage approval is readable at every stage and not only the first. Each site takes ONE snapshot expression and hands the same object to the continuation and to the log entry, so what an operator reads cannot disagree with the state the run will resume from. Snapshot semantics: point-in-time at the suspend, not a live read. The variable map is dead by then (the run has unwound; resume rebuilds a fresh one from the continuation), so there is nothing later to diverge from. The exposure envelope is unchanged. The run-detail read serves the log entry verbatim — no projection, redaction or masking on any field — so `variables` gets exactly the treatment `output` and `steps` already get, under the same #5519 anonymous baseline that gates the whole /automation domain. No new redaction policy is invented; the tests pin that the three fields keep ONE shaping policy. Terminal runs keep exactly the fields they had. Closes#7639
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also reference the affected code. These are read-only:
|
Uh oh!
There was an error while loading. Please reload this page.
Closes#7639
What was wrong
While an automation run was paused,
GET /api/v1/automation/{flow}/runs/{runId}carried novariableskey at all. A run stopped at an approval, a screen or a wait — precisely the state an operator most often needs to inspect — answered with no variable state, so "what did the previous node actually produce, and why did the next one route the way it did?" could only be inferred backwards from whatever the next node happened to resolve.Structural, not a data gap. The engine's two
status: 'paused'recordLogcall sites passedid/flowName/flowVersion/startedAt/durationMs/trigger/stepsand stopped there — while sitting a few lines below the suspend bookkeeping that already computesObject.fromEntries(variables)for the continuation.The premise this was dispatched under, and what measuring it found
The dispatch gated implementation on a falsifiable claim: the same run-detail read already serves
outputandstepsunder the same access control, so addingvariablesis consistency with the existing exposure envelope rather than a new disclosure surface. Ifvariablesturned out to carry a strictly wider class of data — credentialsoutput/stepsprovably cannot hold, or a bypass of a redaction the other fields go through — the instruction was to stop and escalate.The premise holds, and the code settles it more directly than the framing did:
packages/runtime/src/domains/automation.ts:489-496— the handler answersdeps.success(run)with theExecutionLogEntryverbatim. There is no per-field projection, redaction or masking anywhere on this path, for any field. Sooutput,stepsandvariablesare not three policies; they are one object.automation.ts:135-147, applied to the whole/automationdomain. Access control is identical by construction, not by coincidence.variableswas already declared on this surface.packages/spec/src/automation/execution.zod.ts:258declaresvariables: z.record(z.string(), z.unknown()).optional()— "Final state of flow variables" — and the engine's internalExecutionLogEntrydeclares the same optional field. Neither has ever had a producer. The disclosure decision the card worried about was taken at schema-authoring time; only the writer was missing. Same declared-with-no-writer shape asStepLogEntry.retryAttempt([Decision] A failedtry_catchtry-region produces no step at all — after a caught failure an operator cannot tell what failed, how many attempts ran, or which node threw #7546).httpnode'ssigningSecret/headers are config inputs read from variables, never written back to output (builtin/http-nodes.ts:115-170, whose output is{response, status}). A secret reaches variables only if a flow author puts one there — and by that same route it can reachoutput, which is a projection of the identical map.One thing the framing got wrong, recorded because it is a real delta the premise's wording glosses:
outputandstepsare not as wide asvariables.outputis an author opt-in projection of the same map (output[v.name] = variables.get(v.name)forisOutputvariables only), andStepLogEntrycarries no values at all — node ids, types, statuses, timings, error messages, warnings, metrics. The paused snapshot is unconditionally broader: every node output under<nodeId>.<key>, the triggering record and its flattened fields,previous, and the seeded flow inputs. That is unconditional vs. opt-in breadth of the same data class — not a wider class, and not a redaction bypass, so it clears the STOP condition. The residual worth naming for a future card is that this route is gated only by "authenticated", whereas the identical snapshot's other door —sys_automation_run.variables_json, which already persists it verbatim for every paused run — is gated by that system object's permissions.The change
Both
status: 'paused'recordLogsites now carry the snapshot:execute'sisSuspendSignalcatch)resumeInternal's catch) — a multi-stage approval re-pauses here on every stage after the first, so covering only the other site would leave exactly the stages an operator needs to inspect unreadableEach site takes one snapshot expression and hands the same object to both
persistSuspendedRunandrecordLog, so the state run-detail shows cannot disagree with the state the run will resume from — and there is no extra retained copy.Snapshot semantics
Point-in-time copy taken at the suspend — not a live read. The map is dead by the time the log entry exists: the run has unwound to the catch, and
resumerebuilds a fresh map from the continuation rather than reusing this one. So there is nothing later for the snapshot to diverge from, and because the continuation receives the very same object, the snapshot is by construction the state the run resumes from. Onlypausedgains the key;completed/failedkeep exactly the fields they had, since widening those would be a disclosure change with no card behind it.Redaction — what was mirrored
The existing surface applies none, to any field. So the mirror is to invent none, and to pin that fact rather than trust it:
automation-run-detail-passthrough.test.tsdrives one nested value throughoutput(terminal run) and throughvariables(paused run) and asserts they read back deep-equal, andpaused-run-variables.test.tsdoes the same at the engine level. If a future change starts shaping either field without the other, those assertions fail.Tests
packages/services/service-automation/src/paused-run-variables.test.ts(realAutomationEngine, real pause/resume — no engine double):<nodeId>.<key>, the triggering record, declared flow variables, run identityoutputpackages/runtime/src/domains/automation-run-detail-passthrough.test.ts(the wire half):variablespasses through untouchedoutput/steps/variablescode(UNAUTHENTICATED) andstatus(401) per ADR-0112, and that the service is never reachedReverse-verified: all four engine assertions fail on
main's code, and reverting only the resume-path hunk fails exactly the resume-path test — so each site is independently pinned.Deliberately not done
getRunafter a restart. A paused run's log entry lives only in the in-memory ring buffer (recordLogmirrors terminal runs to the store; paused ones by design are not terminal). After a restart,getRunon a still-paused run returnsnulleven thoughsys_automation_runholds the row — a pre-existing gap, orthogonal to the two call sites this card names, and closing it would changegetRun's answer for paused runs from 404 to a run.packages/specchange, so nogen:schema/gen:docsregeneration is required — the key was already declared.Verification
service-automation: 936 tests / 78 files passruntimedomain suite: 426 tests / 21 files passplugin-approvals(consumesgetRun): 456 tests passtsc --noEmitreports nothing on themGenerated by Claude Code