Skip to content

finding: a suspended flow run's variables are unreadable on run-detail — the paused recordLog omits them while the continuation keeps them #7639

Closed

Description

@huangyiirene

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.

Source

Split out of #7530 · evidence from the QA run #7517.

Activity

  1. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Findings triage (first grade): HOLD — finding stays; routing corrected domain:devx → domain:services (single-producer channel; correction, not a lane override).

    • Routing rationale (anchoring rule — the fix's landing package, not the discovery context): both named call sites live in packages/services/service-automation/src/engine.ts. Verified on origin/main @ 8669e5d: two status: 'paused' recordLog sites, neither passes variables/output, while the completed/failed sites in the same file pass output, and the suspend bookkeeping captures variables: Object.fromEntries(variables) a few lines above — exactly as filed. service-automation is on the domain:services row; the devx label reflected the card's discovery path (QA checklist work), not its landing.
    • Why hold, not queue: the card itself gates the fix as a product-surface design call (what a paused run's detail response exposes; a variables snapshot can carry business data, so shape + access control need deciding). Nothing is blocked today — PR docs(qa): correct five approvals checklist items against the #7517 run #7638's functional proxy oracle unblocked the QA item and its knownGaps entry points here.
    • Why not needs-user-decision yet: 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.
    • Restart conditions: ① real operator/user pull to inspect a paused run's variables (that is the moment to escalate the exposure question to needs-user-decision); ② the next service-automation change touching the paused recordLog sites — 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.
    • Dedup: finding: five checklist-text corrections for the approvals area — 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

  2. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    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 recordLog call sites simply omit what the continuation keeps. S-M, structural, named call sites.

    finding → pm:queue (domain:services).


    Generated by Claude Code

  3. self-assigned this
    on Aug 12, 2026
  4. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    CollaboratorAuthor

    Claim: PM loop round 4
    Session: session_01JVfWSQN9RDDGwdVSmMc29x
    Branch: claude/issue-7639-paused-run-variables
    Worktree: cloud session's own clone (mode:cloud)
    Domain: domain:services
    File surface: packages/services/service-automation/src/engine.ts (the two status: 'paused' recordLog call 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: opus
    Serial constraints cleared: same-package predecessors #7742 (PR #7847) and #7747 (PR #7875) are both MERGED; they touched plugin.ts and builtin/notify-node.ts, not engine.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 output and steps for completed/failed runs under the same access control. If that holds, adding the paused variables snapshot is consistency with the existing exposure envelope, not a new disclosure surface — dispatchable. If measurement shows variables carries 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

  5. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    CollaboratorAuthor

    PR: #7896 (draft) · Branch: claude/issue-7639-paused-run-variables

    Verdict on the §3 premise: HOLDS — proceed. Implemented.

    Measured, in order of how much each datum settles:

    1. variables was 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 internal ExecutionLogEntry (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 as StepLogEntry.retryAttempt ([Decision] A failed try_catch try-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 in engine.ts's own docblock — which makes this the enforce route under ADR-0049, not a new surface.
    2. One object, not three fields. packages/runtime/src/domains/automation.ts:489-496 — the handler answers deps.success(run) with the ExecutionLogEntry verbatim. No projection, no redaction, no masking, on any field, anywhere on that path (grep for redact/mask/sanitize across service-automation + the domain: zero non-comment hits).
    3. Access control is identical by construction. The only gate is the [17.0-rc2验收] 安全:REST /actions 与 /automation 派发路由缺少匿名拒绝门 —— 未认证调用者可触发 system 提权的 RLS/FLS 绕过写入 #5519 anonymous baseline at automation.ts:135-147, applied to the whole /automation domain — not per route, not per field.
    4. No credential class. No engine path injects platform secrets into the variable map. Connector credentials live in the connector registry and are never handed to a flow; the http node's signingSecret/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 reaches output.
    5. Same data already has a durable door. 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 — "output and steps already serve the same thing" — is false about breadth, though true about the envelope. Measured:

    • output is an author opt-in projection of the same map: output[v.name] = variables.get(v.name) for isOutput variables only (engine.ts:2977, 3726, 5934).
    • steps carries 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.
    • The paused snapshot is unconditionally broader: every node output under <nodeId>.<key> (engine.ts:5149), $record/record plus 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 of output for 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' recordLog sites — 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 both persistSuspendedRun and recordLog, 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 resume rebuilds 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) and variables (paused run), asserted deep-equal, at both the engine and the wire. Fails if a future change shapes one without the other.

    Only paused gains the key — completed/failed keep 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 — real AutomationEngine, 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 across output/steps/variables, and the anonymous gate asserting both code (UNAUTHENTICATED) and status (401) per ADR-0112 (AC Migrate from npm to pnpm #6).
    • Reverse-verified: all four engine assertions fail on 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-automation 936 tests ✅ · runtime domains 426 ✅ · plugin-approvals (a getRun consumer) 456 ✅ · ESLint clean · tsc --noEmit reports nothing on the changed files. GitHub CI not yet reported at the time of writing. No packages/spec file touched, so AC #7's gen:schema/gen:docs regeneration does not apply — the key was already declared.

    Deliberately not done

    getRun after a restart. A paused run's log entry lives only in the in-memory ring buffer (recordLog mirrors terminal runs to the store; a paused run by design is not terminal), so after a restart getRun on a still-paused run returns null even though sys_automation_run holds the row. That is a pre-existing gap orthogonal to the two call sites this card names, and closing it changes getRun's answer for paused runs from 404 to a run — a separate card if you want it.


    Generated by Claude Code

  6. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    CollaboratorAuthor

    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 为什么无处可读。
    • paused 快照无条件更宽:每个节点输出的 <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 域一把门,不分路由、不分字段,所以该域的 run output、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 的 isSuspendSignal catch,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

  7. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    CollaboratorAuthor

    更正:上一条 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions