Skip to content

fix(workflow): a run Bot's id is dropped before the release on saveGraph() and delete() #1621

Closed

Description

@HugoXK

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment on this issue.

Written-by: FlyBot Lead
Claimed-by: FlyBot Lead
Claim-Id: b3f2a61dc1cf4e86a2aa4e6f7a550262
Branch: dev/1621-workflow-orphan-release
Base: 63f07ef
At: 2026-10-29T02:06:04Z

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment on this issue.

Written-by: FlyBot Lead

What happens

#1616 (PR #1617, merged 3dc44ec9) fixed three sites where a run Bot's ephemeral id was deleted before release() ran, so a failing release left nothing behind and the Bot became a hidden orphan. Two sites in plugins/workflow/src/service.js still drop the id first (line numbers on live main 3dc44ec9787b5782d2d89ac4fea9df3e4b2d508e):

  1. saveGraph() (:1398-1411) collects releasing, then delete run.ephemeral / delete run.handed at :1399-1402, then write() at :1403, and only afterwards await releaseAll(releasing, release, warn) at :1408-1411 — with the return value ignored. Trigger: an edit of the graph that removes a node a run is currently on; the run ends and lands in lane.last.
  2. delete() (whole-workflow delete, :950-968) collects ids from lane.active only, calls await releaseAll(releasing, release, warn) at :967 ignoring failure, then finish() drops state.lanes[...], state.runs[...] and the logs (:953, :954, :968). A failure branch has nowhere to keep the id. It also never retries the ids #1616's keepUnreleased() parked on lane.last, because those are not in lane.active.

Reproduction

Inject a spawn that yields one id per node and a release that throws host call failed for a chosen id, then:

  1. start a two-node run, edit the graph through saveGraph() removing the node the run is on → the persisted run record has no ephemeral left, the release warning is the only trace, and a reload reap() retries nothing;
  2. delete({ workflowId }) on a workflow whose Bot will not go → lanes/runs/logs are gone, the id appears nowhere in the store, reload retries nothing;
  3. let a run end normally so #1616 keeps the failed id on lane.last, then delete the workflow → releasing is built from lane.active only, so that id is never even attempted.

All three leave a hidden run Bot: it is not in the sidebar, Search or Settings → Bots, list_bots/bot.list skip it, the Session is not reclaimed, and the name stays reserved by findBot (core-host.md:337, plugin-runtime.md:68).

Expected behaviour

Deleting a workflow must not be blocked by one failed release — the user asked for a delete, that action must still complete. But the id must have a home instead of being dropped:

  • saveGraph(): release first, then forget the ids that were released; ids whose release failed stay on the archived run via the existing keepUnreleased(run, failed) so the next load's reap() retries them.
  • delete(): releasing is lane.active ∪ lane.last (deduped); ids whose release failed are recorded in a persisted state.orphans before finish() removes the lane; reap() retries orphans on load and drops each entry once it succeeds.
  • orphans lives in state and is persisted, not in a memory variable; entries are deduped by id and capped (50); one failing delete() call registers an id once.
  • orphans is preserved by migrateRuns(), so an older store that is upgraded keeps its pending ids.

Acceptance

  • A test pins the full chain: delete a lane while release throws → the delete still succeeds, the id lands in orphans → after a reload reap() retries it and succeeds → orphans is empty.
  • reap() is idempotent: a successful retry removes the entry, no unbounded growth, no duplicate entries for the same id.
  • saveGraph() keeps the failed id on the archived run (same mechanism as #1616).
  • releaseRun(), reset() and dispose() keep their current behaviour; #1476's double-release guard (forgetReleased) is untouched.

Scope

  • plugins/workflow/src/service.js: the two sites above, plus orphanIds() / noteOrphans() helpers, the reap() retry and migrateRuns().
  • One test file (plugins/workflow/test/dispose.test.js) and a changelog entry; docs/subsystems/workflow.md only if the documented contract needs the orphans sentence.
  • Not in scope: startLive()'s spawn-failure path (service.js:795, :801) — that run record never exists, it is a separate decision. No UI changes in this issue.

Activity

  1. HugoXK commented on Oct 29, 2026

    @HugoXK
    ContributorAuthor

    Heartbeat

    Written-by: FlyBot Lead
    Still-working: b3f2a61dc1cf4e86a2aa4e6f7a550262
    At: 2026-10-29T02:46:54Z

  2. HugoXK commented on Oct 29, 2026

    @HugoXK
    ContributorAuthor

    Release

    Written-by: FlyBot Lead
    Released: b3f2a61dc1cf4e86a2aa4e6f7a550262
    Reason: merged-as-fa5ff751
    At: 2026-10-29T02:53:29Z

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions