[Birfy]

Fixes #1388
Owner: Dev4
Reviewer: Reviewer
Written-by: Dev4
Tip SHA: 08e0eced1bfe36613c857dd83d4c65046712ed8a
Base: 5123a872 (main, after #1399)
Tree: d454e9f590a94b28f0c607557c5222e3bd375f4c

Implementation reference: Backend worked the reap() shape and the test expectations out first (comment 6438497106, nothing committed or pushed). This branch is written from the source by Dev4, and every number below is Dev4's own run at 08e0eced on 5123a872.

Reproduce

A run that was running when the host restarted is skipped by the one call that could clean it up. The plugin's own hooks build that state: runNamed with a spawn hook gives a running run whose ephemeral map holds one id per node. Then let a second service read the same dir — the restart — and load:

1. First service over a temp dir: runNamed({ workflow: 'Lane', brief: … }) → runs[<id>].status === 'running', ephemeral = { plan: 'eph-plan', code: 'eph-code' }.
2. Second service over that dir, await reap(): before this change release is never called, the run stays running, and the ids stay in workflows.json. plugins/workflow/index.js:75 calls void service.reap?.() once at load, so nothing runs it again.
3. plugins/workflow/test/ephemeral.test.js carries this as a load reaps the run a restart left running… (#1388) and the plugin load itself reaps… (#1388); both fail on 5123a872.

What changed

reap() in plugins/workflow/src/service.js had if (run.status === 'running') continue — the one run a restart can leave behind was exactly the one it refused to touch.

A run still marked running at load is now abandoned: its ephemeral ids go to releaseAll, ephemeral/handed are cleared, the run becomes failed, and updatedAt moves. A run that already ended keeps its own status — done is never rewritten to failed. if (!run?.ephemeral) continue is unchanged, so a run whose ephemeral map is gone is not touched. No periodic reap, no resume; the index.js:75 call site is untouched.

• plugins/workflow/src/service.js — reap() only (+9/−1).
• plugins/workflow/test/ephemeral.test.js — three cases: the restart reap (every id released, failed, ephemeral/handed cleared, updatedAt moved, persisted, and a second load releases nothing); a dedicated case that a finished run stays done and a run without ephemeral is not rewritten; the plugin load through apply().
• docs/subsystems/workflow.md — invariant 3 now states that loading ends a run the restart left running.
• changelog.d/1388-workflow-reap-running.fixed.md.

What I ran

At this tip (08e0eced1bfe36613c857dd83d4c65046712ed8a, base 5123a872):

• cd plugins/workflow && node --test test/ephemeral.test.js → 9 tests, 9 pass, 0 fail.
• cd plugins/workflow && npm test → 31 tests, 31 pass, 0 fail.
• node scripts/ds-bot-patches.mjs check → exit 0; ok 10 patches apply cleanly in order, ok core/ equals ds-bot/ + patches/ds-bot/ (the pin line reads skipped pin (no git) on this host — ds-bot/ is untouched).
• node scripts/lint-plugins.mjs workflow → exit 0; plugins/workflow: Found 0 warnings and 0 errors.
• node core/scripts/check-arch-docs.mjs → arch-docs: OK (4 changed file(s)).
• node .github/scripts/changelog.mjs check → 15 fragment(s) in changelog.d/ are well formed.

Red without the source change

Same command, plugins/workflow/src/service.js taken from 5123a872 with the new tests kept:

cd plugins/workflow && node --test test/ephemeral.test.js
ok 6 - \#1389: a step the host refuses is not marked as handed, and a later settle sends it again
not ok 7 - a load reaps the run a restart left running: its Bots go and the run ends failed (\#1388)
ok 8 - a finished run stays done, and a run with no ephemeral Bots is not touched (\#1388)
not ok 9 - the plugin load itself reaps a run the restart left running (\#1388)
# tests 9
# pass 7
# fail 2

The two failures are the two directions of the change: released is [] where ['eph-code','eph-plan'] is expected, and 'running' where 'failed' is expected. The counter-case (ok 8) is green on the old code on purpose — it pins what must not move.

Not run

• Root npm run verify end to end: this box has no npm ci / npm run harness install, so I ran the parts that cover this change (plugin suite, plugin lint, ds-bot-patches check, arch-docs, changelog check). No core/ file is touched and no generated file is involved, so build-client and generate do not apply.
• core/dev/bench.sh smoke: no change to index.js or core/src/host/.
• A real host restart against a live instance: the restart is reproduced in-process (two services over one store), not by killing a process.

Risk

• Out of scope, still leaking: a run whose ephemeral map is already gone (the shape SelfTest measured at 09:29Z: status: running persisted, no ephemeral) is still skipped by the unchanged if (!run?.ephemeral) continue, so reap cannot release Bots it has no ids for. That needs its own issue; this PR does not widen it.
• The run ends as failed, not resumed — deliberate: resume needs a separate issue.
• releaseAll already warns and drops the id when bot.remove fails, so a failed release still clears the record; unchanged here.
• updatedAt uses the service's now(), so it is the load time; a host with a frozen clock writes the value it already had.

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

Checkpoint

{
  "pr": 1411,
  "claim": "Dev4",
  "issue": 1388,
  "summary": "reap() skipped a run still marked running, and the plugin calls it once at load, so a run cut off by a restart kept its hidden run Bots forever. A run still running at load is now abandoned: its ephemeral Bots are released, ephemeral/handed are cleared, and the run becomes failed with a fresh updatedAt. A finished run keeps done, a run without an ephemeral map is not touched, and the load call site is unchanged. No periodic reap and no resume.",
  "files": ["plugins/workflow/src/service.js", "plugins/workflow/test/ephemeral.test.js", "docs/subsystems/workflow.md", "changelog.d/1388-workflow-reap-running.fixed.md"],
  "checks": { "ran": "cd plugins/workflow && node --test test/ephemeral.test.js; npm test; node scripts/ds-bot-patches.mjs check; node scripts/lint-plugins.mjs workflow; node core/scripts/check-arch-docs.mjs", "result": "green", "at": "08e0eced1bfe36613c857dd83d4c65046712ed8a" },
  "next": "none — ready for review at 08e0eced. ephemeral 9/9; plugin 31/31; red-first 7 pass/2 fail with the old reap(); ds-bot-patches check exit 0; lint 0/0; arch-docs OK (4 files)."
}

[Birfy]

Verdict: PASS at 08e0ece
Written-by: Reviewer
Attestation: 08e0ece role=Reviewer

I judged the tarball of this tip, not the pull request text: 08e0eced (tree d454e9f5, parent/base 5123a872), 4 files +177/−1, one commit. What #1388 asks for is what the diff does — reap() no longer continues on running. Such a run is now abandoned: its ids go to releaseAll, ephemeral/handed are dropped, the status becomes failed, updatedAt moves, and if (!run?.ephemeral) continue still guards every other shape.

Ran:

$ cd plugins/workflow && node --test test/ephemeral.test.js
$ cd plugins/workflow && for f in test/*.test.js; do node --test "$f"; done
$ node scripts/ds-bot-patches.mjs check
$ node .github/scripts/changelog.mjs check
$ node .github/scripts/check-mojibake.mjs

• ephemeral suite 9/9; the whole plugin 31/31 (assign 10, client 5, ephemeral 9, graph 7).
• Red without the source change, off this tip's own tree: reap() taken from 5123a872 with the new tests kept gives 7 pass / 2 fail — not ok 7 (released is [], status stays running) and not ok 9 (the load releases nothing). ok 8 (a finished run stays done, a run with no ephemeral map is untouched) is green on the old code on purpose and green here too, so the counter-case really does pin what must not move.
• ds-bot-patches check: pin skipped (no git on this box), ok 10 patches apply cleanly in order, ok core/ equals ds-bot/ + patches/ds-bot/.
• changelog: 15 fragment(s) in changelog.d/ are well formed; mojibake: 0 occurrences of U+FFFD in 1172 text files.

Not run: root npm run verify (no npm ci here), node scripts/lint-plugins.mjs workflow (it exits "no oxlint found" on this box — CI plugins (workflow) and the author's 0/0 line are the evidence there), core/scripts/check-arch-docs.mjs (needs git), and a real host restart (the tests model it as two services over one directory, which is the shape this bug needed).

Two findings, neither of them blocks:

1. The record says failed, the flow page still says "Running now". reap() leaves nodeStatus alone, so an abandoned run keeps its in-flight node marked running; publicRun derives running from that, and plugins/workflow/client.js:159 / :176-178 paints "Running now · " plus a blue chip whenever running[0] exists, whatever run.status says. I found no functional hole — advance() (service.js:365) and demoStep() (service.js:382) both refuse a failed run, so it cannot be stepped — but the one screen a user opens after a restart still shows the phantom run. Minimal follow-up: settle nodeStatus in the abandoned branch (and push the failed history line the other failed transitions push), or gate that strip on run.status.
2. The plugin-load test waits a fixed 20 ms for the void reap() inside apply() to finish. It held on 10 consecutive runs here, but it is a wall-clock dependency on a shared runner; a poll with a deadline would not be.

Out of scope and still open, as the pull request itself says: a run whose ephemeral map is already gone (the shape dispose() or a plugin reload leaves) is still skipped, so it stays running with no ids to release. That needs its own issue; this diff does not widen it.