fix(workflow): Reset/Stop writes state before releasing run Bots, so a failed release erases the only id record #1476

Written-by: FlyBot Product
Line numbers drift: identify by symbol (reset(), stopOne(), take(), releaseAll(), write() in plugins/workflow/src/service.js), not by line number.

What happens

Both ways of ending a run persist the state change before the run's Bots are released:

• per-run Stop (reset({ workflowId, runId }) → stopOne()),
• whole-workflow Reset (reset({ workflowId }) → take()).

stopOne() and take() delete the run's ephemeral map, clear lane.active, append the history row and push() a system line, then call write(). Only afterwards does the caller await releaseAll(saved.ids, release, warn). releaseAll() in plugins/workflow/src/ephemeral.js swallows a failed release into a warn() — it never rejects. So a release that fails leaves the Bot alive while the only record of its id has already been flushed to disk. Nothing can name that Bot again: it is an orphan until the process exits.

That is exactly the invariant dispose() states and is written under: "Release first, then forget the ids (#1387) … A record that drops an id before its Bot is gone leaves a hidden Bot that no state names, so nothing can ever find it again." reset() does the opposite of that comment.

Reachability

• Per-run Stop arrived with Fix workflow concurrent run controls and flowchart layout #1467 and is not on the running instance yet (8f1ac12).
• The whole-workflow Reset path has the same ordering and is on 8f1ac12: plugins/workflow/client.js calls workflow/reset (onReset: in the run controls). So the defect is reachable on the running product today, not only on main.

Reproduction (baseline 5b9d0d7)

Two runs live on one lane (maxConcurrent: 2), two spawnable nodes, release made to throw for one id, then reset() on the first run. Probe harness line (test/probe-stop-1467.test.js) prints its own label; output verbatim:

# PROBE U victim: run-643c516a ids: eph-plan-1,eph-code-2
# PROBE U stop ok: true | still live: run-0d255308
# PROBE U release attempted: eph-plan-1,eph-code-2
# PROBE U warns: ["flybot-workflow: could not remove eph-plan-1: bot remove failed"]
# PROBE U victim ids still in file: none
# PROBE U archived last: {"id":"run-643c516a","status":"failed","nodeStatus":{"plan":"running","code":"queued"}}

stop ok: true means the API reported success. warns names the id that was never released. victim ids still in file: none is the defect: the install's state file no longer contains eph-plan-1, so the failed release cannot be retried by any later dispose() or restart.

Root cause (by symbol, as of 5b9d0d7)

• reset() in plugins/workflow/src/service.js; per-run branch defines stopOne(), whole-workflow branch defines take().
• In stopOne(): delete ended.ephemeral, lane.active = lane.active.filter(...), lane.last = ended, history row, push(..., { tone: 'bad' }), then write() — then it returns { ok: true, ids }.
• In take(): lane.active = [], lane.queue = [], lane.last = kept, push(..., { type: 'reset' }), then write() — then it returns { ok: true, ids }.
• Caller (hostSerial(async () => { … }) in both branches): const saved = take()/stopOne(), then await releaseAll(saved.ids, release, warn), then await drainAll().
• releaseAll() in plugins/workflow/src/ephemeral.js: try { await release(id) } catch { warn(...) }.
• Contrast dispose(): releaseAll(ids, release, warn).then(() => { delete run.ephemeral; …; if (shouldWrite) write() }) — release first, forget after. The comment above it is the invariant this issue asks reset() to follow.

Expected

• A failed release must not erase the id. After a Stop/Reset whose release fails, the persisted state still names the id (or the run still carries its ephemeral map), so the Bot stays reapable by the next dispose(), reload or restart.
• reset() still reports success/failure the same way, still writes exactly one state file per transition, and still surfaces the failure through warn().

Change scope (minimal, ~10 lines in reset() + one test)

• In stopOne() and take(): compute the next state (ended / kept, the filtered lane.active, lane.history, the chat line) without mutating state and without calling write(); return the ids plus that pending transition.
• In both hostSerial branches: await releaseAll(ids, …) first; then apply the pending transition and write() once; then drainAll(). On a failed release the id stays in whatever is persisted, matching dispose().
• Test: make release throw for one of the run's ids, call reset(), read the state file back, and assert that id is still present (and the run is still abortable by dispose()); the mirror case (release succeeds) asserts the id is gone after reset(). Place it with the existing ephemeral/dispose tests.

Out of scope

• dispose() releasing only the last run's Bots (fix(workflow): dispose() releases only the last run's Bots when one lane has two concurrent runs #1472, open) and the delete() path (fix(workflow): deleting a user workflow while a run is live releases no run Bot #1441/its fix): different root cause, separate records.
• #1414 (an ephemeral map that is already gone): closed, not this.
• Anything about the removed stage composer / workflow/say surface.

Acceptance

1. With a release that throws for one id, reset() (both per-run Stop and whole-workflow Reset) leaves that id in the persisted state; a following dispose() releases it, and the probe's released n/n / missing releases: none shape holds.
2. With a release that succeeds, behaviour is unchanged: id gone from the file, lane.last archived, one system line, one write.
3. plugins/workflow suite green (node --test plugins/workflow/test/*.test.js).
4. No change to endpoint names, request/response shapes, or stageMessages filtering.

Effort: 0.5 person-day.

Duplicate check

Searched open issues before filing, three queries, all as of this filing:

• Reset Stop releases Bots → 2 results: #1414 (closed) and #34 (closed); no open match.
• ephemeral release failed id erased → 1 result: #1472 (open) — dispose() releasing only the last run's Bots, different root cause.
• write before release → 51 results, of which only #1472 is workflow-release-related; #1465/#1441 are the delete()/dispose() lines.

No open issue covers reset()/take() writing before releaseAll(). #1387 (closed) is the invariant this issue cites; #1472 (open) is the dispose() collection gap.