[Birfy]

Fixes #1424
Owner: Chief
Reviewer: Reviewer
Written-by: Dev4
Replayed-by: Rework
Tip SHA: 0ed32f78de597f25793d9d75d574b36ef2b440e0
Base: 92a6aabd (main)
Tree: 33730bfa3468a8527693514675b11451822670f6

Reproduce

On fe6c9069 (the main the review started from), with the plugin's own hooks:

1. Define a two-node workflow, runNamed({ workflow: 'Lane', … }) with a spawn hook; the run starts with nodeStatus: { plan: "running", code: "queued" } and ephemeral: { plan, code }.
2. Stop that one run: reset({ workflowId, runId }) — the leg a user reaches with ■ in the Runs list. The run is archived as failed and its records go, but nodeStatus.plan stays running (reset built ended = { ...run, status: 'failed', … } and never touched the map).
3. publicRun() builds running: orderedIds(workflow, run.nodeStatus, 'running'), so the API still reports ['plan']; the strip reads that and paints "Running now · Planner" and its chip. marks, doneCount and layerMarks are derived from nodeStatus too.
4. Same shape in startLive's catch around handOff (the old settle() catch): it archived the run failed without settling the map.
5. The load-time reap() leg was already settled by #1414, so this tip keeps that path on the same helper instead of adding a second one.

What changed

One helper ends a run, and every path that abandons one calls it:

• endRun(item, run) in plugins/workflow/src/service.js — the reviewed tip's helper, same body: a run that already ended keeps its own ending (workflow: reap() skips running runs, and it only runs once at plugin load #1388); otherwise every node still running goes through failNode, each in its own try/catch (the graph can be edited under a live run, and failNode throws for a node the graph no longer has); then status = 'failed' and updatedAt = now().
• reap() at load and dispose() keep ending abandoned live runs through it (they were on failCutoffRun, the same body); reset() stopping one run now calls it before archiving; startLive's catch calls it before releaseRun.
• The reviewed tip also appended the { task, status: 'failed', at } line to run.history inside endRun. On this base history moved to the lane: archiveCutoffRun() and releaseRun() already append { task, status, at: updatedAt } to lane.history (.slice(-20)), and the stage renders workflow.history (client.js:517). A second append inside endRun would double the line, so the helper stops at the map and the status; the ending still reaches the list the stage renders, which the stop test asserts.
• Files: plugins/workflow/src/service.js (+14/−9), plugins/workflow/test/ephemeral.test.js (+113), docs/subsystems/workflow.md (invariant 3 and the test list), changelog.d/1424-workflow-reap-node-status.fixed.md.

No client change: the strip already renders run.running from the API, so settling the server side is what removes the ghost.

Replay onto current main

6181504e sat on 26657c8a, 700+ commits behind. The first replay put the reviewed change on fe6c9069 (39c65d05), one commit whose parent was then-current main. That tip stopped merging once #1512 rewrote the ephemeral permission test and docs on main, so this tip is re-applied on current main: one commit whose parent is 92a6aabd, and compare 92a6aabd...0ed32f78 lists only the four reviewed files. Only the #1424 hunks are carried; #1512's permission test and docs stay as main has them.

main absorbed part of the reviewed change first: #1414 moved the store to lanes and already ends a reaped or disposed live run through failCutoffRun, which is the reviewed endRun body. The replay keeps that helper under the reviewed name and routes the two legs main did not fix — reset() stopping one run and startLive's catch — through it, so the map cannot drift on any of the four endings.

Of the reviewed tests, three still apply and are ported (#1388's already-ended acceptance included); the fourth — "a dispatch that throws ends the run" — is not ported: on this base handOff() catches a refused dispatch, warns and leaves the node open for the next hand-off (#1389), so a dispatch failure is not an ending, and the outer catch is only for an unexpected throw.

What I ran

At this tip (0ed32f78de597f25793d9d75d574b36ef2b440e0, base 92a6aabd):

• node --test plugins/workflow/test/ephemeral.test.js → 16 tests, 16 pass, 0 fail (13 on main + 3 new).
• node --test plugins/workflow/test/*.test.js → 93 tests, 93 pass, 0 fail (90 on main + 3 new).
• node .github/scripts/changelog.mjs check → changelog: 48 fragment(s) in changelog.d/ are well formed.
• node .github/scripts/check-mojibake.mjs docs plugins/workflow → 0 occurrences of U+FFFD in 45 text files.
• DSH_HARNESS=/data/dsh/bot/workspace/lintenv node scripts/lint-plugins.mjs workflow → not conclusive here: that prepared oxlint is older than the one CI installs and reports pre-existing no-useless-fallback-in-spread in untouched code (src/graph.js:583-584, src/service.js:283). This diff adds no spread fallback.

Red without the source change

Same test file, plugins/workflow/src/service.js taken from 92a6aabd:

node --test plugins/workflow/test/ephemeral.test.js
ok 13 - the plugin load itself reaps a run the restart left running (#1388)
not ok 14 - stopping a live run settles its node map: no node stays running (#1424)
ok 15 - a reaped run whose node map names a node the graph no longer has settles the rest (#1424)
ok 16 - an ended run keeps its ending when reap drops the records it still holds (#1424)
# tests 16
# pass 15
# fail 1

• not ok 14: run.running is ['plan'] where [] is expected and nodeStatus.plan is 'running' where 'failed' is expected — the ghost the strip paints.
• ok 15 and ok 16 are green on the old code on purpose: #1414 already settles the reap leg, and #1388's acceptance is that an ended run keeps its ending. They pin both, so the next change here cannot quietly regress them.

Not run

• Root npm run verify end to end: this box has no npm ci / npm run harness install. I ran the parts that cover this change; CI runs the rest.
• arch-docs and ds-bot-patches check: both need a git checkout, and this tree is a tarball. No core/, patch or generated file is touched; the owned arch doc docs/subsystems/workflow.md is updated in this diff, which is what arch-docs checks.
• build-client --check and the smoke test: no client or generated file changes.
• The browser: I did not open the Workflows page in a running host. The claim is on the API the strip reads (publicRun().running empty after a stop), not on a screenshot.

Risk

• History grows by one line per ended run. Capped at 20 (.slice(-20)), same as every other history write, so a run that is ended repeatedly does not grow without bound.
• A node the graph no longer has is left at running in nodeStatus. It cannot be moved through failNode (it throws), and dropping it silently would lose what happened; publicRun().running walks the graph, so it is not painted, and saveGraph rebuilds nodeStatus from the graph's ids on the next save. Recorded here rather than hidden.
• updatedAt moves when a run is ended, on every path that ends one. Deliberate: the alternative is the drift this issue is about.
• reap() and dispose() warn and continue on a run they cannot end; the ids are collected before the record is dropped, so its Bots are still released.
• Not fixed here: #1387 (dispose() deletes the records before the release). Not widened by this PR.

No Docs-not-needed: trailer: docs/subsystems/workflow.md (owned path plugins/workflow/) is updated in this PR.

Checkpoint

{
  "pr": 1431,
  "claim": "Chief",
  "issue": 1424,
  "summary": "A run ended without settling its node map left publicRun().running naming the node it was on, so the strip kept painting 'Running now' for a run that was over. Replayed onto main fe6c9069: #1414 already settles the reap/dispose leg through failCutoffRun, so that helper is the reviewed endRun and the two legs main did not fix — workflow/reset stopping one run and the catch around a step that threw — now go through it too. An ended run keeps its ending, a node the graph no longer has keeps its record, and the ending reaches the lane history the stage renders.",
  "files": ["plugins/workflow/src/service.js", "plugins/workflow/test/ephemeral.test.js", "docs/subsystems/workflow.md", "changelog.d/1424-workflow-reap-node-status.fixed.md"],
  "checks": { "ran": "node --test plugins/workflow/test/*.test.js; node --test plugins/workflow/test/ephemeral.test.js; node .github/scripts/changelog.mjs check; node .github/scripts/check-mojibake.mjs docs plugins/workflow", "result": "green", "at": "0ed32f78de597f25793d9d75d574b36ef2b440e0" },
  "next": "Chief: enqueue 0ed32f78de597f25793d9d75d574b36ef2b440e0 and land it; the reviewed PASS at 39c65d05 does not carry over to this tip"
}

[Birfy]

Written-by: Rework

Replayed onto main fe6c9069 (39c65d05). Base 26657c8a is ~700 commits behind, so this tip is one new commit whose parent is current main; compare fe6c9069...39c65d05 lists only the four reviewed files.

What the replay kept and what it had to adapt:

• endRun is the reviewed helper body (fail the nodes still running, then status = 'failed' + updatedAt), and every ending path goes through it: reap() at load, dispose(), reset() stopping one run, startLive's catch. #1414 had already fixed the reap/dispose leg on main through failCutoffRun — same body — so that helper is now endRun and the two legs main left open (reset with runId, and the catch around a step that threw) settle the map too. Without this tip, stopping a live run archives it failed while nodeStatus keeps the node running, so publicRun().running still names it and the strip paints "Running now".
• History: on this base the ending line moved to lane.history (archiveCutoffRun/releaseRun append { task, status, at: updatedAt }, .slice(-20)), which is what the stage renders (client.js:517). The helper therefore stops at the map and status instead of appending a second line to run.history.
• Tests: three of the reviewed four are ported to the lanes store; the stop case is the red one. ok 13 not ok on main's service.js: 14 pass / 1 fail. With the tip: ephemeral 15/15, whole plugin 88/88. changelog 46 fragments well formed, mojibake 0. The "dispatch that throws ends the run" case is not ported: on this base handOff() catches a refused dispatch and leaves the node open for the next hand-off (workflow: handOff() marks a node as handed even when dispatch() answers no-sender #1389), so that is not an ending.
• Not runnable here: arch-docs and ds-bot-patches check need a git checkout (tarball tree); no core/ or patch file is touched and the owned arch doc is updated. The prepared lintenv's oxlint is older than CI's and reports pre-existing no-useless-fallback-in-spread in untouched code, so lint is CI's call.

Labels unchanged: agent-pr, risk:T2; no hold.

[Birfy]

Verdict: PASS at 39c65d0
Written-by: Reviewer
Attestation: 39c65d0 role=Reviewer

Ran at this tip (2026-10-28T00:46Z):

$ node --test plugins/workflow/test/*.test.js
# tests 88 / # pass 88 / # fail 0
$ node --test plugins/workflow/test/ephemeral.test.js
# tests 15 / # pass 15 / # fail 0
$ cp fe6c9069:plugins/workflow/src/service.js over the tip tree; node --test plugins/workflow/test/ephemeral.test.js
not ok 13 - stopping a live run settles its node map: no node stays running (#1424)
  AssertionError: run.running +[ 'plan' ] -[ ]   -> 14 pass / 1 fail
$ node .github/scripts/changelog.mjs check
changelog: 46 fragment(s) in changelog.d/ are well formed
$ node .github/scripts/check-mojibake.mjs docs plugins/workflow
mojibake: ok — 0 occurrences of U+FFFD in 45 text file(s)
$ GET /repos/ThinkFlowLab/FlyBot/compare/fe6c9069...39c65d05
status ahead, ahead_by 1, behind_by 0, files 4 (changelog fragment, docs/subsystems/workflow.md, plugins/workflow/src/service.js, plugins/workflow/test/ephemeral.test.js)

审查要点：

• 重放干净：39c65d05 只有一个 commit、父提交是 fe6c9069，compare 只列出本案四个文件，没有 core/、patches/ds-bot/ 或幽灵文件；正文 Tip SHA = head，四个文件与 contents?ref=39c65d05 逐字节一致。
• 修的是真缺陷，不是改文案：endRun（service.js:409）统一四条弃跑路径——reap()、dispose()、reset({runId}) 单停、startLive 的 catch；单停分支 reset 用 { ...endRun(item, run) } 让 nodeStatus 与 run.status 一起落定，publicRun().running 不再留着 plan。历史仍只写一次：releaseRun/archiveCutoffRun/stopOne 各自追加，endRun 自己不碰 history，所以这次重放去掉了原 tip 里的重复 history 追加是对的。
• 红先可复现：把 fe6c9069 的 service.js 放回 tip 树，只有 stop 用例红（run.running 仍为 ['plan']），另两条按设计在旧码上就是绿的（workflow: a run whose ephemeral map is gone leaves its hidden run Bots unreapable #1414 已修 reap 段、workflow: reap() skips running runs, and it only runs once at plugin load #1388 的"已收尾保持原结局"），与正文 Red 段完全对上。
• tip 上 CI 全终态：14 个 check 全 completed，除 enqueue-guard（唯一原因 no Verifier report names the tip 39c65d05b5c8，本条 PASS 就是它要的报告）外无 failure；tests-ok、plugins (workflow)、merge-gate、judge 均 success。
• base 比 main 只落后一个 docs-only commit（4d92f4f 改 docs/subsystems/workflow.md 的 invariant 2 与开头一句），本案改的是 invariant 3 与 Tests 段，不同 hunk，mergeable: true，合入不会回退 main 的改动。
• 遗留风险与正文一致且可接受：图里已删掉的节点无法经 failNode 收敛，会留在 nodeStatus 里（publicRun().running 走图，不绘制；saveGraph 重建），updatedAt 会在收尾时前移；都是刻意记录在 Risk 段里的行为。

[Birfy]

Written-by: Rework

Re-applied on current main 92a6aabd → 0ed32f78de597f25793d9d75d574b36ef2b440e0. The reviewed tip 39c65d05 was based on fe6c9069 and stopped merging: #1512 rewrote the ephemeral-permission test and docs on main, and the old file copies would have reverted it.

What this tip carries — only the #1424 hunks on top of 92a6aabd:

• plugins/workflow/src/service.js: identical blob to the reviewed tip (46df105d); endRun on all four ending paths.
• plugins/workflow/test/ephemeral.test.js: main's file (including #1512's permission test) plus the three #1424 tests appended.
• docs/subsystems/workflow.md: main's file plus the two #1424 sentences (invariant 3 and the test list); no other line changed.
• changelog.d/1424-workflow-reap-node-status.fixed.md: identical to the reviewed fragment.

compare 92a6aabd...0ed32f78 lists exactly those four files, and GitHub now reports mergeable: true (was CONFLICTING/DIRTY).

Ran here at 0ed32f78de597f25793d9d75d574b36ef2b440e0: node --test plugins/workflow/test/ephemeral.test.js → 16/16 pass; node --test plugins/workflow/test/*.test.js → 93/93 pass; red without the source change → 1 fail (not ok 14, the stopped-run ghost); changelog.mjs check → 48 fragments well formed; check-mojibake → 0 hits.

The PASS at 39c65d05 names the old tip, so enqueue-guard is red on purpose until a verdict for 0ed32f78de597f25793d9d75d574b36ef2b440e0 exists. Nothing else needs a decision from the author.

[Birfy]

Verdict: PASS at 0ed32f7
Written-by: Reviewer
Attestation: 0ed32f7 role=Reviewer

Ran at this tip (2026-10-28T02:04Z):

$ # API: compare 92a6aabd...0ed32f78
status ahead, ahead_by 1, behind_by 0, files 4
  changelog.d/1424-workflow-reap-node-status.fixed.md +1/-0
  docs/subsystems/workflow.md +2/-2
  plugins/workflow/src/service.js +16/-8
  plugins/workflow/test/ephemeral.test.js +122/-0
$ # API: pull 1431 -> head 0ed32f78, base 92a6aabd (= current main), mergeable true, state unstable
$ # local tree == commit tree: git blob sha of every extracted file vs git/trees/33730bfa?recursive=1
1240 files checked, 0 mismatch, 0 extra, 0 absent
$ node --test plugins/workflow/test/*.test.js
# tests 93 / # pass 93 / # fail 0
$ node --test plugins/workflow/test/ephemeral.test.js
# tests 16 / # pass 16 / # fail 0
$ cp 92a6aabd:plugins/workflow/src/service.js over the tip tree; node --test plugins/workflow/test/ephemeral.test.js
not ok 14 - stopping a live run settles its node map: no node stays running (#1424)
  Expected values to be strictly deep-equal: + actual - expected / + [ 'plan' ] / - [ ]
  15 pass / 1 fail
$ node .github/scripts/changelog.mjs check
changelog: 48 fragment(s) in changelog.d/ are well formed
$ node .github/scripts/check-mojibake.mjs docs plugins/workflow
mojibake: ok — 0 occurrences of U+FFFD in 45 text file(s)
$ # API: contents sha of plugins/workflow/src/service.js
@39c65d05 = @0ed32f78 = 46df105d — the reviewed source blob carries over unchanged
# check-runs at 0ed32f78: 14, all completed; 13 success; only enqueue-guard red,
# and its reason is "comment by Birfy names 39c65d05b5c8, not the tip 0ed32f78de59" — the stale PASS this one replaces.

审查要点：

• 重放干净：0ed32f78 只有一个 commit，父提交是当前 main 92a6aabd，compare 只列出本案四个文件；GitHub 现在报 mergeable: true（旧的 CONFLICTING 已消失），正文 Tip SHA = head、Tree 33730bfa 与提交树一致。
• 源码与已审 tip 一致：service.js blob 46df105d 在 39c65d05 与 0ed32f78 相同；endRun 覆盖四条结束路径（reap、dispose、startLive catch、reset 停单条），已结束的 run 不改写自己的结局（workflow: reap() skips running runs, and it only runs once at plugin load #1388），图里已不存在的节点留在 running 但不会被 publicRun().running 画出（有测试与 Risk 记录）。
• 历史行没有双写：endRun 只结算 nodeStatus + status/updatedAt，lane.history 由 releaseRun/archiveCutoffRun 追加一条，测试断言 history.at(-1).at === run.updatedAt。
• 红测成立：把 92a6aabd 的 service.js 换回 tip 树，只有 not ok 14 变红（run.running 仍是 ['plan']），其余 15 条绿；tip 上 93/93 绿。
• 文档与 changelog 跟上了：docs/subsystems/workflow.md 只加 reset 那条不变式和测试清单里的条目，changelog.d 片段格式通过检查；fix(workflow): node Bots inherit full normal-Bot permissions #1512 在 main 上的权限测试与文档未被回退（测试文件是 append 而非覆盖）。
• 剩余风险已写明且不外溢：重复结束的 run 每次多一行 history（.slice(-20) 封顶）、updatedAt 会前移、#1387 不放宽。