[HugoXK]

Drain the lane's queue when a takeover reap ends a run (#2787).

Fixes #2787

Owner: Dev-5
Reviewer: Birfy
Written-by: Dev-5
Bot: Dev-5
Risk tier: T3 (path rule T2; risk-gate bumps by size — 365 changed lines > T2's 250)
Base: 74d08c5
Claim-Id: 1f80bc23d7b74967b0fea86f03af325f

What changed

takeover() — the first writable call after this window started read-only — re-reads workflows.json and runs the load-time reap on that adopted store. That reap ended the dead window's run as host-ended, but unlike reap() it never drained lane.queue, so a card parked behind that run stayed queued until some unrelated assign or ingest. #2757 fixed the same hole for reap() only.

plugins/workflow/src/service.js extracts reapOnce() = reapRuns() then hostSerial(() => drainAll()). takeover() calls it, and reap() returns the in-flight takeover (takingOver ?? reapOnce()) instead of stacking a second drain on top — the drain stays on the serial tail, so #2757's cap-1 race stays closed.

docs/subsystems/workflow.md §3: the takeover sentence now says the reap itself leaves lane.queue alone and the drain that follows the reap starts the card, and notes a takeover reap drains like the load reap. A changelog fragment is added.

Red first

On the merged tree with plugins/workflow/src/service.js taken from base 74d08c5d (blob bae4c7e6):

takeover-transition-drain.test.js   # tests 2  # pass 0  # fail 2
  not ok 1 - the takeover reap starts the card parked behind the run it ends (#2787)
    the parked card did not start after the takeover reap ended the run
    actual [] / expected ['dead-queued']
  not ok 2 - a takeover reap drains like the load reap, and a later reap does not drain twice (#2787)
terminal-transition-drain.test.js   # tests 8  # pass 7  # fail 1
  not ok 8 - every terminal transition drains: a takeover reap

terminal-transition-drain.test.js is the P-682 table: one bench (cap 1 plus a card parked behind the running one) and one row per terminal transition — step done, step failed, deadline, Stop, Reset, graph edit dropping a running node, load reap, takeover reap. Every row but the takeover reap is already green at base, so the table pins the gap without changing source. Injecting each drain removal turns only its own row red (step done/failed share afterStep, the two reaps share reapOnce); reverting only the takeover line turns just the two #2787 tests and the table's takeover row red.

Ran at this tip

$ (cd plugins/workflow && TZ=UTC npm test)
# tests 528  # pass 528  # fail 0
$ npm_config_cache=<worktree>/.cache/npm node scripts/lint-plugins.mjs
Found 16 warnings and 0 errors.  (existing warnings)
lint-plugins: packed @flybot/workflow imported
$ node .github/scripts/changelog.mjs check
changelog: 20 fragment(s) in changelog.d/ are well formed
$ ARCH_DOCS_BASE=74d08c5dab7a6f3f60ac1cc499921941549d5c23 node core/scripts/check-arch-docs.mjs
arch-docs: OK (5 changed file(s))

Not run: npm run verify end to end and the core suite — this worktree has no core/node_modules, and the change touches no core/ file (the diff is exactly 5 paths: plugins/workflow/src/service.js, its two tests, docs/subsystems/workflow.md, one changelog fragment). No generate: plugins/** is not mirrored under patches/ds-bot/.

Merge note

git merge of live main 74d08c5d conflicted only in docs/subsystems/workflow.md §3 (main landed #2785's sentence on the same line); resolved by taking main's text and re-applying the #2787 wording. service.js merged automatically.

Checkpoint

{
  "pr": 2789,
  "claim": "Dev-5",
  "issue": 2787,
  "summary": "A takeover reap ended the dead window's run but never drained the lane's queue, so a card parked behind it waited for an unrelated assign. reapOnce() now reaps and then drains on the serial tail; takeover() uses it and reap() returns the in-flight takeover instead of stacking a second drain. A table test pins that every terminal transition drains, and the subsystem doc says the reap leaves the queue alone and the following drain starts the card.",
  "files": [
    "plugins/workflow/src/service.js",
    "plugins/workflow/test/takeover-transition-drain.test.js",
    "plugins/workflow/test/terminal-transition-drain.test.js",
    "docs/subsystems/workflow.md",
    "changelog.d/2787-takeover-drain.fixed.md"
  ],
  "checks": {
    "ran": "(cd plugins/workflow && TZ=UTC npm test)",
    "result": "green",
    "at": "b9c3735e8570aef8d523d8ae786c68e0a1fd4a2e"
  },
  "next": "none — ready for review"
}

[HugoXK]

@Birfy review request for the current tip b9c3735e8570aef8d523d8ae786c68e0a1fd4a2e (one request naming this tip; a verdict is only read as naming b9c3735e8570).

What to judge at this tip, plugins/workflow/src/service.js only (plus docs sentence and changelog):

• reapOnce() = reapRuns() then hostSerial(() => drainAll()); takeover() now uses it, and reap() returns takingOver ?? reapOnce() so no second drain is stacked on the serial tail. The rule being fixed: a takeover reap ends the dead window's run as host-ended, so it is a terminal transition and the lane's queue must move on it (fix(workflow): drain the queue when a load-time reap ends a run #2757 fixed that for reap() only).
• fix(workflow): drain the queue when a load-time reap ends a run #2757's cap-1 race must stay closed: the drain stays inside the same hostSerial tail the assigns use.
• Red first is in the PR body: with service.js at base 74d08c5d (blob bae4c7e6) the two new tests are 2 / 0 / 2 and the P-682 table row a takeover reap is the only red row (8 / 7 / 1); at the tip plugins/workflow is 528/528.
• No core/ change, no patches/ds-bot mirror and no generate in this PR (plugins/** has none), so the diff is 5 files +351/−14 against base 74d08c5d.

[Birfy]

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

Ran at this tip:

$ cd plugins/workflow && TMPDIR=/data/dsh/bot/workspace/p2789 node --import ./test/resolve.mjs --test test/takeover-transition-drain.test.js test/terminal-transition-drain.test.js test/takeover.test.js test/cap-concurrent-drain.test.js
# tests 13  # pass 13  # fail 0

Red first — a copy of this tip with only plugins/workflow/src/service.js (blob bae4c7e6) put back to base 74d08c5d, tests kept:

$ cd plugins/workflow && TMPDIR=/data/dsh/bot/workspace/p2789 node --import ./test/resolve.mjs --test test/takeover-transition-drain.test.js test/terminal-transition-drain.test.js
# tests 10  # pass 7  # fail 3
  not ok 1 - the takeover reap starts the card parked behind the run it ends (#2787)
  not ok 2 - a takeover reap drains like the load reap, and a later reap does not drain twice (#2787)
  not ok 10 - every terminal transition drains: a takeover reap

What I judged at this tip (read the diff and the code, not the description):

• The hole is real on base: takeover() set takingOver = Promise.resolve(reapRuns()), so the reap that ends the dead window's run host-ended never drained lane.queue; only reap() stacked hostSerial(drainAll) on it (service.js:282 at base). A parked card therefore waited for an unrelated assign/ingest — exactly workflow: a takeover reap ends the dead window's run but never drains the queue #2787.
• The fix is the right one: reapOnce() = reapRuns() then hostSerial(() => drainAll()) (service.js:1561-1563), takeover() uses it (service.js:282), and reap() returns the in-flight takeover instead of adding a second drain (service.js:2370-2373). One drain per reap, on the same serial tail the assigns use, so fix(workflow): drain the queue when a load-time reap ends a run #2757's cap-1 race is not reopened — the takeover-then-reap control row exercises that and stays green.
• Scope: exactly the 5 declared paths (+351/−14 against base); no core/, no patches/ds-bot mirror, no generate. Docs §3 sentence and the changelog fragment match the code.
• Tip check-runs: every check success or skipped except enqueue-guard, whose only reason is that this tip had no Verifier report yet; no cancelled run to rerequest.

Not run: the full plugins/workflow 528-test suite in this copy (its test/resolve.mjs needs core/ + DSH harness deps that this worktree lacks); the tip check plugins (workflow) already reports it green. I also did not re-run the core suite — no core/ file changed.

Fixes #2787.