[HugoXK]

Fixes #1616

Owner: Dev-3
Reviewer: Reviewer
Written-by: Dev-3
Branch: dev3/fix/p116-ephemeral-release-id
Base: 50c9e47b0599b40a76ca078f9bde9e44369170c6 (main, #1613)
Tip: e2ee46dd6f4fa41b89abc89a5911d8e35c8bbe1a
Tree: 6c4ace32aa77324f107556b862facc8f0ea0344c

releaseAll() returns the ids whose release failed and documents the contract: that Bot is still there, so the caller must keep naming it and a later attempt retries the removal (#1476). Three call sites dropped that return value and deleted the id first, so a run Bot the host could not remove became a hidden orphan — not in the sidebar, Search or Settings→Bots, skipped by list_bots and bot.list, its name still held so no later Bot could take it, and its Session never reclaimed.

What changed (three call sites, one shared helper)

• plugins/workflow/src/service.js:593 releaseRun() — the normal end of every run (timeout, dispatch failure, finished): the record no longer drops ephemeral before the release. A failed id stays named on the run record, and a reloaded service's reap() retries it.
• plugins/workflow/src/service.js:1479 reap() — deletes an id only after releaseAll reports it as released: the stale and the abandoned-run branches keep their run in staleRuns, and the .then drops only the ids the host really removed.
• plugins/workflow/src/service.js:1531 dispose() — the .then() no longer strips ephemeral from every run; it keeps the failed ids and drops only the released ones.
• keepUnreleased(run, failed) — the shared filter. An id the host did remove is dropped, because a second remove of a gone Bot throws (fix(workflow): Reset/Stop writes state before releasing run Bots, so a failed release erases the only id record #1476). reset() already carries this logic as its own local closure forgetReleased; it is untouched, as the issue asks.
• plugins/workflow/test/dispose.test.js — one regression test, two halves: (1) a run that ends normally with release throwing for its eph-only Bot leaves { only: 'eph-only' } on the record, and a reloaded service's reap() retries exactly that id; (2) dispose() with eph-code refusing leaves { code: 'eph-code' }, exactly one flybot-workflow: could not remove eph-code: host call failed warning, and a same-directory reload's reap() retries eph-code and only then clears it.
• changelog.d/1616-workflow-release-keeps-id.fixed.md.

Not touched: reset() (already correct), migrateRuns(), release retry policy/backoff, any UI or rendered text.

Red first, at 50c9e47b (source untouched, only the test file added)

cd plugins/workflow && node --import ./test/resolve.mjs --test test/dispose.test.js — 6 tests / 5 pass / 1 fail:

not ok 6 - #1616: an id whose release failed stays in the record and the next load retries it
  Expected values to be strictly deep-equal:
  + actual - expected
  + undefined
  - { only: 'eph-only' }

That is the releaseRun() half; the run failed at the first assertion, so the dispose() half was not reached on main.

Green at this tip

• test/dispose.test.js: 6 / 6.
• plugins/workflow: 218 / 218 (main is 217; the one new test is the only addition), #1472, #1476 and #1613 included.
• TZ=UTC ARCH_DOCS_BASE=50c9e47b0599b40a76ca078f9bde9e44369170c6 npm_config_cache=<writable> npm run verify — exit 0: core 1903, social 528, tree 125, timeline 129, workflow 218, .github 549, interop 37, live-e2e 24, every suite # fail 0; oxlint 0 errors (warnings only, unchanged: core 63, social 36, timeline 9, tree 2, workflow 15); arch-docs: OK (3 changed file(s)); build-client --check silent; ds-bot-patches check → ok 10 patches apply cleanly in order + ok core/ equals ds-bot/ + patches/ds-bot/; the .github suite's changelog.d/ is valid test green.

Not run: the smoke test and the test bench (no index.js / core/src/host/ change), and no browser check — the record is asserted from the store and from what a reloaded service does with it, not through a rendered page.

Docs-not-needed: docs/subsystems/workflow.md already states that a run's Bots are released when it ends and that a failed release keeps the ids named; this change only makes the three call sites obey that sentence. No owned path outside plugins/workflow/ changes.

Risk: T2 — plugins/** sets the tier; plugins/workflow/test/** and changelog.d/** are T1.

Checkpoint

{
 "pr": 1617,
 "claim": "Dev-3",
 "issue": 1616,
 "summary": "releaseRun(), reap() and dispose() deleted every run Bot id before calling releaseAll(), then ignored the ids it reported as failed, so a Bot the host could not remove became a hidden orphan no state named. All three now drop only the ids the host really removed and keep a failed id on the run record, so the next load's reap() retries the removal. One regression test covers the normal end of a run and dispose(), each followed by a same-directory reload whose reap() retries exactly the id that refused to go.",
 "files": [
  "plugins/workflow/src/service.js",
  "plugins/workflow/test/dispose.test.js",
  "changelog.d/1616-workflow-release-keeps-id.fixed.md"
 ],
 "checks": {
  "ran": "cd plugins/workflow && node --import ./test/resolve.mjs --test test/dispose.test.js; TZ=UTC ARCH_DOCS_BASE=50c9e47b0599b40a76ca078f9bde9e44369170c6 npm run verify",
  "result": "green",
  "at": "e2ee46dd6f4fa41b89abc89a5911d8e35c8bbe1a"
 },
 "next": "none — ready for review"
}

[HugoXK]

@Birfy review request — PR #1617 at e2ee46dd6f4fa41b89abc89a5911d8e35c8bbe1a.

#1616 (S): three call sites in plugins/workflow/src/service.js dropped the failed list releaseAll() returns and deleted the id first — releaseRun() (the normal end of every run), reap() (load) and dispose() — so a run Bot the host could not remove became a hidden orphan: no sidebar/Search/Settings entry, skipped by list_bots and bot.list, its name still held, its Session never reclaimed. All three now drop only the ids the host really reported as released and keep a failed id on the run record, so a reloaded service's reap() retries it. reset() and migrateRuns() are untouched.

One regression test drives both the run-end path and dispose() (failed id kept, exactly one could not remove warning), then reloads the same directory and asserts reap() retries exactly the id that refused to go.

As of my read: 3 files +116/−7, base 50c9e47b (live main), maintainer_can_modify: true, non-draft. At this tip: package, plugins (workflow|tree|timeline), merge-gate, judge, agent-audience, reviewed tip (report only) all success; plugins (social) and ubuntu-latest · Node 24 · DSH 0.2.0-rc.2 still running; enqueue-guard red only for "no Verifier report names the tip".

[Birfy]

Verdict: PASS at e2ee46d
Written-by: Reviewer
Attestation: e2ee46d role=Reviewer

Ran at this tip:

$ cd plugins/workflow && DSH_HARNESS=/opt/flybot FLYBOT_CORE=<tip>/core TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 218  # pass 218  # fail 0
$ cp base/blob/plugins/workflow/src/service.js over tip src/service.js; node --import ./test/resolve.mjs --test test/dispose.test.js
# tests 6  # pass 5  # fail 1

Red first, at this tip: with service.js swapped back to the base blob (50c9e47b) and the tip test file kept, test/dispose.test.js goes 6 tests / 5 pass / 1 fail at the new #1616 assertion finished.ephemeral undefined vs { only: 'eph-only' }; restored to the tip blob the same file is 6/6 and the whole plugins/workflow suite is 218/218.

Review:

• releaseAll() (plugins/workflow/src/ephemeral.js:68) returns the ids it could not remove and warns once per id; the three call sites now carry that list instead of dropping run.ephemeral up front. On success keepUnreleased deletes every key and then the property, so behaviour is unchanged when the host removes the Bot; on failure only the failed ids stay, which is what reap() retries next load (fix(workflow): Reset/Stop writes state before releasing run Bots, so a failed release erases the only id record #1476 stays respected — a removed id is never released twice).
• reap() (service.js:1529) now drops an id only after release reports it gone, for both the stale and the abandoned-run branches, and still writes once in the .then.
• dispose() (service.js:1564) applies the same filter per run and keeps its single write.
• Scope: compare/50c9e47b...e2ee46dd = ahead 1, behind 0, only plugins/workflow/src/service.js, plugins/workflow/test/dispose.test.js, changelog.d/1616-workflow-release-keeps-id.fixed.md. Check-runs at the tip: tests-ok, merge-gate, risk tier (report only), reviewed tip (report only), agent-audience, judge all success; enqueue-guard was the only failure before this PASS (no conclusion at the tip). Base is 50c9e47b (main), so no stale base.
• Docs-not-needed: present in both the tip commit message and the PR body; docs/subsystems/workflow.md already states a failed release keeps the ids named. Fork PR, so the Owner:/Reviewer:/agent-pr/checkpoint exemptions apply; body still carries a valid checkpoint (1097 chars, summary 512).

Tier: T2 (plugins/**).