From 83d202f0186648884f9e1d20a7215d0b162f6d1f Mon Sep 17 00:00:00 2001 From: Danyang Chen Date: Wed, 28 Oct 2026 17:22:20 +0800 Subject: [PATCH] fix(workflow): give a step the time its work needs A live step used to be killed after ten minutes (#1580). Implementing or reviewing a change takes twenty to forty, so every step that did real work failed mid-flight and the run was lost. The deadline is now the workflow step timeout, an integer from 1 to 120 minutes with 45 as the default; a value outside that range is refused when the workflow is written, and the timeout message names the number that applied. --- changelog.d/1580-step-deadline.fixed.md | 1 + docs/subsystems/workflow.md | 8 +-- plugins/workflow/index.js | 1 + plugins/workflow/src/service.js | 47 +++++++++++++++- plugins/workflow/test/deadline.test.js | 74 ++++++++++++++++++++++++- 5 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 changelog.d/1580-step-deadline.fixed.md diff --git a/changelog.d/1580-step-deadline.fixed.md b/changelog.d/1580-step-deadline.fixed.md new file mode 100644 index 000000000..fd2d0ceac --- /dev/null +++ b/changelog.d/1580-step-deadline.fixed.md @@ -0,0 +1 @@ +**Workflow steps get the time the work needs (#1580):** a live step used to be killed after ten minutes, and ten minutes is shorter than implementing or reviewing a change takes, so every step that did real work failed mid-flight and the run was lost. The deadline is now per workflow: `stepTimeoutMinutes`, an integer from 1 to 120, given when the workflow is created or through the new `workflow/step-timeout` call, with 45 minutes as the default. A value outside that range is refused when it is written instead of silently falling back, and the timeout message names the number that applied. diff --git a/docs/subsystems/workflow.md b/docs/subsystems/workflow.md index 117fb5ab1..632e2672e 100644 --- a/docs/subsystems/workflow.md +++ b/docs/subsystems/workflow.md @@ -28,7 +28,7 @@ Everything in `plugins/workflow/`: the graph rules, the run store, the matcher, | --- | --- | | `plugins/workflow/index.js` | Plugin entry (`flybot-workflow`). Endpoints, `run_workflow`, `define_workflow`, the board listener, and the reply middleware that ingests a step result and denies delivery to persistent Bot chats. | | `plugins/workflow/src/graph.js` | Nodes, edges, positions, and named inputs and outputs. A node starts when its forward predecessors are settled and a taken edge reaches it. Same-rank nodes run together. A forward cycle is refused. A loop edge is the only cycle for success paths. A `when` edge chooses a branch; the other is skipped. A required input with no upstream provider is refused before a run starts. An `onFail` edge provides failure recovery: when a node fails with recovery attempts remaining, the workflow resets to the upstream target and re-runs through the failing node (tracked by `failAttempts` per edge, with a configurable `max` bound, 1–8, default 3). After attempts are exhausted, the node fails permanently. | -| `plugins/workflow/src/service.js` | The store, lanes (active runs, FIFO queue, terminal history), `maxConcurrent`, `rename`, `delete`, `saveGraph`, `define`, `runNamed`, `ingest`, `stepResult`. `rename` updates a workflow's name and optional description; `delete` removes the workflow, releases ephemeral Bots, and cleans logs/runs/routes. Factory template ids (e.g. `delivery`) are normal saved rows and may be deleted. A spawn hook makes one run Bot per node. Disposing the plugin ends every live run still marked `running` (nodes in flight are failed, the run is archived) and then releases every run Bot and drops those ids from the saved record only once the release has finished, handing the release back as a promise the host awaits, so a shutdown cannot persist a run as `running` with no ephemeral map and cannot leave hidden Bots that no state names (#1387, #1414). A released run Bot also gives its runner name back to the allocator instead of keeping it for the lifetime of the process. A dispatch that does not land is not marked handed: it is retried twice with a short backoff, and a step still unanswered after `STEP_DEADLINE_MS` (10 minutes) fails the run and releases its Bots (#1530). | +| `plugins/workflow/src/service.js` | The store, lanes (active runs, FIFO queue, terminal history), `maxConcurrent`, `rename`, `delete`, `saveGraph`, `define`, `runNamed`, `ingest`, `stepResult`. `rename` updates a workflow's name and optional description; `delete` removes the workflow, releases ephemeral Bots, and cleans logs/runs/routes. Factory template ids (e.g. `delivery`) are normal saved rows and may be deleted. A spawn hook makes one run Bot per node. Disposing the plugin ends every live run still marked `running` (nodes in flight are failed, the run is archived) and then releases every run Bot and drops those ids from the saved record only once the release has finished, handing the release back as a promise the host awaits, so a shutdown cannot persist a run as `running` with no ephemeral map and cannot leave hidden Bots that no state names (#1387, #1414). A released run Bot also gives its runner name back to the allocator instead of keeping it for the lifetime of the process. A dispatch that does not land is not marked handed: it is retried twice with a short backoff, and a step still unanswered after its workflow's `stepTimeoutMinutes` (45 minutes by default) fails the run and releases its Bots (#1530, #1580). | | `plugins/workflow/src/ephemeral.js` | Builds each node's run Bot from its prompt, role, model and tools, and releases the ids the host returns. `node.tools` names the connectors that node may call: `spawn` passes the cleaned list to `fb.bot.create`, the host records it as `stepTools`, and `connector-access.js` allows only the connectors it names (`github` covers `mcp__github__*`, `composio` covers `mcp__composio__*`) — a node that names none reaches none. File and shell rights do not come from that list: every node Bot gets the `danger-full-access` preset a normal sidebar Bot has, and `EPHEMERAL_DENIED_TOOLS` still refuses the team tools at the guard and approval layers. The list is also stored in `fields.tools` for the UI. | | `plugins/workflow/src/match.js` | Which open card matches a Workflow or a local Bot. | | `plugins/workflow/src/template.js` | Factory delivery/weekly graphs plus `EARLY_END_GUIDANCE` for Claim/Verify/Triage early-end markers; 发布交付流水线 (a parallel branch) and the weekly report, each node with the inputs and outputs that edge carries. | @@ -38,7 +38,7 @@ Everything in `plugins/workflow/`: the graph rules, the run store, the matcher, ## Entry points - Endpoint `workflow/state` and `workflow/templates` are reads. -- `workflow/create`, `workflow/rename`, `workflow/delete`, `workflow/assign`, `workflow/concurrency`, `workflow/graph`, `workflow/advance`, `workflow/reset`, `workflow/ingest` write. `workflow/concurrency` sets that workflow's `maxConcurrent` (an integer from 1 to 8); the same control is in Settings → Workflow graphs. `workflow/rename` updates a workflow's name and optional description; read-only hosts are refused. `workflow/delete` removes a workflow from state, releases its ephemeral Bots if a run exists, and cleans logs/runs/routes. +- `workflow/create`, `workflow/rename`, `workflow/delete`, `workflow/assign`, `workflow/concurrency`, `workflow/graph`, `workflow/advance`, `workflow/reset`, `workflow/ingest` write. `workflow/concurrency` sets that workflow's `maxConcurrent` (an integer from 1 to 8); the same control is in Settings → Workflow graphs. `workflow/step-timeout` sets that workflow's `stepTimeoutMinutes` (an integer from 1 to 120, or `null` to go back to the default), and `workflow/create` accepts the same field. `workflow/rename` updates a workflow's name and optional description; read-only hosts are refused. `workflow/delete` removes a workflow from state, releases its ephemeral Bots if a run exists, and cleans logs/runs/routes. - Tool `run_workflow` starts a named run with a brief. Tool `define_workflow` writes the same graph the canvas edits. - `flybot.tasks.onWrite` calls `ingest`. A new Bot match returns `{ ownerBotId }`. A Workflow match starts the run inside the plugin and leaves the card's owner to the host. @@ -46,7 +46,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). 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). -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):** When a node completes **or fails** with `needs-human`, `already-claimed`, `tip-unchanged`, or `open-pr` detected in its outputs or reply text (see `checkEarlyEnd` in Key files), 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. Preferred signals are a declared OUTPUT equal to the marker, or a whole reply line `EARLY-END `; the engine also honors common phrases (`tip unchanged` / `unchanged tip`, `has open PR` / `already has open PR` / `found open PR`, plus the existing already-claimed heuristics) so a Bot that forgets the structured marker cannot soft-lock Claim↔Verify on the same tip. 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. 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 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). 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 — `STEP_DEADLINE_MS`, ten minutes, one constant in `service.js` (armed even when the host cannot spawn ephemeral Bots); when it expires the node and the run fail as `failed`, the run's ephemeral Bots are released, the chat says the step timed out, and `warn` names it once. The deadline is cleared as soon as the step moves, and by Pause, Stop, Reset, delete, `reap` and `dispose`; a paused run 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 that says `could not answer your message` or `model call failed` fails that node and does not start its successor. A 429 or rate-limit mention in any other reply does not. +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):** When a node completes **or fails** with `needs-human`, `already-claimed`, `tip-unchanged`, or `open-pr` detected in its outputs or reply text (see `checkEarlyEnd` in Key files), 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. Preferred signals are a declared OUTPUT equal to the marker, or a whole reply line `EARLY-END `; the engine also honors common phrases (`tip unchanged` / `unchanged tip`, `has open PR` / `already has open PR` / `found open PR`, plus the existing already-claimed heuristics) so a Bot that forgets the structured marker cannot soft-lock Claim↔Verify on the same tip. 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. 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 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). 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 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 says the step timed out, and `warn` names it once. The deadline is cleared as soon as the step moves, and by Pause, Stop, Reset, delete, `reap` and `dispose`; a paused run 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 that says `could not answer your message` or `model call failed` fails that node and does not start its successor. A 429 or rate-limit mention in any other reply does not. 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, and the chip count does not exceed that list. 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 : `. The Bot reports `OUTPUT : `; a single declared output may be the reply body. 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`. @@ -59,7 +59,7 @@ Everything in `plugins/workflow/`: the graph rules, the run store, the matcher, ## 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), 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`), and a sidebar click that leaves the Workflows section in place. Run `npm test` in `plugins/workflow`. The host seam that applies `ownerBotId` is `core/test/task-route.test.js`. 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), 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`), and a sidebar click that leaves the Workflows section in place. Run `npm test` in `plugins/workflow`. The host seam that applies `ownerBotId` is `core/test/task-route.test.js`. 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/index.js b/plugins/workflow/index.js index db0e4b370..c8e5ee62c 100644 --- a/plugins/workflow/index.js +++ b/plugins/workflow/index.js @@ -104,6 +104,7 @@ export function apply(ctx, config = {}) { fb.api.handle('workflow/delete', body => service.delete(body)), fb.api.handle('workflow/assign', body => service.assign(body)), fb.api.handle('workflow/concurrency', body => service.setConcurrency(body)), + fb.api.handle('workflow/step-timeout', body => service.setStepTimeout(body)), fb.api.handle('workflow/set-model', body => service.setModel(body)), fb.api.handle('workflow/advance', body => service.advance(body)), fb.api.handle('workflow/reset', body => service.reset(body)), diff --git a/plugins/workflow/src/service.js b/plugins/workflow/src/service.js index 6df02791c..8d36641b6 100644 --- a/plugins/workflow/src/service.js +++ b/plugins/workflow/src/service.js @@ -24,9 +24,24 @@ const MAX_CONCURRENT = 8 // (#1530). These are the only two numbers a stuck run depends on: how long one // step may stay unanswered, and how long a hand-off that did not land waits // before it is tried again. Both are cleared with the step they belong to. -export const STEP_DEADLINE_MS = 10 * 60 * 1000 +// Ten minutes was shorter than the work a node is asked to do — implementing or +// reviewing a change takes twenty to forty — so every step that did real work +// was killed mid-flight (#1580). The default covers one such step now, and a +// workflow may set its own within these bounds. +export const DEFAULT_STEP_TIMEOUT_MINUTES = 45 +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] +function stepTimeoutValue(raw) { + const value = Number(raw) + if (!Number.isInteger(value) || value < MIN_STEP_TIMEOUT_MINUTES || value > MAX_STEP_TIMEOUT_MINUTES) { + throw new Error(`stepTimeoutMinutes is an integer from ${MIN_STEP_TIMEOUT_MINUTES} to ${MAX_STEP_TIMEOUT_MINUTES}`) + } + return value +} + function idOf(prefix) { return `${prefix}-${randomBytes(4).toString('hex')}` } @@ -176,7 +191,7 @@ export function createWorkflowService({ function armDeadline(item, run, nodeId) { if (timers.has(`deadline:${run.id}:${nodeId}`)) return - setTimerAt('deadline', run.id, nodeId, STEP_DEADLINE_MS, () => expireStep(item.id, run.id, nodeId)) + setTimerAt('deadline', run.id, nodeId, stepTimeoutMsOf(item), () => expireStep(item.id, run.id, nodeId)) } // Give every running step the deadline it started with and drop the timers of @@ -199,9 +214,9 @@ export function createWorkflowService({ // A step nobody answers is not progress. Fail the run, free its Bots and say // which step timed out, so a dead Bot cannot hold a lane slot forever. function expireStep(workflowId, runId, nodeId) { - const minutes = Math.round(STEP_DEADLINE_MS / 60_000) return hostSerial(async () => { const item = workflow(workflowId) + const minutes = stepTimeoutMinutesOf(item) const lane = laneOf(workflowId) const run = lane.active.find(candidate => candidate.id === runId) if (!item || !run || run.status !== 'running' || run.paused === true) return @@ -278,6 +293,7 @@ export function createWorkflowService({ triggers: [...(item.triggers ?? [])], chain: chainLabel(item), maxConcurrent: maxOf(item), + stepTimeoutMinutes: stepTimeoutMinutesOf(item), activeCount: runningOf(laneOf(item.id)).length, queueDepth: (laneOf(item.id).queue ?? []).length, queue: (laneOf(item.id).queue ?? []).map(task => ({ id: task.id, title: task.title })), @@ -309,6 +325,19 @@ export function createWorkflowService({ return Math.min(MAX_CONCURRENT, value) } + // How long this workflow lets one live step stay unanswered (#1580). A stored + // value outside the bounds can only come from a file edited by hand, so it + // falls back to the default instead of ending the run early. + function stepTimeoutMinutesOf(item) { + const value = Number(item?.stepTimeoutMinutes) + if (!Number.isInteger(value) || value < MIN_STEP_TIMEOUT_MINUTES || value > MAX_STEP_TIMEOUT_MINUTES) return DEFAULT_STEP_TIMEOUT_MINUTES + return value + } + + function stepTimeoutMsOf(item) { + return stepTimeoutMinutesOf(item) * 60_000 + } + function displayRun(id) { const live = runningOf(laneOf(id)) if (live.length > 0) return live[0] @@ -887,6 +916,9 @@ export function createWorkflowService({ if (authoredBy) item.authoredBy = authoredBy const cap = Number(body.maxConcurrent) if (Number.isInteger(cap) && cap >= 1 && cap <= MAX_CONCURRENT) item.maxConcurrent = cap + if (body.stepTimeoutMinutes !== undefined && body.stepTimeoutMinutes !== null && body.stepTimeoutMinutes !== '') { + item.stepTimeoutMinutes = stepTimeoutValue(body.stepTimeoutMinutes) + } state.workflows.push(item) const memberNames = item.nodes.map(node => node.name).join('、') push(item.id, [{ kind: 'system', text: `Workflow「${item.name}」已创建 · 成员 ${memberNames}` }], now()) @@ -931,6 +963,15 @@ export function createWorkflowService({ return present() }) }, + setStepTimeout(body = {}) { + const workflowId = String(body.workflowId ?? '') + const raw = body.stepTimeoutMinutes + const saved = mutate(workflowId, (item) => { + if (raw === null || raw === '') delete item.stepTimeoutMinutes + else item.stepTimeoutMinutes = stepTimeoutValue(raw) + }) + return saved + }, setModel(body = {}) { if (readOnly()) return { ok: false, code: 'read-only', error: 'The host is read-only' } const workflowId = String(body.workflowId ?? '') diff --git a/plugins/workflow/test/deadline.test.js b/plugins/workflow/test/deadline.test.js index 7bc82b87d..f6b0b1077 100644 --- a/plugins/workflow/test/deadline.test.js +++ b/plugins/workflow/test/deadline.test.js @@ -6,7 +6,7 @@ // timers are cleared when the step moves or the run stops. import assert from 'node:assert/strict' import { test } from 'node:test' -import { HANDOFF_RETRY_DELAYS_MS, STEP_DEADLINE_MS, createWorkflowService } from '../src/service.js' +import { DEFAULT_STEP_TIMEOUT_MINUTES, HANDOFF_RETRY_DELAYS_MS, STEP_DEADLINE_MS, createWorkflowService } from '../src/service.js' const CLOCK = 1_700_000_000_000 const ROSTER = [ @@ -317,3 +317,75 @@ test('#1530: a live step with no ephemeral map still expires at the deadline', a assert.ok(warnings.some(line => /timed out|超时/.test(line))) assert.equal(timers.pending(), 0) }) + +// #1580: the deadline is per workflow, and ten minutes was shorter than the work +// a node does, so every implementing or reviewing step was killed mid-flight. +const TIMED_NODES = [ + { id: 'plan', name: 'Planner', role: 'plan', prompt: 'Write the plan.' }, + { id: 'code', name: 'Coder', role: 'code', prompt: 'Implement it.' }, +] + +function makeTimed(svc, minutes, name) { + const made = svc.create({ + name, + nodes: TIMED_NODES, + edges: [{ from: 'plan', to: 'code' }], + ...(minutes === undefined ? {} : { stepTimeoutMinutes: minutes }), + }) + assert.equal(made.ok, true) + return made.workflows.find(item => item.name === name) +} + +test('#1580: the default deadline covers real work, so ten minutes no longer fails a step', async () => { + const { svc, released, timers } = harness() + const flow = makeTimed(svc, undefined, 'Default deadline') + assert.equal(DEFAULT_STEP_TIMEOUT_MINUTES, 45) + assert.equal(row(svc.state(), flow.id).stepTimeoutMinutes, DEFAULT_STEP_TIMEOUT_MINUTES) + + await svc.assign({ workflowId: flow.id, taskId: '#1580-default', title: 'Real work' }) + await timers.tick(10 * 60 * 1000) + assert.equal(row(svc.state(), flow.id).run.status, 'running', 'ten minutes is inside the default') + assert.equal(released.length, 0) + + await timers.tick(STEP_DEADLINE_MS - 10 * 60 * 1000) + const after = row(svc.state(), flow.id) + assert.equal(after.run.status, 'failed') + assert.equal(after.run.nodeStatus.plan, 'failed') + assert.match(chat(svc.state(), flow.id), /45 分钟/) +}) + +test('#1580: a workflow that sets stepTimeoutMinutes times its steps out on its own number', async () => { + const { svc, timers } = harness() + const flow = makeTimed(svc, 20, 'Twenty minute step') + assert.equal(row(svc.state(), flow.id).stepTimeoutMinutes, 20) + + await svc.assign({ workflowId: flow.id, taskId: '#1580-own', title: 'Own number' }) + await timers.tick(20 * 60 * 1000 - 1) + assert.equal(row(svc.state(), flow.id).run.status, 'running') + + await timers.tick(1) + const after = row(svc.state(), flow.id) + assert.equal(after.run.status, 'failed') + assert.match(chat(svc.state(), flow.id), /20 分钟/) +}) + +test('#1580: a stepTimeoutMinutes outside 1..120 is refused when the workflow is written', () => { + const { svc } = harness() + for (const [value, name] of [[0, 'Zero'], [121, 'Too long'], ['abc', 'Not a number'], [-5, 'Negative'], [1.5, 'Fractional']]) { + const made = svc.create({ name, stepTimeoutMinutes: value, nodes: TIMED_NODES, edges: [{ from: 'plan', to: 'code' }] }) + assert.equal(made.ok, false, name + ' must be refused') + assert.match(String(made.error), /stepTimeoutMinutes/) + } + + const made = svc.create({ name: 'Set later', nodes: TIMED_NODES, edges: [{ from: 'plan', to: 'code' }] }) + assert.equal(made.ok, true) + const flow = made.workflows.find(item => item.name === 'Set later') + assert.equal(flow.stepTimeoutMinutes, DEFAULT_STEP_TIMEOUT_MINUTES) + assert.equal(svc.setStepTimeout({ workflowId: flow.id, stepTimeoutMinutes: 999 }).ok, false) + const good = svc.setStepTimeout({ workflowId: flow.id, stepTimeoutMinutes: 30 }) + assert.equal(good.ok, true) + assert.equal(row(good, flow.id).stepTimeoutMinutes, 30) + const cleared = svc.setStepTimeout({ workflowId: flow.id, stepTimeoutMinutes: null }) + assert.equal(cleared.ok, true) + assert.equal(row(cleared, flow.id).stepTimeoutMinutes, DEFAULT_STEP_TIMEOUT_MINUTES) +})