From cfc384647b68aea8adc74f16b213e5405fc55061 Mon Sep 17 00:00:00 2001 From: Dev-3 Date: Tue, 10 Nov 2026 04:16:11 +0000 Subject: [PATCH 1/3] workflow: end a run whose host call never answers (#2748) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #2748 Owner: Dev-3 Reviewer: Birfy Written-by: Dev-3 Claim-Id: 9df438ed10ac478b8d037965974377ec Base: f2ef4dfaccedd40315bd585474d0902118dc0ba8 Risk tier: T2 ## The defect The step deadline is the one thing that bounds every wait in a live run, and `expireStep()` ran the whole expiry inside `hostSerial`. The serial tail is exactly where a host call that never answers sits: `dispatch`, `release` and `spawn` are each awaited there. So a host that never answered parked the tail for ever, the timer that ends the run fired and queued behind it, and the run stayed `running` with its Bots still held and its lane slot never freed. On a Bot start the card was worse off: `takeNext()` had already shifted it out of `lane.queue`, so a mute host left the card neither running nor queued. ## The change - `plugins/workflow/src/service.js` — `expireStep()` no longer runs inside `hostSerial` (`return hostSerial(async () => {…})` becomes `return (async () => {…})()`), the one-line shape the issue names; the body still ends with `await drainAll()`. A deadline can no longer be queued behind the wait it bounds, and a dispatch or release that never answers leaves the run reaching `failed` with its Bots released and its timers cleared. - `SPAWN_DEADLINE_MS` (60 s) and `boundedSpawn()`: a start whose Bots the host does not create within that bound is refused with `code: 'spawn-timeout'`, and a late answer's Bots are released instead of leaked. `startLive()` wraps `spawnAll(…)` in it, so `drainAll()` takes the existing `started.ok === false` path and unshifts the card it had taken back to `lane.queue`. - `plugins/workflow/test/tail-hang.test.js` — three probes on a fake clock: C1 a dispatch that never settles, C2 another workflow's release that never settles, C4 a spawn that never answers. C1/C2 assert the run and node end `failed` with Bots released and no timer left; C4 asserts the call answers, the queue still holds the card behind it, and the lane is idle. - `docs/subsystems/workflow.md` — the deadline paragraph states that the expiry is not queued behind the wait it bounds, and the start bound; the test list names the three hangs. - `changelog.d/2748-workflow-stuck-run.fixed.md` — the user-visible entry. ## Red first At the live base `f2ef4dfa` with only the test file added, twice in a row with the same shape: `# tests 3 / # pass 0 / # fail 3`. not ok 1 - #2748 C1: a dispatch that never settles still reaches failed at its step deadline not ok 2 - #2748 C2: a release that never settles does not starve another workflow's deadline not ok 3 - #2748 C4: a spawn that never answers keeps the queued card instead of losing it C1/C2 read `expected: 'failed' / actual: 'running'`; C4 reads `the call answers instead of hanging for ever` — `false !== true`. ## Checks at the tip - the same file with the fix: `# tests 3 / # pass 3 / # fail 0`, twice, with PROBE lines C1 `status: failed node: failed active: 0 released: 2 timersLeft: 0`, C2 `alpha: failed bravo before: running after its deadline: failed bravo node: failed released: 2 timersLeft: 0`, C4 `settled: true queue: 1 active: 0 card: #2748-C4b`. - the whole `plugins/workflow` suite: `# tests 511 / # pass 511 / # fail 0`. - `node .github/scripts/changelog.mjs check`: fragments well formed. Not run: root `npm run verify` (this clone has no `core/node_modules`; the plugin suite ran with `DSH_HARNESS` pointing at the packaged DSH), the core suite, and the smoke bench — the diff touches no host route, turn or delivery path. Reviews the ordering fact honestly: `plugins/workflow/src/service.js` carried two other open pull requests (#2753, #2757) while this was prepared; live main was re-read at `f2ef4dfa` before this branch was cut and every reading above was taken on it. --- changelog.d/2748-workflow-stuck-run.fixed.md | 1 + docs/subsystems/workflow.md | 4 +- plugins/workflow/src/service.js | 48 ++++- plugins/workflow/test/tail-hang.test.js | 205 +++++++++++++++++++ 4 files changed, 252 insertions(+), 6 deletions(-) create mode 100644 changelog.d/2748-workflow-stuck-run.fixed.md create mode 100644 plugins/workflow/test/tail-hang.test.js diff --git a/changelog.d/2748-workflow-stuck-run.fixed.md b/changelog.d/2748-workflow-stuck-run.fixed.md new file mode 100644 index 000000000..81625d457 --- /dev/null +++ b/changelog.d/2748-workflow-stuck-run.fixed.md @@ -0,0 +1 @@ +A Workflow run whose host call never answered no longer stays running for ever: a step whose deadline passes is ended as failed even while a dispatch, a release or a Bot start is still waiting, with its Bots released. A start whose Bots the host does not create within 60 seconds is refused, and a queued card a drain had already taken out of the queue goes back to it instead of disappearing. ([#2748](https://github.com/ThinkFlowLab/FlyBot/issues/2748)) diff --git a/docs/subsystems/workflow.md b/docs/subsystems/workflow.md index c857e0279..6f4c27376 100644 --- a/docs/subsystems/workflow.md +++ b/docs/subsystems/workflow.md @@ -59,7 +59,7 @@ Everything in `plugins/workflow/`: the graph rules, the run store, the matcher, 1. Friends' Bots are not nodes. A `remote` Bot is refused by create, define and the matcher. 2. The canvas and the run share one graph: nodes, edges and positions. A drag commits `workflow/graph`, but only while **Edit** is on — a factory preset is copied first (#1378). A wire or a reconnect that would list a pair the graph already carries commits nothing new, because `normalizeGraph` refuses a graph that lists one pair twice and then saves none of it: unguarded, the plane would show no edge, no change and only a save error (#2163). Status highlighting reads that same graph. Replacing edges does not recompute who is already running. The Settings canvas paints that graph on the same 960px design plane, scaled to the width the settings page gives it, so the canvas is not a second scroll container inside the page's own scroll; its pointer deltas are divided by the scale, and its wires keep a stronger stroke than a plain divider so an edge can be followed at that size (#1483). While another dsh holds the Bot team the host is read-only (`roster.readOnly`): the page still reads through `workflow/state`, keeps its controls disabled, and offers no **Edit**, **Add link** or wire port, so it cannot take a change that core would refuse with `bot/read-only` and then drop (#2187). The same window draws the sidebar's workflow list without **New**, the row's **Rename**/**Delete** or **Hand a task to a Workflow**: the list is read-only too, and an `roster.readOnly` that is not a string, or holds only whitespace, counts as writable — trimmed the way `core/src/client/workflows.js:570` trims it (#2200). The Stage a row opens carries the same lock: it prints the host's sentence under `role="status"` and draws no **Hand a task to a Workflow**, no **Re-match**, no **Pause**/**Resume**/**Stop**, no **Edit**/drag palette and no **Reset**, while the row click, **Progress**, **Flow** and viewing a run still work; a writable roster draws every control exactly as before (#2213). Mounting the list in a read-only window still reads `projects` so the Project filter fills, but it sends no `workflow/ingest` for the open cards: ingest is a claim/match write the host refuses, so a reader opening the list writes nothing; a writable window still ingests every open card (#2222). The read-only sentence can also land after the list has mounted — a cold window starts on the empty roster, `ready: false` and no `readOnly` — so the ingest step asks the value the latest render saw rather than the one the mount saw, and it skips the board read and stops between two cards once the window has turned read-only. The Settings page draws no read-only sentence of its own: the host's Settings header already prints one for every page it shows (`core/src/client/settings.js` `bt-read-only`, `role="status"`, `hostText(roster.readOnly)`), and a second copy inside the page said the same thing twice (#2202). -3. **A live run advances only when its ephemeral Bot replies, following the graph's edges in order.** That reply advances the run and may appear on the workflow stage log; it is not delivered into a persistent sidebar Bot conversation (for example Chief). The `workflow-step` send middleware denies those replies after `stepResult` runs. Nodes start only when their forward predecessors finish and edges reach them. **Early termination (#1537, #1538, #1546, #1570, #1571):** When a node completes **or fails**, the run ends immediately with status `done`, sets `run.earlyEnd` to the condition name, and does not advance to successors, take a success-path `loop`, or burn `onFail` recovery attempts. The two markers that report the task itself end it only on a structured signal (#1546): `needs-human` and `already-claimed` must be a declared OUTPUT whose value **equals** the marker, or a whole reply line that is exactly `EARLY-END ` in any case. A step that quotes an issue title, negates the label or reports it absent keeps running. `tip-unchanged` (#1570) and `open-pr` (#1571) accept that same structured signal — in a declared OUTPUT the value counts as a whole word, so a brief that names `data-open-project` is not `open-pr` (#1866) — and also honor their prose fallbacks (`tip unchanged` / `unchanged tip`, `open PR already` / `already has an open PR` / `has an open PR` / `found an open PR`), so a Bot that forgets the structured marker cannot soft-lock Claim↔Verify on the same tip. See `checkEarlyEnd` in Key files. Factory prompt guidance lives in `EARLY_END_GUIDANCE` (`plugins/workflow/src/template.js`) for Settings graphs that wire Triage / Claim / Verify: Verify must escalate once on an unchanged tip, not re-run the full suite. There is no scripted or seeded run path: `workflow/state` exposes only `live === true` runs — the running ones, plus the last run when it is still `live` and ended `failed` — a failed sibling stays on the stage beside whatever is running now, with `run.failure` (`{ code, at, nodeName?, text? }`) naming what ended it, so the room does not have to open Runs to learn that a run failed (#1493, #1494). The failed sibling is left out when a running run carries the same task id: that is a retry, and the live run keeps the row (#1507). History entries carry the run `id` and the same `failure`, so a Runs row shows when the run ended and why, and a row for a run already on the stage is not listed twice. A run does not create a schedule. `scheduleLabel` is display text. When the host can create Bots, starting a live run (`assign`, `run_workflow`, `ingest`) spawns one ephemeral Bot per node from that node's prompt, role, model and tools, sends it the step from the Main Bot, and removes every instance when the run finishes, fails for good, or is reset. A `failOnce` retry keeps them. They are hidden and left out of the sidebar. Loading the plugin does not spawn Bots for the default workflows. Plugin unload (`dispose`) and the load-time `reap` both end a live run still marked `running`: in-flight nodes are failed, the run is archived as `failed`, and any ephemeral Bot ids are released. A **read-only** host — a second window on the same `DSH_HOME`, the reader `home-lock` describes — does neither: it never ran those runs, so it cannot tell which of them a dead host left behind, and it must not end them, release their step Bots or write its older copy over the writer's store; both calls return without touching the record and the writer's own `reap` decides (#2673). **Taking the lock over is a state change (#2704):** the first call that finds the host writable after this window started read-only — a read, `reap`, `dispose` or any write — re-reads `workflows.json` once before it changes anything, because the copy it loaded predates what the dead window saved, and then runs the same load-time `reap` once on that adopted store. That reap ends the dead window's `running` run as `host-ended` (or heals a `live:true` run whose Bot map is already gone, #1414) and releases its step Bots; `lane.queue` is never touched, so a card that was merely `queued` keeps its place and starts when the lane drains. Only the first such call re-reads — a writer that later loses the lock is still refused, exactly as before. A run already written as `running` with no ephemeral map (an older unload) is healed the same way on the next load (#1414). Loading is also the one moment a run the host left behind is noticed when its ephemeral map is still present: `reap` releases those instances and records the run as `failed`. Nothing resumes it. A run that already ended keeps its own status (#1388). `workflow/reset` stopping one live run ends it the same way before its Bots are released: nodes still `running` become `failed`, so the strip cannot keep painting a run that is over (#1424). A graph edit that drops a node that is still running ends that run the way Stop does: the node is failed, the run is archived `failed`, its node Bots are released, and the queue drains (#1549); an id whose release failed stays named on that archived run, and the next load's `reap` retries it, instead of the edit forgetting it (the #1616 contract). Every wait is bounded (#1530). A dispatch that does not land is not marked handed: the chat names the code and the attempt, and it is retried after a short backoff at most twice, never twice inside one hand-off, so a 429 is not hammered. When it still does not land, the step and the run fail with the reason written into the chat. Each running live step also holds a deadline — its workflow's `stepTimeoutMinutes`, `DEFAULT_STEP_TIMEOUT_MINUTES` (45 minutes) unless the record sets one, an integer from 1 to 120 given at create, through `define_workflow`, or through `workflow/step-timeout` (armed even when the host cannot spawn ephemeral Bots; #1530, #1580); when it expires the node and the run fail as `failed`, the run's ephemeral Bots are released, the chat and `run.failure` say the step's deadline was reached (`step deadline reached (45 min)`, the number its lane set), and `warn` names it once; the line never reads as a reply that did not arrive, because a step still working is killed by the same timer (#1868). The deadline is cleared as soon as the step moves, and by Pause, Reset, delete, `reap`, `dispose` and a Stop that released every Bot; a run the owner paused holds none, and Resume arms it again from that moment. A timer never outlives the run that started it. A reply that matches no running step — its run ended, or the node already moved — is reported through `warn` once per Bot and run instead of being dropped silently. A reply fails that node on a structured signal only: the host's failure notice carries `failure: true`, which the middleware hands `stepResult` as `ok: false` (a caller-set `ok: false` counts too), so an answer that quotes `could not answer your message` or `model call failed` while auditing it advances the step (#2677). An old host without the marker falls back to the notice shapes at the start of the text only (` could not answer your message: …`, ` was stopped before it could answer your message.`, and the bare `could not answer your message` / `The model call failed.` an old host wrote), never a free substring, so an ordinary answer that merely ends in the model-failure wording keeps advancing the step (#2741). A stopped turn is a failed step, with the stop notice as its reason, instead of finishing as done and handing that notice to the next node (#2741). A 429 or rate-limit mention in any other reply does not fail a step. Stop and Reset (`workflow/reset`) follow the same order as `dispose`: they release the run's Bots before the record forgets their ids, and an id whose release failed stays named on the still-live run, so the next Stop, Reset, dispose or load-time `reap` retries it (#1476). A run left that way is marked `paused` and `releasePending` and shows as Paused: it hands off no further node, `resume` is refused with an error, and Stop again is the retry. It also keeps the deadline of the node it was on — the one paused state that holds one — and a graph save re-syncs timers without taking it away (a save that cleared it left the run holding its slot with nothing left to free it); the deadline releases again there, ends the run as `paused-expired` and frees the slot, re-arming itself while a Bot still will not go (#2453, P-560). A whole-workflow Reset that could not release every Bot freezes every run it did not reset, because each of them has lost the ids whose release did go. The markers go away when the run ends: Stop again, `dispose`, `reap`, or its own completion. +3. **A live run advances only when its ephemeral Bot replies, following the graph's edges in order.** That reply advances the run and may appear on the workflow stage log; it is not delivered into a persistent sidebar Bot conversation (for example Chief). The `workflow-step` send middleware denies those replies after `stepResult` runs. Nodes start only when their forward predecessors finish and edges reach them. **Early termination (#1537, #1538, #1546, #1570, #1571):** When a node completes **or fails**, the run ends immediately with status `done`, sets `run.earlyEnd` to the condition name, and does not advance to successors, take a success-path `loop`, or burn `onFail` recovery attempts. The two markers that report the task itself end it only on a structured signal (#1546): `needs-human` and `already-claimed` must be a declared OUTPUT whose value **equals** the marker, or a whole reply line that is exactly `EARLY-END ` in any case. A step that quotes an issue title, negates the label or reports it absent keeps running. `tip-unchanged` (#1570) and `open-pr` (#1571) accept that same structured signal — in a declared OUTPUT the value counts as a whole word, so a brief that names `data-open-project` is not `open-pr` (#1866) — and also honor their prose fallbacks (`tip unchanged` / `unchanged tip`, `open PR already` / `already has an open PR` / `has an open PR` / `found an open PR`), so a Bot that forgets the structured marker cannot soft-lock Claim↔Verify on the same tip. See `checkEarlyEnd` in Key files. Factory prompt guidance lives in `EARLY_END_GUIDANCE` (`plugins/workflow/src/template.js`) for Settings graphs that wire Triage / Claim / Verify: Verify must escalate once on an unchanged tip, not re-run the full suite. There is no scripted or seeded run path: `workflow/state` exposes only `live === true` runs — the running ones, plus the last run when it is still `live` and ended `failed` — a failed sibling stays on the stage beside whatever is running now, with `run.failure` (`{ code, at, nodeName?, text? }`) naming what ended it, so the room does not have to open Runs to learn that a run failed (#1493, #1494). The failed sibling is left out when a running run carries the same task id: that is a retry, and the live run keeps the row (#1507). History entries carry the run `id` and the same `failure`, so a Runs row shows when the run ended and why, and a row for a run already on the stage is not listed twice. A run does not create a schedule. `scheduleLabel` is display text. When the host can create Bots, starting a live run (`assign`, `run_workflow`, `ingest`) spawns one ephemeral Bot per node from that node's prompt, role, model and tools, sends it the step from the Main Bot, and removes every instance when the run finishes, fails for good, or is reset. A `failOnce` retry keeps them. They are hidden and left out of the sidebar. Loading the plugin does not spawn Bots for the default workflows. Plugin unload (`dispose`) and the load-time `reap` both end a live run still marked `running`: in-flight nodes are failed, the run is archived as `failed`, and any ephemeral Bot ids are released. A **read-only** host — a second window on the same `DSH_HOME`, the reader `home-lock` describes — does neither: it never ran those runs, so it cannot tell which of them a dead host left behind, and it must not end them, release their step Bots or write its older copy over the writer's store; both calls return without touching the record and the writer's own `reap` decides (#2673). **Taking the lock over is a state change (#2704):** the first call that finds the host writable after this window started read-only — a read, `reap`, `dispose` or any write — re-reads `workflows.json` once before it changes anything, because the copy it loaded predates what the dead window saved, and then runs the same load-time `reap` once on that adopted store. That reap ends the dead window's `running` run as `host-ended` (or heals a `live:true` run whose Bot map is already gone, #1414) and releases its step Bots; `lane.queue` is never touched, so a card that was merely `queued` keeps its place and starts when the lane drains. Only the first such call re-reads — a writer that later loses the lock is still refused, exactly as before. A run already written as `running` with no ephemeral map (an older unload) is healed the same way on the next load (#1414). Loading is also the one moment a run the host left behind is noticed when its ephemeral map is still present: `reap` releases those instances and records the run as `failed`. Nothing resumes it. A run that already ended keeps its own status (#1388). `workflow/reset` stopping one live run ends it the same way before its Bots are released: nodes still `running` become `failed`, so the strip cannot keep painting a run that is over (#1424). A graph edit that drops a node that is still running ends that run the way Stop does: the node is failed, the run is archived `failed`, its node Bots are released, and the queue drains (#1549); an id whose release failed stays named on that archived run, and the next load's `reap` retries it, instead of the edit forgetting it (the #1616 contract). Every wait is bounded (#1530). The deadline is not queued behind the wait it bounds (#2748): an expiry runs off the serial tail, so a host call that never answers — a dispatch, a release, a spawn — cannot park the one timer that ends the run, and the run still reaches `failed` with its Bots released. A start whose Bots the host does not create within `SPAWN_DEADLINE_MS` (60 s) is refused, and the card a drain had already taken out of `lane.queue` goes back to it instead of vanishing (a late spawn's Bots are released). A dispatch that does not land is not marked handed: the chat names the code and the attempt, and it is retried after a short backoff at most twice, never twice inside one hand-off, so a 429 is not hammered. When it still does not land, the step and the run fail with the reason written into the chat. Each running live step also holds a deadline — its workflow's `stepTimeoutMinutes`, `DEFAULT_STEP_TIMEOUT_MINUTES` (45 minutes) unless the record sets one, an integer from 1 to 120 given at create, through `define_workflow`, or through `workflow/step-timeout` (armed even when the host cannot spawn ephemeral Bots; #1530, #1580); when it expires the node and the run fail as `failed`, the run's ephemeral Bots are released, the chat and `run.failure` say the step's deadline was reached (`step deadline reached (45 min)`, the number its lane set), and `warn` names it once; the line never reads as a reply that did not arrive, because a step still working is killed by the same timer (#1868). The deadline is cleared as soon as the step moves, and by Pause, Reset, delete, `reap`, `dispose` and a Stop that released every Bot; a run the owner paused holds none, and Resume arms it again from that moment. A timer never outlives the run that started it. A reply that matches no running step — its run ended, or the node already moved — is reported through `warn` once per Bot and run instead of being dropped silently. A reply fails that node on a structured signal only: the host's failure notice carries `failure: true`, which the middleware hands `stepResult` as `ok: false` (a caller-set `ok: false` counts too), so an answer that quotes `could not answer your message` or `model call failed` while auditing it advances the step (#2677). An old host without the marker falls back to the notice shapes at the start of the text only (` could not answer your message: …`, ` was stopped before it could answer your message.`, and the bare `could not answer your message` / `The model call failed.` an old host wrote), never a free substring, so an ordinary answer that merely ends in the model-failure wording keeps advancing the step (#2741). A stopped turn is a failed step, with the stop notice as its reason, instead of finishing as done and handing that notice to the next node (#2741). A 429 or rate-limit mention in any other reply does not fail a step. Stop and Reset (`workflow/reset`) follow the same order as `dispose`: they release the run's Bots before the record forgets their ids, and an id whose release failed stays named on the still-live run, so the next Stop, Reset, dispose or load-time `reap` retries it (#1476). A run left that way is marked `paused` and `releasePending` and shows as Paused: it hands off no further node, `resume` is refused with an error, and Stop again is the retry. It also keeps the deadline of the node it was on — the one paused state that holds one — and a graph save re-syncs timers without taking it away (a save that cleared it left the run holding its slot with nothing left to free it); the deadline releases again there, ends the run as `paused-expired` and frees the slot, re-arming itself while a Bot still will not go (#2453, P-560). A whole-workflow Reset that could not release every Bot freezes every run it did not reset, because each of them has lost the ids whose release did go. The markers go away when the run ends: Stop again, `dispose`, `reap`, or its own completion. 4. Each workflow keeps a FIFO queue and up to `maxConcurrent` live runs (default 2, an integer from 1 to 8 on the record, via `workflow/concurrency`, or in Settings → Workflow graphs). A task that does not fit is enqueued. When a run finishes, every workflow with a free slot is drained, oldest enqueue first. A new assign on a workflow that already has a queue waits behind it. A task does not replace a running run or release that run's Bots. Migrating an old store keeps only runs that were still `running` with ephemeral Bot ids; other legacy scripted rows are dropped from active state (finished tasks may remain in lane history). History records a finished or failed live run. Lowering the cap does not stop a run that has already started. Assigning `{ botId }` claims the card for that local Bot and does not enqueue. A remote Bot is refused. `activeCount` is `runningOf(lane)` — live runs with `live === true` and `status === 'running'`. The stage aggregate chip and the Runs tab (运行记录) must agree: every such run appears as its own running row and progress card with its own `status` (never a hardcoded Running), the chip count does not exceed that list, and a failed sibling appears with its time and reason. On the Runs list, the clock stays in a non-shrinking column with a gap before the status chip, and a Failed row paints the Failed chip once — a generic Failed with no host detail does not also draw `wf-hist-reason` (#1618). 5. A node declares up to eight inputs and eight outputs. A required input is an output of the same name on an ancestor, or `task`, `title`, or `brief` on a root. A start with no provider is refused, and Bots spawned for that attempt are released. On a Bot-backed run a successor stays queued until its required inputs are present. If every forward predecessor is done, failed or skipped and an input is still missing, that node fails and the run stops. The dispatch text carries `Input : `. It also lists the OUTPUT lines the step owes — its declared ports plus every output its outgoing edges read, whether a condition's `when` or a loop's `until` (`edgeOutputs` in `src/graph.js`) — spelled `OUTPUT : ` when an edge compares that output against a value — so a node that declares no outputs is still told which line its own gate reads (#2669, #2685). The Bot reports `OUTPUT : `; a single declared output may be the reply body. A declared output a gate reads is required (§3): when a `when`/`else` or `until` edge reads it and the step's reply does not produce it, the step fails with `failure.text` `Missing OUTPUT ` instead of settling, so the gate never reads silence as "not this branch" and the run cannot close `done` on it; a delivered value that does not equal `eq` still completes the step and skips the untaken branch (#1773). One exception keeps the work a step already did (#2583, #2658): when `state` is the only gate-read output missing and the step's own reply shows its result — a pull request link, `pull/`, a line saying it opened PR #n, a `Claimed-by:` / `Claimed #n` line, or its own declared OUTPUT names the delivery (`pr`/`pull` whose value is only a pull number — a positive integer, optional leading `#`, optional trailing parenthetical such as `2657 (verified)` — or `merged`/`merge-commit` carrying a 40-character commit sha; any other output name counts for nothing) — the step stands as `done`, the run takes the node's first declared `state` branch (on the fast line `state: ready → claim`, so the pull request reaches gate/enqueue instead of the `else` route the step never chose), and the run record carries `outputMissing: true` and `producedPull: ` when a pull request was named; every other missing verdict still fails, and a step whose reply shows no such result still fails the same way. `stepArtifacts` reads that evidence and `firstBranchValue` names the branch, both in `src/graph.js`; the engine has no GitHub client, so the reply is the only evidence it can read. Nodes start in edge order. 6. Each node is a real ephemeral Bot for that run, then it is removed. A permanent sidebar Bot (`botId`) is optional and is not required for the step to run. **File and shell rights are the same as a normal sidebar Bot's:** every node Bot gets the `danger-full-access` preset, and `EPHEMERAL_DENIED_TOOLS` still refuses the team-admin tools at the guard and approval layers (#1466, 29 names). **`node.tools` selects connectors only:** `spawn` passes the node's cleaned list to `fb.bot.create`, the host records it as `stepTools`, and `connector-access.js` allows `mcp__github__*` for `github` and `mcp__composio__*` for `composio` — a connector the node did not name is refused, and a node that names none reaches none. The list is also stored in `fields.tools` for the UI. A hidden run Bot cannot show an approval card, so the approval seam answers the same way the guard does. The host stores this on the run Bot as `stepTools` and `permission`. @@ -123,7 +123,7 @@ conclusion about the pull request. ## Tests -`plugins/workflow/test/`: graph advance and join, graph edit, assign and persistence, the FIFO queue and concurrency cap, Bot-reply advance, a dispatch that fails without being marked handed, a bounded hand-off retry and a step deadline that fails the run and clears its timer (#1530), a per-workflow `stepTimeoutMinutes` that replaces the ten-minute constant and is refused outside 1..120 (#1580), disposing releases the Bots before the record drops their ids and a released runner name is reusable again, assign-to-bot versus assign-to-workflow, inputs and outputs along edges, a loop that breaks and an if/else branch, failure recovery (onFail edges with max attempts, exhaustion behavior, and compatibility with failOnce and loop edges), templates, `define` / `runNamed`, the matcher, ephemeral spawn and cleanup, a stopped run with its settled node map (#1424), the reply middleware, the client canvas, the settings page (including a condition and a loop), rename and delete (including factory-id rows such as `delivery`), the stage chat's scroll rule (a new visible line follows, a filtered node reply moves nothing), the assign dialog's preselected card and its failed and empty board states (#1457, #1458), a sidebar click that leaves the Workflows section in place, and the claim rules — a re-routed card moves with one claim and one route, the same target writes nothing, a trigger never invalidates, the held list is capped and omits a Bot claim, the held page answering `heldLimit` up to its 200 cap and the stage unfolding it behind its `wf-more` row (#2256), `rematch: true` drops one claim, and the panel's one re-match action goes through `ingest` (#1647), and the assigned brief reaching the step prompt while a task without one still gets its title, a parked card keeping its brief on disk, and an unknown `assign` field coming back in `ignored` (#1869). Run `npm test` in `plugins/workflow`. The host seam that applies `ownerBotId` is `core/test/task-route.test.js`, which also pins that a card written through `track_tasks` reaches the listener (#1462). The host omission of run Bots, and the grant of `stepTools` plus the hidden-approval answer, is `core/test/flybot.test.js`. Which connectors a run Bot may call is `core/test/connector-access.test.js`, and the chain from a node's `tools` through `spawn` to that guard — the built-in delivery nodes included — is `core/test/workflow-node-connectors.test.js`. +`plugins/workflow/test/`: graph advance and join, graph edit, assign and persistence, the FIFO queue and concurrency cap, Bot-reply advance, a dispatch that fails without being marked handed, a bounded hand-off retry and a step deadline that fails the run and clears its timer (#1530), the three hangs a host call can park the tail with — a dispatch, a release and a spawn that never answer — ending `failed` at the deadline and putting the queued card back (#2748), a per-workflow `stepTimeoutMinutes` that replaces the ten-minute constant and is refused outside 1..120 (#1580), disposing releases the Bots before the record drops their ids and a released runner name is reusable again, assign-to-bot versus assign-to-workflow, inputs and outputs along edges, a loop that breaks and an if/else branch, failure recovery (onFail edges with max attempts, exhaustion behavior, and compatibility with failOnce and loop edges), templates, `define` / `runNamed`, the matcher, ephemeral spawn and cleanup, a stopped run with its settled node map (#1424), the reply middleware, the client canvas, the settings page (including a condition and a loop), rename and delete (including factory-id rows such as `delivery`), the stage chat's scroll rule (a new visible line follows, a filtered node reply moves nothing), the assign dialog's preselected card and its failed and empty board states (#1457, #1458), a sidebar click that leaves the Workflows section in place, and the claim rules — a re-routed card moves with one claim and one route, the same target writes nothing, a trigger never invalidates, the held list is capped and omits a Bot claim, the held page answering `heldLimit` up to its 200 cap and the stage unfolding it behind its `wf-more` row (#2256), `rematch: true` drops one claim, and the panel's one re-match action goes through `ingest` (#1647), and the assigned brief reaching the step prompt while a task without one still gets its title, a parked card keeping its brief on disk, and an unknown `assign` field coming back in `ignored` (#1869). Run `npm test` in `plugins/workflow`. The host seam that applies `ownerBotId` is `core/test/task-route.test.js`, which also pins that a card written through `track_tasks` reaches the listener (#1462). The host omission of run Bots, and the grant of `stepTools` plus the hidden-approval answer, is `core/test/flybot.test.js`. Which connectors a run Bot may call is `core/test/connector-access.test.js`, and the chain from a node's `tools` through `spawn` to that guard — the built-in delivery nodes included — is `core/test/workflow-node-connectors.test.js`. ## Connector ceiling (#1544) diff --git a/plugins/workflow/src/service.js b/plugins/workflow/src/service.js index 4bc1ac77b..feaf41bbc 100644 --- a/plugins/workflow/src/service.js +++ b/plugins/workflow/src/service.js @@ -55,6 +55,11 @@ export const MIN_STEP_TIMEOUT_MINUTES = 1 export const MAX_STEP_TIMEOUT_MINUTES = 120 export const STEP_DEADLINE_MS = DEFAULT_STEP_TIMEOUT_MINUTES * 60 * 1000 export const HANDOFF_RETRY_DELAYS_MS = [1000, 2000] +// #2748: a start whose Bots the host never finishes creating leaves the card +// out of `lane.queue` (takeNext already shifted it) and the call hanging. The +// spawn gets its own bound, shorter than a step's: no card waits a whole step +// deadline for an answer that may never come. +export const SPAWN_DEADLINE_MS = 60 * 1000 function stepTimeoutValue(raw) { const value = Number(raw) @@ -392,13 +397,48 @@ export function createWorkflowService({ write() } + // #2748 C4: a host that never answers a spawn must not keep the card out of the + // queue with the call hanging. The wait is bounded; on the deadline the start is + // refused, and the Bots a late answer creates are released rather than leaked. + function boundedSpawn(pending) { + return new Promise((resolve, reject) => { + let settled = false + const handle = setTimer(() => { + settled = true + const error = new Error(`the host did not create the run Bots within ${Math.round(SPAWN_DEADLINE_MS / 1000)}s`) + error.code = 'spawn-timeout' + Promise.resolve(pending) + .then(ids => (typeof release === 'function' ? releaseAll(Object.values(ids ?? {}), release, warn) : [])) + .catch(() => {}) + reject(error) + }, SPAWN_DEADLINE_MS) + handle?.unref?.() + Promise.resolve(pending).then((ids) => { + if (settled) return + settled = true + clearTimer(handle) + resolve(ids) + }, (error) => { + if (settled) return + settled = true + clearTimer(handle) + reject(error) + }) + }) + } + // A step that reaches its deadline is failed, frees its Bots and says which // step it was, so a lost Bot cannot hold a lane slot forever. The deadline is a // limit on the wait, not proof nothing arrived: a step was killed at 45 minutes // while still writing (#1868), so the line names the deadline it reached and // never reads as a reply that never came. function expireStep(workflowId, runId, nodeId) { - return hostSerial(async () => { + // #2748: the deadline must not be queued behind the wait it bounds. The tail + // can be parked for ever by a host call that never answers (a dispatch, a + // release, a spawn), and a deadline that waited its turn behind one never + // ran — the run stayed `running` with its timer already consumed. The record + // is settled here, off the tail, and the drain that follows runs outside it. + return (async () => { const item = workflow(workflowId) const minutes = stepTimeoutMinutesOf(item) const lane = laneOf(workflowId) @@ -427,7 +467,7 @@ export function createWorkflowService({ await releaseRun(workflowId, run) await drainAll() return present() - }).catch(error => warn(`flybot-workflow: step ${nodeId} could not be timed out: ${error?.message ?? error}`)) + })().catch(error => warn(`flybot-workflow: step ${nodeId} could not be timed out: ${error?.message ?? error}`)) } // The bytes of a store this service could not read are kept before the first write @@ -1202,10 +1242,10 @@ export function createWorkflowService({ try { // #1544: the caller that started the run and the Bot that saved the graph cap the connectors // this run may use; the host intersects them with each node's `tools`. - ephemeral = await spawnAll(item.nodes, bots() ?? [], spawn, item.model ?? '', { + ephemeral = await boundedSpawn(spawnAll(item.nodes, bots() ?? [], spawn, item.model ?? '', { callerBotId: task.callerBotId, authorBotId: item.authoredBy, - }) + })) } catch (error) { await releaseAll(error.spawned ?? [], release, warn) return refuse(error) diff --git a/plugins/workflow/test/tail-hang.test.js b/plugins/workflow/test/tail-hang.test.js new file mode 100644 index 000000000..65e6032fe --- /dev/null +++ b/plugins/workflow/test/tail-hang.test.js @@ -0,0 +1,205 @@ +// #2748 probes: an unbounded host call inside the serial tail parks the one +// thing that bounds everything else (the step deadline). Each case below is red +// on f2ef4dfa and names the terminal state the fix must leave behind. +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { STEP_DEADLINE_MS, createWorkflowService } from '../src/service.js' + +const CLOCK = 1_700_000_000_000 +const ROSTER = [ + { id: 'main', name: 'Chief', role: 'lead', isMain: true, remote: false }, +] + +function scheduler() { + let seq = 0 + const queue = new Map() + function setTimer(fn, ms) { + const id = ++seq + queue.set(id, { fn, due: Math.max(1, Number(ms) || 0) }) + return id + } + function clearTimer(id) { + queue.delete(id) + } + // Fire every timer that comes due within `ms` without awaiting what it starts: + // a timer whose promise never settles is exactly the state under test, so + // awaiting it would hang the test instead of describing the bug. + function fire(ms) { + let left = Math.max(0, Number(ms) || 0) + let fired = 0 + for (;;) { + let next + for (const [id, entry] of queue) { + if (entry.due > left) continue + if (!next || entry.due < next.entry.due) next = { id, entry } + } + if (!next) break + queue.delete(next.id) + left -= next.entry.due + for (const entry of queue.values()) entry.due -= next.entry.due + fired += 1 + Promise.resolve(next.entry.fn()).catch(() => {}) + } + for (const entry of queue.values()) entry.due -= left + return fired + } + return { setTimer, clearTimer, fire, pending: () => queue.size } +} + +async function settle(rounds = 25) { + for (let i = 0; i < rounds; i += 1) await new Promise(done => setImmediate(done)) +} + +function harness({ dispatch = async () => ({ ok: true }), spawn, release } = {}) { + const spawned = [] + const released = [] + const dispatched = [] + const warnings = [] + const timers = scheduler() + let seq = 0 + const svc = createWorkflowService({ + persist: false, + now: () => CLOCK, + bots: () => ROSTER, + spawn: spawn ?? (async (spec) => { + const id = `eph-${spec.nodeId}-${++seq}` + spawned.push({ id, spec }) + return id + }), + release: release ?? (async (id) => { released.push(id) }), + dispatch: async (step) => { + dispatched.push(step) + return dispatch(step) + }, + setTimer: timers.setTimer, + clearTimer: timers.clearTimer, + warn: (line) => warnings.push(String(line)), + }) + return { svc, spawned, released, dispatched, warnings, timers } +} + +function make(svc, name = 'Land check') { + const made = svc.define({ + name, + nodes: [ + { id: 'plan', name: 'Planner', role: 'plan', prompt: 'Write the plan.' }, + { id: 'code', name: 'Coder', role: 'code', prompt: 'Implement it.' }, + ], + edges: [{ from: 'plan', to: 'code' }], + }) + assert.equal(made.ok, true) + return made.workflows.find(item => item.name === name) +} + +function row(state, workflowId) { + return state.workflows.find(item => item.id === workflowId) +} + +test('#2748 C1: a dispatch that never settles still reaches failed at its step deadline', async () => { + const never = new Promise(() => {}) + const { svc, released, timers } = harness({ dispatch: () => never }) + const flow = make(svc) + const assigning = svc.assign({ workflowId: flow.id, taskId: '#2748-C1', title: 'Hang dispatch' }) + assigning.catch(() => {}) + await settle() + + const started = row(svc.state(), flow.id) + assert.equal(started.run.status, 'running', 'the run started and is waiting on the peer Bot') + assert.equal(started.run.nodeStatus.plan, 'running') + + timers.fire(STEP_DEADLINE_MS + 1) + await settle() + + const after = row(svc.state(), flow.id) + // eslint-disable-next-line no-console + console.log(`# PROBE C1 status: ${after.run.status} node: ${after.run.nodeStatus.plan} active: ${after.activeCount} released: ${released.length} timersLeft: ${timers.pending()}`) + assert.equal(after.run.status, 'failed', 'the deadline ends the run even while the dispatch hangs') + assert.equal(after.run.nodeStatus.plan, 'failed') + assert.equal(after.activeCount, 0) + assert.ok(released.length > 0, 'the run Bots are released') +}) + +test('#2748 C2: a release that never settles does not starve another workflow\'s deadline', async () => { + const hang = new Set() + const released = [] + let svc + const timers = scheduler() + let seq = 0 + svc = createWorkflowService({ + persist: false, + now: () => CLOCK, + bots: () => ROSTER, + spawn: async (spec) => { + const id = `eph-${spec.nodeId}-${++seq}` + return id + }, + release: (id) => { + if (hang.has(id)) return new Promise(() => {}) + released.push(id) + return Promise.resolve() + }, + dispatch: async () => ({ ok: true }), + setTimer: timers.setTimer, + clearTimer: timers.clearTimer, + warn: () => {}, + }) + const alpha = make(svc, 'Alpha') + const bravo = make(svc, 'Bravo') + await svc.assign({ workflowId: alpha.id, taskId: '#2748-C2a', title: 'Alpha' }) + await svc.assign({ workflowId: bravo.id, taskId: '#2748-C2b', title: 'Bravo' }) + const alphaRun = row(svc.state(), alpha.id).run + const bravoRun = row(svc.state(), bravo.id).run + for (const id of Object.values(alphaRun.ephemeral)) hang.add(id) + + const stopping = svc.reset({ workflowId: alpha.id, runId: alphaRun.id }) + stopping.catch(() => {}) + await settle() + + timers.fire(STEP_DEADLINE_MS + 1) + await settle() + + const bravoAfter = row(svc.state(), bravo.id) + const bravoReleased = Object.values(bravoRun.ephemeral).filter(id => released.includes(id)) + // eslint-disable-next-line no-console + console.log(`# PROBE C2 alpha: ${row(svc.state(), alpha.id).run.status} bravo before: ${bravoRun.status} after its deadline: ${bravoAfter.run.status} bravo node: ${bravoAfter.run.nodeStatus.plan} released: ${released.length} timersLeft: ${timers.pending()}`) + assert.equal(bravoAfter.run.status, 'failed', 'the other workflow still reaches its deadline') + assert.equal(bravoAfter.activeCount, 0) + assert.equal(bravoReleased.length, Object.values(bravoRun.ephemeral).length, 'its Bots are released') +}) + +test('#2748 C4: a spawn that never answers keeps the queued card instead of losing it', async () => { + const never = new Promise(() => {}) + let hangSpawn = false + const { svc, timers } = harness({ + spawn: (spec) => (hangSpawn ? never : Promise.resolve(`eph-${spec.nodeId}`)), + }) + const flow = make(svc) + svc.setConcurrency({ workflowId: flow.id, maxConcurrent: 1 }) + await svc.assign({ workflowId: flow.id, taskId: '#2748-C4a', title: 'Running card' }) + const first = row(svc.state(), flow.id).run + await svc.assign({ workflowId: flow.id, taskId: '#2748-C4b', title: 'Queued card' }) + assert.equal(row(svc.state(), flow.id).queueDepth, 1, 'the second card waits for the slot') + + // The slot frees while the host is no longer answering a spawn: takeNext has + // already shifted the card, so the only thing that keeps it is a terminal path. + hangSpawn = true + let settled = false + const finishing = (async () => { + await svc.stepResult({ botId: first.ephemeral.plan, text: 'plan done' }) + await svc.stepResult({ botId: first.ephemeral.code, text: 'code done' }) + })() + finishing.then(() => { settled = true }, () => { settled = true }) + await settle() + assert.equal(row(svc.state(), flow.id).queueDepth, 0, 'on base the card is already out of the queue') + + timers.fire(STEP_DEADLINE_MS + 1) + await settle() + + const after = row(svc.state(), flow.id) + // eslint-disable-next-line no-console + console.log(`# PROBE C4 settled: ${settled} queue: ${after.queueDepth} active: ${after.activeCount} card: ${after.queue[0]?.id ?? 'none'}`) + assert.equal(settled, true, 'the call answers instead of hanging for ever') + assert.equal(after.activeCount, 0) + assert.equal(after.queueDepth, 1, 'the card is back in the queue') + assert.equal(after.queue[0].id, '#2748-C4b') +}) From cc35bf484932aba44e24b02a6e3d978495f09aa0 Mon Sep 17 00:00:00 2001 From: Dev-3 Date: Tue, 10 Nov 2026 04:24:13 +0000 Subject: [PATCH 2/3] workflow: say a late start's release failed instead of dropping it (#2748) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #2748 Owner: Dev-3 Written-by: Dev-3 Claim-Id: 9df438ed10ac478b8d037965974377ec Base: f2ef4dfaccedd40315bd585474d0902118dc0ba8 Risk tier: T2 `package` refused the first tip: `.github/scripts/check-empty-catch.test.mjs` (#213) lists `plugins/workflow/src/service.js:412` — the `.catch(() => {})` on the late answer's release chain. The handler now warns with the reason, which is what the rest of the file does, and the release path itself is unchanged: `releaseAll` already warns once per Bot it could not remove and returns the ids that failed. `releaseAll` is also the one place that already answers "no release function" with an empty list, so the `typeof release === 'function'` ternary in front of it was dead and is gone; the behaviour is the same either way. Re-read after the change: the three #2748 probes are 3/3, `plugins/workflow` is 511/511, `node .github/scripts/check-empty-catch.mjs` exits 0, and oxlint is 0 errors with the same 8 pre-existing warnings. --- plugins/workflow/src/service.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/workflow/src/service.js b/plugins/workflow/src/service.js index feaf41bbc..b13f07055 100644 --- a/plugins/workflow/src/service.js +++ b/plugins/workflow/src/service.js @@ -408,8 +408,10 @@ export function createWorkflowService({ const error = new Error(`the host did not create the run Bots within ${Math.round(SPAWN_DEADLINE_MS / 1000)}s`) error.code = 'spawn-timeout' Promise.resolve(pending) - .then(ids => (typeof release === 'function' ? releaseAll(Object.values(ids ?? {}), release, warn) : [])) - .catch(() => {}) + .then(ids => releaseAll(Object.values(ids ?? {}), release, warn)) + // releaseAll already warns once per Bot it could not remove; this catches only a + // throw from the chain itself, and the start is refused either way, so say it. + .catch(error => warn(`flybot-workflow: a late start could not release its Bots: ${error?.message ?? error}`)) reject(error) }, SPAWN_DEADLINE_MS) handle?.unref?.() From a79976fddd7599ab32b0615c0595aea051305820 Mon Sep 17 00:00:00 2001 From: Dev-3 Date: Tue, 10 Nov 2026 04:59:09 +0000 Subject: [PATCH 3/3] workflow: keep a lane's cap while a start waits on the host (#2748) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two steps whose deadlines pass in the same turn each call drainAll(). The first start is parked on the host creating its run Bots, and takeNext() counts only begun runs, so the second drain read the lane as empty and started another card: maxConcurrent 1 with activeCount 2 (Test's advisory on #2773). A start now counts against its lane from the moment its card is taken until beginRun() makes the run visible to runningOf(), and every way out of a start gives that count back — the refused ones and the ones the host answers late included. The count is not persisted: it lasts one start. The queue and the scheduled cards are unchanged, so a card the cap keeps out is still the next one to start when a slot frees. plugins/workflow/test/cap-concurrent-drain.test.js is the case: red at cc35bf484932aba44e24b02a6e3d978495f09aa0 (activeCount 2 at maxConcurrent 1), green with this change. --- changelog.d/2748-workflow-stuck-run.fixed.md | 2 +- docs/subsystems/workflow.md | 4 +- plugins/workflow/src/service.js | 95 ++++++++---- .../test/cap-concurrent-drain.test.js | 138 ++++++++++++++++++ 4 files changed, 204 insertions(+), 35 deletions(-) create mode 100644 plugins/workflow/test/cap-concurrent-drain.test.js diff --git a/changelog.d/2748-workflow-stuck-run.fixed.md b/changelog.d/2748-workflow-stuck-run.fixed.md index 81625d457..b3737b716 100644 --- a/changelog.d/2748-workflow-stuck-run.fixed.md +++ b/changelog.d/2748-workflow-stuck-run.fixed.md @@ -1 +1 @@ -A Workflow run whose host call never answered no longer stays running for ever: a step whose deadline passes is ended as failed even while a dispatch, a release or a Bot start is still waiting, with its Bots released. A start whose Bots the host does not create within 60 seconds is refused, and a queued card a drain had already taken out of the queue goes back to it instead of disappearing. ([#2748](https://github.com/ThinkFlowLab/FlyBot/issues/2748)) +A Workflow run whose host call never answered no longer stays running for ever: a step whose deadline passes is ended as failed even while a dispatch, a release or a Bot start is still waiting, with its Bots released. A start whose Bots the host does not create within 60 seconds is refused, and a queued card a drain had already taken out of the queue goes back to it instead of disappearing. A lane also keeps its concurrency limit while a start is still waiting on the host: two steps ending in the same turn no longer start one card too many. ([#2748](https://github.com/ThinkFlowLab/FlyBot/issues/2748)) diff --git a/docs/subsystems/workflow.md b/docs/subsystems/workflow.md index 6f4c27376..39e467c71 100644 --- a/docs/subsystems/workflow.md +++ b/docs/subsystems/workflow.md @@ -60,7 +60,7 @@ Everything in `plugins/workflow/`: the graph rules, the run store, the matcher, 2. The canvas and the run share one graph: nodes, edges and positions. A drag commits `workflow/graph`, but only while **Edit** is on — a factory preset is copied first (#1378). A wire or a reconnect that would list a pair the graph already carries commits nothing new, because `normalizeGraph` refuses a graph that lists one pair twice and then saves none of it: unguarded, the plane would show no edge, no change and only a save error (#2163). Status highlighting reads that same graph. Replacing edges does not recompute who is already running. The Settings canvas paints that graph on the same 960px design plane, scaled to the width the settings page gives it, so the canvas is not a second scroll container inside the page's own scroll; its pointer deltas are divided by the scale, and its wires keep a stronger stroke than a plain divider so an edge can be followed at that size (#1483). While another dsh holds the Bot team the host is read-only (`roster.readOnly`): the page still reads through `workflow/state`, keeps its controls disabled, and offers no **Edit**, **Add link** or wire port, so it cannot take a change that core would refuse with `bot/read-only` and then drop (#2187). The same window draws the sidebar's workflow list without **New**, the row's **Rename**/**Delete** or **Hand a task to a Workflow**: the list is read-only too, and an `roster.readOnly` that is not a string, or holds only whitespace, counts as writable — trimmed the way `core/src/client/workflows.js:570` trims it (#2200). The Stage a row opens carries the same lock: it prints the host's sentence under `role="status"` and draws no **Hand a task to a Workflow**, no **Re-match**, no **Pause**/**Resume**/**Stop**, no **Edit**/drag palette and no **Reset**, while the row click, **Progress**, **Flow** and viewing a run still work; a writable roster draws every control exactly as before (#2213). Mounting the list in a read-only window still reads `projects` so the Project filter fills, but it sends no `workflow/ingest` for the open cards: ingest is a claim/match write the host refuses, so a reader opening the list writes nothing; a writable window still ingests every open card (#2222). The read-only sentence can also land after the list has mounted — a cold window starts on the empty roster, `ready: false` and no `readOnly` — so the ingest step asks the value the latest render saw rather than the one the mount saw, and it skips the board read and stops between two cards once the window has turned read-only. The Settings page draws no read-only sentence of its own: the host's Settings header already prints one for every page it shows (`core/src/client/settings.js` `bt-read-only`, `role="status"`, `hostText(roster.readOnly)`), and a second copy inside the page said the same thing twice (#2202). 3. **A live run advances only when its ephemeral Bot replies, following the graph's edges in order.** That reply advances the run and may appear on the workflow stage log; it is not delivered into a persistent sidebar Bot conversation (for example Chief). The `workflow-step` send middleware denies those replies after `stepResult` runs. Nodes start only when their forward predecessors finish and edges reach them. **Early termination (#1537, #1538, #1546, #1570, #1571):** When a node completes **or fails**, the run ends immediately with status `done`, sets `run.earlyEnd` to the condition name, and does not advance to successors, take a success-path `loop`, or burn `onFail` recovery attempts. The two markers that report the task itself end it only on a structured signal (#1546): `needs-human` and `already-claimed` must be a declared OUTPUT whose value **equals** the marker, or a whole reply line that is exactly `EARLY-END ` in any case. A step that quotes an issue title, negates the label or reports it absent keeps running. `tip-unchanged` (#1570) and `open-pr` (#1571) accept that same structured signal — in a declared OUTPUT the value counts as a whole word, so a brief that names `data-open-project` is not `open-pr` (#1866) — and also honor their prose fallbacks (`tip unchanged` / `unchanged tip`, `open PR already` / `already has an open PR` / `has an open PR` / `found an open PR`), so a Bot that forgets the structured marker cannot soft-lock Claim↔Verify on the same tip. See `checkEarlyEnd` in Key files. Factory prompt guidance lives in `EARLY_END_GUIDANCE` (`plugins/workflow/src/template.js`) for Settings graphs that wire Triage / Claim / Verify: Verify must escalate once on an unchanged tip, not re-run the full suite. There is no scripted or seeded run path: `workflow/state` exposes only `live === true` runs — the running ones, plus the last run when it is still `live` and ended `failed` — a failed sibling stays on the stage beside whatever is running now, with `run.failure` (`{ code, at, nodeName?, text? }`) naming what ended it, so the room does not have to open Runs to learn that a run failed (#1493, #1494). The failed sibling is left out when a running run carries the same task id: that is a retry, and the live run keeps the row (#1507). History entries carry the run `id` and the same `failure`, so a Runs row shows when the run ended and why, and a row for a run already on the stage is not listed twice. A run does not create a schedule. `scheduleLabel` is display text. When the host can create Bots, starting a live run (`assign`, `run_workflow`, `ingest`) spawns one ephemeral Bot per node from that node's prompt, role, model and tools, sends it the step from the Main Bot, and removes every instance when the run finishes, fails for good, or is reset. A `failOnce` retry keeps them. They are hidden and left out of the sidebar. Loading the plugin does not spawn Bots for the default workflows. Plugin unload (`dispose`) and the load-time `reap` both end a live run still marked `running`: in-flight nodes are failed, the run is archived as `failed`, and any ephemeral Bot ids are released. A **read-only** host — a second window on the same `DSH_HOME`, the reader `home-lock` describes — does neither: it never ran those runs, so it cannot tell which of them a dead host left behind, and it must not end them, release their step Bots or write its older copy over the writer's store; both calls return without touching the record and the writer's own `reap` decides (#2673). **Taking the lock over is a state change (#2704):** the first call that finds the host writable after this window started read-only — a read, `reap`, `dispose` or any write — re-reads `workflows.json` once before it changes anything, because the copy it loaded predates what the dead window saved, and then runs the same load-time `reap` once on that adopted store. That reap ends the dead window's `running` run as `host-ended` (or heals a `live:true` run whose Bot map is already gone, #1414) and releases its step Bots; `lane.queue` is never touched, so a card that was merely `queued` keeps its place and starts when the lane drains. Only the first such call re-reads — a writer that later loses the lock is still refused, exactly as before. A run already written as `running` with no ephemeral map (an older unload) is healed the same way on the next load (#1414). Loading is also the one moment a run the host left behind is noticed when its ephemeral map is still present: `reap` releases those instances and records the run as `failed`. Nothing resumes it. A run that already ended keeps its own status (#1388). `workflow/reset` stopping one live run ends it the same way before its Bots are released: nodes still `running` become `failed`, so the strip cannot keep painting a run that is over (#1424). A graph edit that drops a node that is still running ends that run the way Stop does: the node is failed, the run is archived `failed`, its node Bots are released, and the queue drains (#1549); an id whose release failed stays named on that archived run, and the next load's `reap` retries it, instead of the edit forgetting it (the #1616 contract). Every wait is bounded (#1530). The deadline is not queued behind the wait it bounds (#2748): an expiry runs off the serial tail, so a host call that never answers — a dispatch, a release, a spawn — cannot park the one timer that ends the run, and the run still reaches `failed` with its Bots released. A start whose Bots the host does not create within `SPAWN_DEADLINE_MS` (60 s) is refused, and the card a drain had already taken out of `lane.queue` goes back to it instead of vanishing (a late spawn's Bots are released). A dispatch that does not land is not marked handed: the chat names the code and the attempt, and it is retried after a short backoff at most twice, never twice inside one hand-off, so a 429 is not hammered. When it still does not land, the step and the run fail with the reason written into the chat. Each running live step also holds a deadline — its workflow's `stepTimeoutMinutes`, `DEFAULT_STEP_TIMEOUT_MINUTES` (45 minutes) unless the record sets one, an integer from 1 to 120 given at create, through `define_workflow`, or through `workflow/step-timeout` (armed even when the host cannot spawn ephemeral Bots; #1530, #1580); when it expires the node and the run fail as `failed`, the run's ephemeral Bots are released, the chat and `run.failure` say the step's deadline was reached (`step deadline reached (45 min)`, the number its lane set), and `warn` names it once; the line never reads as a reply that did not arrive, because a step still working is killed by the same timer (#1868). The deadline is cleared as soon as the step moves, and by Pause, Reset, delete, `reap`, `dispose` and a Stop that released every Bot; a run the owner paused holds none, and Resume arms it again from that moment. A timer never outlives the run that started it. A reply that matches no running step — its run ended, or the node already moved — is reported through `warn` once per Bot and run instead of being dropped silently. A reply fails that node on a structured signal only: the host's failure notice carries `failure: true`, which the middleware hands `stepResult` as `ok: false` (a caller-set `ok: false` counts too), so an answer that quotes `could not answer your message` or `model call failed` while auditing it advances the step (#2677). An old host without the marker falls back to the notice shapes at the start of the text only (` could not answer your message: …`, ` was stopped before it could answer your message.`, and the bare `could not answer your message` / `The model call failed.` an old host wrote), never a free substring, so an ordinary answer that merely ends in the model-failure wording keeps advancing the step (#2741). A stopped turn is a failed step, with the stop notice as its reason, instead of finishing as done and handing that notice to the next node (#2741). A 429 or rate-limit mention in any other reply does not fail a step. Stop and Reset (`workflow/reset`) follow the same order as `dispose`: they release the run's Bots before the record forgets their ids, and an id whose release failed stays named on the still-live run, so the next Stop, Reset, dispose or load-time `reap` retries it (#1476). A run left that way is marked `paused` and `releasePending` and shows as Paused: it hands off no further node, `resume` is refused with an error, and Stop again is the retry. It also keeps the deadline of the node it was on — the one paused state that holds one — and a graph save re-syncs timers without taking it away (a save that cleared it left the run holding its slot with nothing left to free it); the deadline releases again there, ends the run as `paused-expired` and frees the slot, re-arming itself while a Bot still will not go (#2453, P-560). A whole-workflow Reset that could not release every Bot freezes every run it did not reset, because each of them has lost the ids whose release did go. The markers go away when the run ends: Stop again, `dispose`, `reap`, or its own completion. -4. Each workflow keeps a FIFO queue and up to `maxConcurrent` live runs (default 2, an integer from 1 to 8 on the record, via `workflow/concurrency`, or in Settings → Workflow graphs). A task that does not fit is enqueued. When a run finishes, every workflow with a free slot is drained, oldest enqueue first. A new assign on a workflow that already has a queue waits behind it. A task does not replace a running run or release that run's Bots. Migrating an old store keeps only runs that were still `running` with ephemeral Bot ids; other legacy scripted rows are dropped from active state (finished tasks may remain in lane history). History records a finished or failed live run. Lowering the cap does not stop a run that has already started. Assigning `{ botId }` claims the card for that local Bot and does not enqueue. A remote Bot is refused. `activeCount` is `runningOf(lane)` — live runs with `live === true` and `status === 'running'`. The stage aggregate chip and the Runs tab (运行记录) must agree: every such run appears as its own running row and progress card with its own `status` (never a hardcoded Running), the chip count does not exceed that list, and a failed sibling appears with its time and reason. On the Runs list, the clock stays in a non-shrinking column with a gap before the status chip, and a Failed row paints the Failed chip once — a generic Failed with no host detail does not also draw `wf-hist-reason` (#1618). +4. Each workflow keeps a FIFO queue and up to `maxConcurrent` live runs (default 2, an integer from 1 to 8 on the record, via `workflow/concurrency`, or in Settings → Workflow graphs). A start that is still waiting on the host counts against that cap: the card leaves the queue before its run exists, so two steps ending in the same turn would otherwise both read the free slot and start a card each, one past `maxConcurrent` (#2748). A task that does not fit is enqueued. When a run finishes, every workflow with a free slot is drained, oldest enqueue first. A new assign on a workflow that already has a queue waits behind it. A task does not replace a running run or release that run's Bots. Migrating an old store keeps only runs that were still `running` with ephemeral Bot ids; other legacy scripted rows are dropped from active state (finished tasks may remain in lane history). History records a finished or failed live run. Lowering the cap does not stop a run that has already started. Assigning `{ botId }` claims the card for that local Bot and does not enqueue. A remote Bot is refused. `activeCount` is `runningOf(lane)` — live runs with `live === true` and `status === 'running'`. The stage aggregate chip and the Runs tab (运行记录) must agree: every such run appears as its own running row and progress card with its own `status` (never a hardcoded Running), the chip count does not exceed that list, and a failed sibling appears with its time and reason. On the Runs list, the clock stays in a non-shrinking column with a gap before the status chip, and a Failed row paints the Failed chip once — a generic Failed with no host detail does not also draw `wf-hist-reason` (#1618). 5. A node declares up to eight inputs and eight outputs. A required input is an output of the same name on an ancestor, or `task`, `title`, or `brief` on a root. A start with no provider is refused, and Bots spawned for that attempt are released. On a Bot-backed run a successor stays queued until its required inputs are present. If every forward predecessor is done, failed or skipped and an input is still missing, that node fails and the run stops. The dispatch text carries `Input : `. It also lists the OUTPUT lines the step owes — its declared ports plus every output its outgoing edges read, whether a condition's `when` or a loop's `until` (`edgeOutputs` in `src/graph.js`) — spelled `OUTPUT : ` when an edge compares that output against a value — so a node that declares no outputs is still told which line its own gate reads (#2669, #2685). The Bot reports `OUTPUT : `; a single declared output may be the reply body. A declared output a gate reads is required (§3): when a `when`/`else` or `until` edge reads it and the step's reply does not produce it, the step fails with `failure.text` `Missing OUTPUT ` instead of settling, so the gate never reads silence as "not this branch" and the run cannot close `done` on it; a delivered value that does not equal `eq` still completes the step and skips the untaken branch (#1773). One exception keeps the work a step already did (#2583, #2658): when `state` is the only gate-read output missing and the step's own reply shows its result — a pull request link, `pull/`, a line saying it opened PR #n, a `Claimed-by:` / `Claimed #n` line, or its own declared OUTPUT names the delivery (`pr`/`pull` whose value is only a pull number — a positive integer, optional leading `#`, optional trailing parenthetical such as `2657 (verified)` — or `merged`/`merge-commit` carrying a 40-character commit sha; any other output name counts for nothing) — the step stands as `done`, the run takes the node's first declared `state` branch (on the fast line `state: ready → claim`, so the pull request reaches gate/enqueue instead of the `else` route the step never chose), and the run record carries `outputMissing: true` and `producedPull: ` when a pull request was named; every other missing verdict still fails, and a step whose reply shows no such result still fails the same way. `stepArtifacts` reads that evidence and `firstBranchValue` names the branch, both in `src/graph.js`; the engine has no GitHub client, so the reply is the only evidence it can read. Nodes start in edge order. 6. Each node is a real ephemeral Bot for that run, then it is removed. A permanent sidebar Bot (`botId`) is optional and is not required for the step to run. **File and shell rights are the same as a normal sidebar Bot's:** every node Bot gets the `danger-full-access` preset, and `EPHEMERAL_DENIED_TOOLS` still refuses the team-admin tools at the guard and approval layers (#1466, 29 names). **`node.tools` selects connectors only:** `spawn` passes the node's cleaned list to `fb.bot.create`, the host records it as `stepTools`, and `connector-access.js` allows `mcp__github__*` for `github` and `mcp__composio__*` for `composio` — a connector the node did not name is refused, and a node that names none reaches none. The list is also stored in `fields.tools` for the UI. A hidden run Bot cannot show an approval card, so the approval seam answers the same way the guard does. The host stores this on the run Bot as `stepTools` and `permission`. 7. A loop edge (`loop: true`, `max` from 1 to 8, default 3, optional `until: { output, eq }`) is the only cycle for success paths. The body runs again until `until` matches or `max` runs have finished, and the forward edges wait until then. Restarting the body drops the branch choice made inside it and queues a sibling that the previous pass skipped, so the next pass can take the other side. A `when: { output, eq }` edge is taken only when that output equals `eq`. `when: { else: true }` is taken only when no other `when` matched. A `when` or `until` that is present but cannot be honoured — a blank output name, or one outside `/^[a-z][a-z0-9-]{0,40}$/` such as `result_ok` — refuses the save with `A condition link needs an output name like "ready"` (or the loop's own line) instead of being dropped: before #2155 the refused `when` was indistinguishable from an absent one and the edge fired unconditionally, which is the opposite of what the author asked. An absent `when`/`until` is still a plain link, no early stop. Settings → Workflow graphs keeps **Add link** disabled for a Condition with a blank output name, and the refusal shows in the page's `.wf-err` alert. A node whose forward edges are all closed is `skipped` and does not block a join. A run cannot spin: every loop has a max. An `onFail` edge (`onFail: true`, `max` recoveries from 1 to 8, default 3) provides failure recovery: when a node fails, if an `onFail` edge points from that node back to an upstream target and recoveries remain (tracked in `failAttempts` per edge), the workflow resets every node from the recovery target through the failing node (the target to `running`, the rest to `queued`) and re-runs them in order. After `max` recoveries are used, the node fails permanently and the run stops. An `onFail` edge does not interfere with `failOnce` retry (which restarts the same node once) or success-path `loop` edges. Settings → Workflow graphs can add a link, a condition, or a loop; a failure recovery edge is added through `define_workflow` (`onFail: true`) and has no Settings control yet. @@ -123,7 +123,7 @@ conclusion about the pull request. ## Tests -`plugins/workflow/test/`: graph advance and join, graph edit, assign and persistence, the FIFO queue and concurrency cap, Bot-reply advance, a dispatch that fails without being marked handed, a bounded hand-off retry and a step deadline that fails the run and clears its timer (#1530), the three hangs a host call can park the tail with — a dispatch, a release and a spawn that never answer — ending `failed` at the deadline and putting the queued card back (#2748), a per-workflow `stepTimeoutMinutes` that replaces the ten-minute constant and is refused outside 1..120 (#1580), disposing releases the Bots before the record drops their ids and a released runner name is reusable again, assign-to-bot versus assign-to-workflow, inputs and outputs along edges, a loop that breaks and an if/else branch, failure recovery (onFail edges with max attempts, exhaustion behavior, and compatibility with failOnce and loop edges), templates, `define` / `runNamed`, the matcher, ephemeral spawn and cleanup, a stopped run with its settled node map (#1424), the reply middleware, the client canvas, the settings page (including a condition and a loop), rename and delete (including factory-id rows such as `delivery`), the stage chat's scroll rule (a new visible line follows, a filtered node reply moves nothing), the assign dialog's preselected card and its failed and empty board states (#1457, #1458), a sidebar click that leaves the Workflows section in place, and the claim rules — a re-routed card moves with one claim and one route, the same target writes nothing, a trigger never invalidates, the held list is capped and omits a Bot claim, the held page answering `heldLimit` up to its 200 cap and the stage unfolding it behind its `wf-more` row (#2256), `rematch: true` drops one claim, and the panel's one re-match action goes through `ingest` (#1647), and the assigned brief reaching the step prompt while a task without one still gets its title, a parked card keeping its brief on disk, and an unknown `assign` field coming back in `ignored` (#1869). Run `npm test` in `plugins/workflow`. The host seam that applies `ownerBotId` is `core/test/task-route.test.js`, which also pins that a card written through `track_tasks` reaches the listener (#1462). The host omission of run Bots, and the grant of `stepTools` plus the hidden-approval answer, is `core/test/flybot.test.js`. Which connectors a run Bot may call is `core/test/connector-access.test.js`, and the chain from a node's `tools` through `spawn` to that guard — the built-in delivery nodes included — is `core/test/workflow-node-connectors.test.js`. +`plugins/workflow/test/`: graph advance and join, graph edit, assign and persistence, the FIFO queue and concurrency cap, Bot-reply advance, a dispatch that fails without being marked handed, a bounded hand-off retry and a step deadline that fails the run and clears its timer (#1530), the three hangs a host call can park the tail with — a dispatch, a release and a spawn that never answer — ending `failed` at the deadline and putting the queued card back (#2748), a lane that stays under `maxConcurrent` when two steps end in the same turn (#2748), a per-workflow `stepTimeoutMinutes` that replaces the ten-minute constant and is refused outside 1..120 (#1580), disposing releases the Bots before the record drops their ids and a released runner name is reusable again, assign-to-bot versus assign-to-workflow, inputs and outputs along edges, a loop that breaks and an if/else branch, failure recovery (onFail edges with max attempts, exhaustion behavior, and compatibility with failOnce and loop edges), templates, `define` / `runNamed`, the matcher, ephemeral spawn and cleanup, a stopped run with its settled node map (#1424), the reply middleware, the client canvas, the settings page (including a condition and a loop), rename and delete (including factory-id rows such as `delivery`), the stage chat's scroll rule (a new visible line follows, a filtered node reply moves nothing), the assign dialog's preselected card and its failed and empty board states (#1457, #1458), a sidebar click that leaves the Workflows section in place, and the claim rules — a re-routed card moves with one claim and one route, the same target writes nothing, a trigger never invalidates, the held list is capped and omits a Bot claim, the held page answering `heldLimit` up to its 200 cap and the stage unfolding it behind its `wf-more` row (#2256), `rematch: true` drops one claim, and the panel's one re-match action goes through `ingest` (#1647), and the assigned brief reaching the step prompt while a task without one still gets its title, a parked card keeping its brief on disk, and an unknown `assign` field coming back in `ignored` (#1869). Run `npm test` in `plugins/workflow`. The host seam that applies `ownerBotId` is `core/test/task-route.test.js`, which also pins that a card written through `track_tasks` reaches the listener (#1462). The host omission of run Bots, and the grant of `stepTools` plus the hidden-approval answer, is `core/test/flybot.test.js`. Which connectors a run Bot may call is `core/test/connector-access.test.js`, and the chain from a node's `tools` through `spawn` to that guard — the built-in delivery nodes included — is `core/test/workflow-node-connectors.test.js`. ## Connector ceiling (#1544) diff --git a/plugins/workflow/src/service.js b/plugins/workflow/src/service.js index b13f07055..527e53b3c 100644 --- a/plugins/workflow/src/service.js +++ b/plugins/workflow/src/service.js @@ -1037,13 +1037,28 @@ export function createWorkflowService({ } } + // #2773: a card leaves the queue before its run exists — the host is asked to create the run Bots + // first — and `runningOf()` counts begun runs only. Two drains that reach one lane in the same turn + // therefore both read the free slot and each starts a card, taking the lane past `maxConcurrent` + // (the race that removing the serial tail exposed). A start in flight is counted here from the + // moment its card is taken until `beginRun()` puts the run where `runningOf()` sees it; every way + // out of a start releases it. It is never persisted: it lasts one start. + const startsInFlight = new Map() + const startsOf = id => startsInFlight.get(id) ?? 0 + const holdStart = id => startsInFlight.set(id, startsOf(id) + 1) + const releaseStart = id => { + const left = startsOf(id) - 1 + if (left > 0) startsInFlight.set(id, left) + else startsInFlight.delete(id) + } + // The oldest queued task whose own workflow still has a slot, or null. function takeNext() { let best = null for (const item of state.workflows) { const lane = laneOf(item.id) if (lane.queue.length === 0) continue - if (runningOf(lane).length >= maxOf(item)) continue + if (runningOf(lane).length + startsOf(item.id) >= maxOf(item)) continue const task = lane.queue[0] const at = Number(task.enqueuedAt) || 0 const order = Number(task.queueSeq) || 0 @@ -1237,39 +1252,55 @@ export function createWorkflowService({ } async function startLive(item, task) { - let ephemeral - if (typeof spawn === 'function') { - const holes = missingProviders(item) - if (holes.length > 0) return { ok: false, error: `Node ${holes[0].nodeId} input ${holes[0].name} has no upstream provider` } - try { - // #1544: the caller that started the run and the Bot that saved the graph cap the connectors - // this run may use; the host intersects them with each node's `tools`. - ephemeral = await boundedSpawn(spawnAll(item.nodes, bots() ?? [], spawn, item.model ?? '', { - callerBotId: task.callerBotId, - authorBotId: item.authoredBy, - })) - } catch (error) { - await releaseAll(error.spawned ?? [], release, warn) - return refuse(error) - } + // #2773: this lane's slot is claimed here, before the first wait, and given back the moment + // `beginRun()` makes the run visible to `runningOf()` — or on any way out that never gets that + // far. Holding it any longer (through the hand-off) would keep a slot counted that nothing is + // using, and a dispatch that never answers would hold it for good. + holdStart(item.id) + let holding = true + const releaseHeld = () => { + if (!holding) return + holding = false + releaseStart(item.id) } - const begun = beginRun(item, task, ephemeral) - if (begun.ok === false) { - if (ephemeral) await releaseAll(Object.values(ephemeral), release, warn) - return begun - } - let run = begun.run - if (typeof spawn === 'function') { - try { - run = await handOff(item, run) - } catch (error) { - endRun(item, run, { code: 'dispatch-failed', text: String(error?.message ?? error).slice(0, 200) }) - await releaseRun(item.id, run) - return refuse(error) + try { + let ephemeral + if (typeof spawn === 'function') { + const holes = missingProviders(item) + if (holes.length > 0) return { ok: false, error: `Node ${holes[0].nodeId} input ${holes[0].name} has no upstream provider` } + try { + // #1544: the caller that started the run and the Bot that saved the graph cap the connectors + // this run may use; the host intersects them with each node's `tools`. + ephemeral = await boundedSpawn(spawnAll(item.nodes, bots() ?? [], spawn, item.model ?? '', { + callerBotId: task.callerBotId, + authorBotId: item.authoredBy, + })) + } catch (error) { + await releaseAll(error.spawned ?? [], release, warn) + return refuse(error) + } } + const begun = beginRun(item, task, ephemeral) + releaseHeld() + if (begun.ok === false) { + if (ephemeral) await releaseAll(Object.values(ephemeral), release, warn) + return begun + } + let run = begun.run + if (typeof spawn === 'function') { + try { + run = await handOff(item, run) + } catch (error) { + endRun(item, run, { code: 'dispatch-failed', text: String(error?.message ?? error).slice(0, 200) }) + await releaseRun(item.id, run) + return refuse(error) + } + } + if (run.status === 'done' || run.status === 'failed') await releaseRun(item.id, run) + return present() + } finally { + releaseHeld() } - if (run.status === 'done' || run.status === 'failed') await releaseRun(item.id, run) - return present() } function park(item, task) { @@ -1303,7 +1334,7 @@ export function createWorkflowService({ } function hasRoom(item) { - return runningOf(laneOf(item.id)).length < maxOf(item) + return runningOf(laneOf(item.id)).length + startsOf(item.id) < maxOf(item) } // A non-empty queue is joined at the tail, then the oldest task starts. diff --git a/plugins/workflow/test/cap-concurrent-drain.test.js b/plugins/workflow/test/cap-concurrent-drain.test.js new file mode 100644 index 000000000..e706af7ed --- /dev/null +++ b/plugins/workflow/test/cap-concurrent-drain.test.js @@ -0,0 +1,138 @@ +// #2748 S3 (Test advisory on #2773): taking the step deadline off the serial tail let two drains +// reach one lane in the same turn. A card leaves the queue before its run exists (the host is asked +// to create the run Bots first) and `runningOf()` counts begun runs only, so both drains read the +// free slot and each starts a card: `maxConcurrent` 1 with `activeCount` 2. Red on +// cc35bf484932aba44e24b02a6e3d978495f09aa0; the fix counts a start in flight from the card to +// `beginRun()`. +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { STEP_DEADLINE_MS, createWorkflowService } from '../src/service.js' + +const CLOCK = 1_700_000_000_000 +const ROSTER = [{ id: 'main', name: 'Chief', role: 'lead', isMain: true, remote: false }] + +// Absolute time: a timer keeps the instant it is due, so firing one never moves another, and a +// timer created later is due later than it. `nextAt` takes the earliest timer inside a fixed +// window, which is what lets this test fire the two step deadlines and not the spawn bound the +// first drain arms afterwards. +function scheduler() { + let seq = 0 + let now = 0 + const queue = new Map() + const setTimer = (fn, ms) => { + const id = ++seq + queue.set(id, { fn, due: now + Math.max(1, Number(ms) || 0) }) + return id + } + const clearTimer = id => { queue.delete(id) } + function nextAt(target) { + let next + for (const [id, entry] of queue) { + if (entry.due > target) continue + if (!next || entry.due < next.entry.due) next = { id, entry } + } + if (!next) return null + queue.delete(next.id) + now = next.entry.due + return next.entry.fn + } + return { setTimer, clearTimer, nextAt, pending: () => queue.size } +} + +const settle = async (rounds = 60) => { for (let i = 0; i < rounds; i += 1) await new Promise(done => setImmediate(done)) } + +function harness() { + const spawned = [] + const released = [] + const warnings = [] + const timers = scheduler() + const pending = [] + let deferred = false + let seq = 0 + const svc = createWorkflowService({ + persist: false, + now: () => CLOCK, + bots: () => ROSTER, + spawn: (spec) => { + if (deferred) return new Promise(resolve => pending.push({ spec, resolve })) + const id = `eph-${spec.nodeId}-${++seq}` + spawned.push({ id, spec }) + return Promise.resolve(id) + }, + release: async (id) => { released.push(id) }, + dispatch: async () => ({ ok: true }), + setTimer: timers.setTimer, + clearTimer: timers.clearTimer, + warn: (line) => warnings.push(String(line)), + }) + return { + svc, timers, pending, spawned, released, warnings, + defer(value) { deferred = value }, + } +} + +const make = (svc, name) => { + const made = svc.define({ + name, + nodes: [{ id: 'plan', name: 'Planner', role: 'plan', prompt: 'Plan.' }], + edges: [], + }) + assert.equal(made.ok, true, JSON.stringify(made)) + return made.workflows.find(item => item.name === name) +} + +const row = (svc, id) => svc.state().workflows.find(item => item.id === id) +const tasksOf = row => { + const names = new Set() + for (const run of row.runs ?? []) if (run.task?.id) names.add(run.task.id) + for (const task of row.queue ?? []) if (task.id) names.add(task.id) + return names +} + +test('#2748 S3: two drains that reach one lane in the same turn keep it under maxConcurrent', async () => { + const host = harness() + const { svc, timers, pending } = host + const flow = make(svc, 'Cap') + const other = make(svc, 'Other') + svc.setConcurrency({ workflowId: flow.id, maxConcurrent: 1 }) + await svc.assign({ workflowId: flow.id, taskId: '#S3a', title: 'Running' }) + await svc.assign({ workflowId: flow.id, taskId: '#S3b', title: 'Second' }) + await svc.assign({ workflowId: flow.id, taskId: '#S3c', title: 'Third' }) + await svc.assign({ workflowId: other.id, taskId: '#S3d', title: 'Other lane' }) + const seeded = row(svc, flow.id) + assert.equal(seeded.activeCount, 1, 'one live run holds the only slot') + assert.equal(seeded.queueDepth, 2) + + // Both live runs reach their deadline in the same turn, and the host holds every spawn, so each + // drain is still waiting for its Bots when the other one looks for a card. + host.defer(true) + for (let n = 0; n < 4; n += 1) { + const fire = timers.nextAt(STEP_DEADLINE_MS + 1) + if (!fire) break + Promise.resolve(fire()).catch(() => {}) + await settle() + } + const mid = row(svc, flow.id) + assert.ok(mid.activeCount <= 1, `the cap holds while the starts are in flight (activeCount ${mid.activeCount})`) + + // The host answers every spawn: what the lane counts is what exists now. + for (let guard = 0; guard < 8 && pending.length > 0; guard += 1) { + pending.shift().resolve(`e-${++guard}`) + await settle() + } + await settle() + const after = row(svc, flow.id) + assert.equal(after.activeCount, 1, `one run at maxConcurrent 1, not ${after.activeCount}`) + assert.ok(tasksOf(after).has('#S3c'), 'the card the cap kept out is parked, not dropped: ' + [...tasksOf(after)].join(',')) + assert.ok((after.queue ?? []).some(task => task.id === '#S3c'), 'and it is still the next card in the queue') + + // Nothing drains a parked card by itself: ending the live run is what frees the slot. + host.defer(false) + const live = after.runs.find(run => run.task?.id && run.task.id !== '#S3c') + assert.ok(live, 'the run holding the slot is on the row') + await svc.reset({ workflowId: flow.id, runId: live.id }) + await settle() + const restarted = row(svc, flow.id) + assert.equal(restarted.activeCount, 1) + assert.equal(restarted.runs.find(run => run.status === 'running')?.task?.id, '#S3c', 'the parked card starts once the slot frees') +})