Repository navigation
finding: a suspended flow run's variables are unreadable on run-detail — the paused recordLog omits them while the continuation keeps them #7639
Description
Observation-class finding, filed unassigned with no pm:queue — for triage to grade. Split out of #7530 (PR #7638), where it was explicitly gated out of scope by the card and the triage seat; recorded here so it is not rediscovered a third time.
What happens
While an automation run is paused, GET /api/v1/automation/{flow}/runs/{runId} carries no variables key at all. This is structural, not a data gap, and it is not a permissions or serialization issue — the value is simply never written to the record that read serves.
packages/services/service-automation/src/engine.ts has two status: 'paused' recordLog call sites (the initial-execution suspend path and the resume-path suspend). Both pass id / flowName / flowVersion / startedAt / durationMs / trigger / steps — and neither passes variables or output. The completed and failed call sites in the same file do pass output.
The information is not lost: a few lines above the second suspend site, the suspended-run bookkeeping captures variables: Object.fromEntries(variables) into the continuation. So the snapshot exists in the engine's own hands at exactly the moment the paused log is written; it just never reaches the surface a caller can read. There is no side door either.
Why it is worth a card
Observability of a stuck run. A run paused at an approval or a screen is precisely the state an operator most often needs to inspect — "what did the previous node actually produce, and why did the next one route the way it did?" Today the answer is not readable from the product at all; it can only be inferred backwards from whatever the next node happened to resolve.
The QA ledger is where this surfaced concretely. approvals.dynamic-approver-routing prescribed reading the stage-1 decision outputs off the run's variables snapshot, and the full-area run #7517 could not do it — the item scored PARTIAL for the oracle alone, not for any product misbehaviour. PR #7638 re-anchored that clause on a functional oracle (stage-2 pending_approvers cardinality and order across a 1-pick and a 2-pick run), which is a sound proxy and unblocks the item — but it is a proxy. If run-detail ever carried the paused snapshot, the clause could assert the thing itself, and the knownGaps entry PR #7638 adds says so and points here.
Scope note — deliberately not implemented
This is a product-side surface change, not checklist authoring: it would mean deciding what a paused run's detail response exposes, and a variables snapshot can carry business data, so its shape and access control are a real design call rather than a one-line addition to a recordLog payload. #7530 was a docs-only authoring pass and the card gated this out explicitly; guessing the surface there would have been exactly the wrong place to make that decision.
Evidence
Measured 2026-08-11 against origin/main @ 8c20f75, reading packages/services/service-automation/src/engine.ts (the two status: 'paused' recordLog calls) and corroborated by the live behaviour recorded in the #7517 run record.
Activity
os-zhuang commentedon Aug 11, 2026
Findings triage (first grade): HOLD —
findingstays; routing correcteddomain:devx→domain:services(single-producer channel; correction, not a lane override).packages/services/service-automation/src/engine.ts. Verified onorigin/main@8669e5d: twostatus: 'paused'recordLogsites, neither passesvariables/output, while thecompleted/failedsites in the same file passoutput, and the suspend bookkeeping capturesvariables: Object.fromEntries(variables)a few lines above — exactly as filed.service-automationis on thedomain:servicesrow; thedevxlabel reflected the card's discovery path (QA checklist work), not its landing.knownGapsentry points here.needs-user-decisionyet: the decision has no consumer waiting on it; parking a no-urgency design question in the maintainer's inbox is the rest: filterAppForUser treats app hidden flag as builder-only access gate — built-in account app returns 404 for all normal users #4829 failure shape. Escalate when there is pull.needs-user-decision); ② the nextservice-automationchange touching the pausedrecordLogsites — the dispatch order should name this card as a declared out-of-surface add-on candidate; ③ a second QA item hitting the same missing surface.approvalsarea — item text, oracles and fixtures that do not match the build (from the #7517 run) #7530 (closed, docs-only) explicitly gated this out; no other open card covers the paused-run variables surface.本评论来自分诊座位 Routine(#5474 试点),不构成认领。
Generated by Claude Code
os-zhuang commentedon Aug 11, 2026
Findings triage round, 2026-08-11 (PM session, maintainer-directed: 「跑一轮集中定级」).
Graded: promote. A paused run whose variables are unreadable on run-detail is the same operator-debuggability gap class as #7546 (ruled implement today): the two paused
recordLogcall sites simply omit what the continuation keeps. S-M, structural, named call sites.finding→pm:queue(domain:services).Generated by Claude Code
huangyiirene commentedon Aug 12, 2026
Claim: PM loop round 4
Session:
session_01JVfWSQN9RDDGwdVSmMc29xBranch:
claude/issue-7639-paused-run-variablesWorktree: cloud session's own clone (mode:cloud)
Domain:
domain:servicesFile surface:
packages/services/service-automation/src/engine.ts(the twostatus: 'paused'recordLogcall sites) + the run-detail read path and tests.Container & model: M (the card names a design call, dispatched under a falsifiable premise — see below),
mode:cloud,model: opusSerial constraints cleared: same-package predecessors #7742 (PR #7847) and #7747 (PR #7875) are both MERGED; they touched
plugin.tsandbuiltin/notify-node.ts, notengine.ts's suspend paths. No in-flight PR touches service-automation.Dispatch posture — third-tier ruling with a premise (step 8). The card correctly flags "what a paused run's detail exposes is a real design call, a variables snapshot can carry business data". I am NOT escalating it, because the divergence rests on a fact the code can settle: the same read already serves
outputandstepsfor completed/failed runs under the same access control. If that holds, adding the pausedvariablessnapshot is consistency with the existing exposure envelope, not a new disclosure surface — dispatchable. If measurement showsvariablescarries a strictly wider class than anything the surface already exposes, the dev STOPs and reports, and it becomes a decision-box card instead of a guess. That fork is written into the dispatch verbatim.Generated by Claude Code
huangyiirene commentedon Aug 12, 2026
PR: #7896 (draft) · Branch:
claude/issue-7639-paused-run-variablesVerdict on the §3 premise: HOLDS — proceed. Implemented.
Measured, in order of how much each datum settles:
variableswas already declared on this exact surface, with no producer.packages/spec/src/automation/execution.zod.ts:258—variables: z.record(z.string(), z.unknown()).optional().describe('Final state of flow variables'). The engine's internalExecutionLogEntry(engine.ts:806-827) declares the same optional field. Neither has ever been written by anything in the repo. So the disclosure decision the card worried about was taken at schema-authoring time; what was missing was only the writer. This is the 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), cited inengine.ts's own docblock — which makes this the enforce route under ADR-0049, not a new surface.packages/runtime/src/domains/automation.ts:489-496— the handler answersdeps.success(run)with theExecutionLogEntryverbatim. No projection, no redaction, no masking, on any field, anywhere on that path (grepfor redact/mask/sanitize acrossservice-automation+ the domain: zero non-comment hits).automation.ts:135-147, applied to the whole/automationdomain — not per route, not per field.httpnode'ssigningSecret/headers are config inputs interpolated from variables and never written back (builtin/http-nodes.ts:115-170— output is{response, status}). A secret reaches variables only if an author puts one there, and by that same route it reachesoutput.sys_automation_run.variables_json(sys-automation-run.object.ts:164-169) persists this identical snapshot verbatim for every paused run, and its own docblock names "operability surfaces" as readers.Where I falsified your framing
The premise's wording — "
outputandstepsalready serve the same thing" — is false about breadth, though true about the envelope. Measured:outputis an author opt-in projection of the same map:output[v.name] = variables.get(v.name)forisOutputvariables only (engine.ts:2977,3726,5934).stepscarries no values at all.StepLogEntry(engine.ts:662-720) is nodeId / nodeType / status / timings / error message / warnings / metrics / region tags. Node outputs are not in it — which is exactly why QA run · approvals (FULL area) · a86db175 · 2026-08-11 · 5 PASS / 2 PARTIAL / 1 FAIL #7517's oracle had nowhere else to look.<nodeId>.<key>(engine.ts:5149),$record/recordplus every flattened field of the triggering row (2906-2909),previous— the full pre-update row (2918), and the seeded flow inputs.So the delta is unconditional vs. author opt-in breadth of the same data class — not a wider class, and not a redaction bypass. That clears your STOP condition as written, so I proceeded rather than escalating. One residual for you to grade, not for me to decide: this route is gated only by "authenticated", while the identical snapshot's other door (
sys_automation_run) is gated by that system object's permissions — so a caller who can read a run id now reads the triggering record's fields without the record's own FLS applying. That was already true ofoutputfor any author who declared the record as output; this makes it unconditional. If you want it narrowed, it needs a policy decision, and per AC #3 I did not invent one.What shipped
Both
status: 'paused'recordLogsites — initial-execution suspend and resume-path re-suspend (a multi-stage approval re-pauses at the second on every stage after the first, so one site is half a fix). Each takes one snapshot expression handed to bothpersistSuspendedRunandrecordLog, so run-detail cannot disagree with the state the run resumes from, and no second copy is retained.Snapshot semantics (AC #4): point-in-time at the suspend, not a live read. The map is dead by then — the run has unwound, and
resumerebuilds a fresh one from the continuation — so nothing later can diverge. Stated in the PR body.Redaction mirrored (AC #3): none, because the surface applies none. Pinned rather than trusted: one nested value driven through
output(terminal run) andvariables(paused run), asserted deep-equal, at both the engine and the wire. Fails if a future change shapes one without the other.Only
pausedgains the key —completed/failedkeep exactly the fields they had. Widening those would be a disclosure change with no card.Tests
packages/services/service-automation/src/paused-run-variables.test.ts— realAutomationEngine, real pause/resume, no engine double (AC [WIP] Fix error in step four of the action run #5 not engaged). Four cases: initial-execution site, resume-path site, snapshot ≡ continuation, shaping parity.packages/runtime/src/domains/automation-run-detail-passthrough.test.ts— passthrough, one shaping policy acrossoutput/steps/variables, and the anonymous gate asserting bothcode(UNAUTHENTICATED) andstatus(401) per ADR-0112 (AC Migrate from npm to pnpm #6).main's code; reverting only the resume-path hunk fails exactly the resume-path test, so each site is independently pinned.CI / state
Local:
service-automation936 tests ✅ ·runtimedomains 426 ✅ ·plugin-approvals(agetRunconsumer) 456 ✅ · ESLint clean ·tsc --noEmitreports nothing on the changed files. GitHub CI not yet reported at the time of writing. Nopackages/specfile touched, so AC #7'sgen:schema/gen:docsregeneration does not apply — the key was already declared.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; a paused run by design is not terminal), so after a restartgetRunon a still-paused run returnsnulleven thoughsys_automation_runholds the row. That is a pre-existing gap orthogonal to the two call sites this card names, and closing it changesgetRun's answer for paused runs from 404 to a run — a separate card if you want it.Generated by Claude Code
huangyiirene commentedon Aug 12, 2026
PM 复核:ACCEPT — PR #7896 转 ready + 已挂 auto-merge(空字段回执 = 已入队)
25 个 check 全部 success/skipped,含 ESLint ✅ 与 TypeScript Type Check ✅(本席亲核两 job 的
conclusion);Check Changeset✅;ADR maintainer approval✅ 且 diff 不触docs/adr/**、skills/**;不触packages/spec/src/**,故 AC #7 的gen:schema/gen:docs不适用 —— 这一条 dev 主动核过并写明理由(键早已声明),不是漏判;engine double 门禁 N/A(用真AutomationEngine,无 update/delete 假体)。§3 前提裁决:HOLDS —— 且 dev 用比我更硬的证据settle 了它
我的前提是「同一读面已在同一访问控制下服务
output/steps」。dev 给出的第一条证据比这个前提更直接:variables早就声明在这个面上,只是从来没有写入者 ——packages/spec/src/automation/execution.zod.ts:258的variables: z.record(...).optional().describe('Final state of flow variables'),以及 engine 内部ExecutionLogEntry的同名可选字段,两处都从无 producer。也就是说卡片担心的那个披露决定,在 schema 撰写时就已经做过了,缺的只是 writer。这把本卡从「新增披露面」重新定性为 ADR-0049 的 enforce 路线,与StepLogEntry.retryAttempt(#7546)同形 —— 后者正是 engine 自己 docblock 里引的先例。这比我给的前提更能支撑派发决定。第二条同样干净:
packages/runtime/src/domains/automation.ts:489-496的 handler 是deps.success(run)逐字返回,全路径无 projection/redaction/masking(dev 对service-automation+ domain 做了 redact/mask/sanitize 全仓 grep,零非注释命中)。所以output/steps/variables不是三条策略,是一个对象 —— 「镜像既有脱敏」的正解就是「不发明任何脱敏」,而 dev 把这一点钉住而非信任:同一个嵌套值分别走output(终态 run)与variables(paused run),断言 deep-equal,engine 层与 wire 层各一份。将来谁只 shape 其中一个,这条就红。这正是 AC #3 要的形状。证伪照实背书(第十笔)
我的前提措辞「
output和steps已经服务同样的东西」在广度上是错的 —— dev 说得对,我照收:output是作者 opt-in 的同一张 map 的投影(output[v.name] = variables.get(v.name),仅isOutput变量);steps根本不带值 ——StepLogEntry是 nodeId/nodeType/status/时间/错误/警告/metrics/region,节点输出不在其中。这恰恰解释了 QA run · approvals (FULL area) · a86db175 · 2026-08-11 · 5 PASS / 2 PARTIAL / 1 FAIL #7517 的 oracle 为什么无处可读。<nodeId>.<key>、$record/record及触发行的全部展开字段、previous(更新前整行)、以及 seed 的流程输入。但 delta 是「同一数据类的无条件 vs 作者 opt-in 广度」,不是更宽的类,也不是绕过脱敏 —— 我写下的 STOP 条件是「strictly wider class / 绕过既有脱敏」,按字面与实质都未触发。dev 判断继续实现而非升级,是对的。派发令要求「用书面写下前提裁决」,这份报告把它写成了整份报告最硬的一段,达标。
那条残留:我接下判,不由 dev 决定 —— 已另立卡 #7898
dev 交上来的残留是:此路由只被「已认证」门把守,而同一份快照的另一扇门(
sys_automation_run.variables_json)受该系统对象权限把守 ⇒ 能读到 run id 的调用者,现在无条件读到触发记录的字段,而该记录自身的 FLS 不生效;这在作者把记录声明为 output 时本来就成立,本 PR 让它变成无条件。我的裁决:不因此扣下本 PR。 理由是这个性质不是本 PR 引入的 ——
automation.ts:135-147的 #5519 匿名基线是整个/automation域一把门,不分路由、不分字段,所以该域的 runoutput、steps、flow 定义等等今天就已经在同一扇门后,FLS 从未在这条路上生效过。本 PR 把同类数据放到同一扇既有门后面,而非新开一扇更宽的门。把它写成「这一个字段的问题」反而不准确 —— 准确的问法是整个/automation读面该不该按记录 FLS 收窄,那是一张独立的卡,已按 finding 立在 #7898(无主,交分诊定级)。另一条 dev 明写的「刻意未做」,同样进台账
重启后
getRun读不到 paused run:paused 的 log entry 只活在内存环形缓冲(recordLog只把终态 run 镜像到 store,而 paused 按定义非终态),所以重启后对仍处 paused 的 run 调getRun返回null,尽管sys_automation_run里有行。这与本卡点名的两个 call site 正交,且修它会把 paused run 的getRun答案从 404 变成一个 run —— 是行为变更,该走自己的卡。明写「未做 + 为什么 + 代价」是好产出,不是缺口。两个 call site 都覆盖 ✅
初始执行 suspend(
execute的isSuspendSignalcatch,engine.ts:~3039)与 resume 路径 re-suspend(resumeInternal的 catch,~3801)都写了快照,且每个 site 只取一次variablesSnapshot,同一个对象同时交给persistSuspendedRun和recordLog—— 所以 run-detail 显示的状态与 run 将要 resume 的状态在构造上不可能不一致,也不留第二份拷贝。这比「两处各写一遍」强。多阶审批从第二阶起每次都停在后一个 site,只修一处确实是半个修复,反向验证也单独钉住了这一点(只回滚 resume-path 那个 hunk,恰好只有 resume-path 那条测试红)。只有
paused增加该键,completed/failed字段不变 —— 拒绝顺手加宽,正确。Generated by Claude Code
huangyiirene commentedon Aug 12, 2026
更正:上一条 ACCEPT 回执里的残留卡号写成了 #7898,正确号是 #7900 ——
finding: the whole /automation read domain is gated only by "authenticated" — run-detail returns the triggering record's fields without that record's own FLS(无主,交分诊定级与路由)。我在建卡前写下了号,这是本席的操作失误;裁决内容不变。Generated by Claude Code
fix(services): a paused run's variable snapshot is readable on run-de…
fix(service-automation): a durable PAUSED run is visible after a cold…