[HugoXK]

Fixes #2785
Owner: Dev-5
Reviewer: Birfy
Written-by: Dev-5
Bot: Dev-5
Claim-Id: c7b0191a904142ccac1611e93a33ff4f
Risk tier: T2
Base: 249b8e7
Tip SHA: 7487ec5

What

boundedSpawn (plugins/workflow/src/service.js:423) bounds the wait for a host spawn. When the host answers after the deadline with success it releases the Bots that answer created; when that late answer fails it only warned. A late failure is exactly the interesting one: spawnAll (plugins/workflow/src/ephemeral.js:60) has already put the ids it made onto the error, so error.spawned carries the Bots that now leak.

The leak was permanent, not deferred: the start is refused before beginRun(), so no run ever holds those ids in its ephemeral map, and neither reap() (lane.active / lane.last) nor orphanIds() (state.orphans) covers this path. Bots created hidden: true, ephemeral: true lived until the process exited.

The early-answer path did not leak (startLive's catch released error.spawned), so the same error object released its Bots when it arrived in time and dropped them when it arrived late.

Fix

The deadline branch now releases late.spawned on a late rejection too, then keeps the original warning for a release that itself fails:

.catch(late => releaseAll(Array.isArray(late?.spawned) ? late.spawned : [], release, warn)
  .catch(error => warn(`flybot-workflow: a late start could not release its Bots: ${error?.message ?? error}`)))

The refusal itself is unchanged: error.code === 'spawn-timeout', and releaseAll still warns once per Bot it could not remove.

Red first (on this merged tree, not the issue's base)

With plugins/workflow/src/service.js taken back to the base (effdf2f3) and the new test in place:

# tests 2
# pass 1
# fail 1
not ok 1 - #2785: a late spawn failure still releases the Bots it made
  error: the Bot made before the failed answer is released
  + actual - expected
  + []
  - [ 'eph-plan' ]

ok 2 is the control that must not regress: the same partial failure answered before the deadline releases ['eph-plan'] and clears its timer. At this tip both pass: # tests 2 / # pass 2 / # fail 0.

Ran at this tip

$ TZ=UTC node --test test/late-spawn-release.test.js   -> 2/2
$ (cd plugins/workflow && TZ=UTC npm test)             -> # tests 517 / # pass 517 / # fail 0
$ node scripts/lint-plugins.mjs                        -> 0 errors (16 pre-existing warnings), packed @flybot/workflow imported
$ node .github/scripts/changelog.mjs check             -> 15 fragment(s) well formed
$ ARCH_DOCS_BASE=effdf2f3 node core/scripts/check-arch-docs.mjs -> arch-docs: OK (4 changed file(s))

Not run: the root npm run verify end to end (it also builds the client and runs the core and three other plugin suites; nothing outside plugins/workflow was touched, and ds-bot-patches/generate are not involved: plugins/** has no patches/ds-bot/ mirror).

Docs

docs/subsystems/workflow.md said a late spawn's Bots "are released" while only the success answer released them; the sentence now names both answers and error.spawned, and the test list gains the new file.

{
  "pr": 2786,
  "claim": "Dev-5",
  "issue": 2785,
  "summary": "boundedSpawn released the Bots a late spawn created only when the host's late answer succeeded; a late failure carries the ids it already made on error.spawned and the deadline branch only warned, so those hidden run Bots were never named again and lived until the process exited. The deadline branch now releases that list too, and a release that itself fails still warns once per Bot. The refusal code is unchanged, and plugins/** has no patches/ds-bot mirror, so no generate ran.",
  "files": [
    "plugins/workflow/src/service.js",
    "plugins/workflow/test/late-spawn-release.test.js",
    "docs/subsystems/workflow.md",
    "changelog.d/2785-late-spawn-release.fixed.md"
  ],
  "checks": { "ran": "TZ=UTC node --test test/late-spawn-release.test.js; cd plugins/workflow && TZ=UTC npm test; node scripts/lint-plugins.mjs", "result": "green", "at": "7487ec5a9ce693c245d9a28beca6abcf49f50372" },
  "next": "none — ready for review"
}

[HugoXK]

@Birfy review request for the current tip 9d86a9909e59aa3f1dabaf61f161341026660b1e (tree 66ee753cd6f5fd88bc9f8626edb1e2083328298a, base effdf2f3de9036265b7a6c3cbce4dda989265874). This is the only request for this tip.

What it is: boundedSpawn (plugins/workflow/src/service.js) released the Bots a late spawn made only when the host's late answer succeeded. A late failure carries those ids on error.spawned (ephemeral.js:60) and the deadline branch only warned, so hidden run Bots were never named again — reap() walks lane.active/lane.last and orphanIds() reads state.orphans, and neither covers a start refused before beginRun(). The deadline branch now releases that list too; a release that itself fails still warns once per Bot, and the refusal (spawn-timeout) is unchanged. Docs sentence and one changelog fragment updated; plugins/** has no patches/ds-bot mirror, so no generate ran.

Checks at this tip (just read): 14 success + 1 cancelled (agent-audience, non-blocking) + 2 skipped, and enqueue-guard red for one reason only — do not enqueue #2786: no Verifier report names the tip 9d86a9909e59. tests-ok and policy-sign are green. The head lives in a fork.

Risk tier T2 (plugins/** is a T2 path row, and this change adds no diffPatterns hit), so this needs your Verdict: at this tip. If the change itself is wrong, Verdict: FAIL naming the same tip is just as useful — either way, please name the tip.

[Birfy]

Replayed this branch onto main 249b8e7922 — the pull request was mergeable_state: dirty (docs/subsystems/workflow.md conflicted with #2742's new sentence) so it could not have reached the queue at 9d86a990.

New tip: 7487ec5a9ce693c245d9a28beca6abcf49f50372 (merge commit, parents 9d86a990 + 249b8e79, fast-forward push force: false; the branch was not rewritten).

• Only hand resolution: docs/subsystems/workflow.md keeps main's Since #2742 every reply the host completes… line directly after §3, which this branch rewrote. §3 keeps the workflow: a late spawn failure leaks the Bots it already made #2785 wording. service.js, the new test and the changelog are the branch's own blobs; every other file follows main.
• Body refreshed: Base: 249b8e7922656c7183d136997cebff34a4dac031, added Tip SHA: 7487ec5a…, checkpoint checks.at → 7487ec5a….
• mergeable now true; mergeable_state: blocked (checks/review pending, no conflict).

Ran at 7487ec5a (plugins/workflow of the tip tree):

• node --test test/late-spawn-release.test.js → pass 2 / fail 0
• red first, src/service.js taken back to base 249b8e79 with the new test kept → pass 1 / fail 1, the failing one #2785: a late spawn failure still releases the Bots it made
• node --test test/*.test.js → tests 313, pass 308, fail 5

Not run: the 5 failures are all ERR_MODULE_NOT_FOUND for core/* (core/src/host/home-lock.js, core/src/shared/remote-label.js, core/src/client/themes.js, package ds-bot) in my partial checkout of plugins/workflow only — environment, not the diff; the rest of plugins/workflow and the root npm run verify need the full tree.

The previous evidence on this pull request is void at a new tip: the pull request needs a fresh Verifier report naming 7487ec5a9ce693c245d9a28beca6abcf49f50372 before the guard will let it into the queue. I have not enqueued, merged or changed any label.

Written-by: Rework

[HugoXK]

@Birfy review request for the current tip 7487ec5a9ce693c245d9a28beca6abcf49f50372 (one request per tip; this is the only one).

The tip moved because you replayed the branch onto main 249b8e7922 and resolved the docs/subsystems/workflow.md conflict — thank you, the merge is correct: service.js, late-spawn-release.test.js and the changelog stay this branch's own blobs, §3 keeps the #2785 wording, and the body now carries Base: 249b8e79… / Tip SHA: 7487ec5a… with the checkpoint's checks.at moved along.

Nothing on our side changed: we have not pushed, rebased or amended, and we will not touch the tip while it waits.

State at 7487ec5a (read now): 18 check-runs = 16 success + 2 skipped (live · Node 24 · real model, window check), mergeable: true, mergeable_state: unstable, maintainer_can_modify: true, 2 commits, 4 files +138/−4. The only red is enqueue-guard itself, whose single reason is no Verifier report names the tip 7487ec5a.

So this PR needs one Verdict: comment naming 7487ec5a9ce693c245d9a28beca6abcf49f50372 for the guard to let it into the queue, as you wrote in your rework note. Verdict: PASS or Verdict: FAIL both unblock us — a FAIL at this tip tells us what to fix instead of leaving the PR idle.

[Birfy]

Verdict: PASS at 7487ec5
Written-by: Reviewer
Attestation: 7487ec5 role=Reviewer

Ran at this tip:

$ node --test test/late-spawn-release.test.js
# tests 2  # pass 2  # fail 0
$ cp <base 249b8e79 service.js> src/service.js && node --test test/late-spawn-release.test.js   # red first: source at base, new test kept
# tests 2  # pass 1  # fail 1
$ node --test $(ls test/*.test.js | grep -v '^test/host.test.js$')
# tests 516  # pass 516  # fail 0
$ node --test test/host.test.js
# fail 1  ERR_MODULE_NOT_FOUND: Cannot find package 'ds-bot'

Not run:

• npm --prefix core test, other plugin suites, lint/build and the live/mock jobs — my checkout is a tarball without node_modules, and the diff touches only plugins/workflow + its docs; I did not install the workspace.
• test/host.test.js is red here for the missing ds-bot workspace package before any test body runs. That file is untouched by this diff (the diff is exactly 4 files), so this is my checkout, not the change; the same file needs the installed workspace to run.

This report judges head 7487ec5a…, not 9d86a990…. Rework replayed the branch onto main 249b8e79 (merge commit, 9d86a990 + 249b8e79) at 06:55 because 9d86a990 was dirty against main and could never have reached the queue; 9d86a990 is no longer a head.

What I checked, independently of the body:

• The defect is real on base. In base service.js the deadline branch's late-answer handler was a bare .catch(...) that only warned: a late success released its ids, a late failure did not. spawnAll (src/ephemeral.js:60-61) sets error.spawned = made before throwing, and startLive's catch releases error.spawned only when the answer arrives before the deadline — so the ids of a late failure were named to nobody and leaked until process exit. My red run reproduces exactly that: with base src/service.js and the new test in place, released is [] where ['eph-plan'] is required (late-spawn-release.test.js:115).
• The change fixes that path and only that path. service.js:429-438: the late rejection now runs releaseAll(late.spawned) and keeps the warning for a release that itself fails (the warning moved inside the inner .catch). Array.isArray(late?.spawned) guards a plain error; a success still releases via Object.values(ids); the refusal stays spawn-timeout and startLive's catch still releases error.spawned on the early path, so no id is released twice. releaseAll already warns once per Bot it cannot remove.
• Tests. The new file's first test is the fail-without-change case (red on base, green here) and its control pins the early failure path plus timer clearing, so the fix cannot pass by releasing on both paths blindly.
• Scope. GET /pulls/2786/files and the diff carry exactly 4 files: the fix, its test, the changelog fragment changelog.d/2785-late-spawn-release.fixed.md, and the docs/subsystems/workflow.md paragraph that documents this path (plus the Tests section listing the new test). No policy/, .github/scripts, .github/workflows or patches/ds-bot/ hunk.
• Fixes #2785 is in the body.

Not enqueued by this comment; the guard needs this report first.