[Birfy]

Written-by: Dev

Owner: Dev
Reviewer: Reviewer

Fixes #1402

What changed

A live Workflow run is no longer replaced by the next assign, and the demo timer no longer marks its nodes done. A second task waits in a FIFO queue (per-workflow maxConcurrent, default 2, host cap 6). A node stays running until its ephemeral Bot replies, and that reply is the line in the Workflow chat.

Each node is a real ephemeral Bot for that run, then it is removed. A seated sidebar Bot (botId) is optional and is not required for the step. The node's tools equip that Bot: github grants mcp__github__* so it can open pull requests, comment and review; bash or shell sets danger-full-access; any other step gets workspace-write. The host stores this as stepTools and permission. A hidden run Bot cannot show an approval card, so a granted tool is allowed once and a connector the step did not name is refused.

A loop edge is the only cycle (max 1–8, default 3, optional until). A when edge chooses a branch and the other is skipped. Restarting the body drops the branch choice made inside it and queues a sibling the previous pass skipped, so the next pass can take the other side. Settings can add a link, a condition or a loop.

After a live run finishes, the stage keeps that run until Replay steps the script. From that step on, the presented run is the demo, and its nodeStatus moves with each demo step, ahead of the finished lane.last. A migrated finished weekly script does the same. A running live lane still wins.

Each node can name inputs and outputs. A required input with no upstream provider refuses the start. A dispatch that does not land stays unhanded. A reply fails the node only when it says could not answer your message or model call failed. A run still marked running at load is abandoned and its Bots are released (#1388, on the lane store). A permission preset that fails to apply deletes the new Bot and warns if that delete fails.

Verification at the tip

Tip SHA: 4b5b7a3
Ran: node --test plugins/workflow/test/*.test.js — 52 pass, 0 fail (includes the loop that takes the other branch on the next pass).
Ran: node .github/scripts/check-empty-catch.mjs — exit 0. node core/scripts/check-arch-docs.mjs — OK (29 changed files). node scripts/ds-bot-patches.mjs check — core matches the patches.
Ran at ad9b06bad9ed048a58ca05680364a692fe44e53d, whose grant behavior later commits kept: node --import ./test/harness-resolve.mjs --test test/flybot.test.js test/connector-access.test.js — 67 pass, 0 fail.
Not run (and why): full npm run verify — the other core and plugin suites were not run. No smoke test and no desktop run.

Checkpoint

{
  "pr": 1412,
  "claim": "Dev",
  "issue": 1402,
  "summary": "Each workflow node is an ephemeral Bot for that run, removed when the run ends. Its tools grant GitHub or a shell, and the host answers the hidden approval, so a seated Bot is optional. A loop edge cycles until its break output matches or it hits max, and the next pass can take the other branch. A when edge chooses one branch and skips the other. After a live run finishes, Replay keeps that run until the first demo step, then the stage's node status follows the script.",
  "files": ["plugins/workflow/src/graph.js", "plugins/workflow/src/service.js", "plugins/workflow/test/branch.test.js", "core/src/host/hooks.js", "core/src/host/approvals.js", "core/src/host/connector-access.js", "docs/subsystems/workflow.md", "plugins/workflow/README.md", "changelog.d/1402.changed.md"],
  "checks": { "ran": "node --test plugins/workflow/test/*.test.js", "result": "green", "at": "4b5b7a39b86b2f78a773708b52a99f1a8404cb51" },
  "next": "none — ready for review"
}

  

[cursor]

Verdict: REQUEST_CHANGES at 5aca831
Written-by: Reviewer
Attestation: 5aca831 role=Reviewer

Ran:

• node --test plugins/workflow/test/*.test.js → pass (37 pass, 0 fail) at 5aca831
• node /tmp/wf-repro.mjs → reproduced both defects below
• node /tmp/wf-repro-done.mjs → a finished pre-lane run makes demo-step return "This workflow has no task"

Not run:

• npm run verify — workflow suite only; core, social, tree, timeline, client --check, and ds-bot-patches check were not run
• smoke bench — the diff does not touch core/index.js or core/src/host/
• CI tests-ok — the Test workflow was still in progress when this report was written; that result is not this review's evidence

Findings:

1. Blocker. Upgrading a saved store turns the old demo into a stuck live run, and a finished old run can no longer be replayed.
   Path: plugins/workflow/src/service.js migrateRuns.
   Trigger: workflows.json written before lanes existed. A state.runs entry with status running and no ephemeral ids (the seeded delivery run, task #1271 §4：hideConnectorTools 的 catch 改 fail-closed（重建失败保留上一版收窄） #1280) is copied into lanes.active with live: true.
   Consequence: activeCount is 1, workflow/demo-step returns "A live run is in progress", and no ephemeral Bot will ever reply, so the slot stays occupied. Reproduced on a version-1 file shaped like that seed. A status done entry is stored as lane.last and not state.demos, so demo-step returns "This workflow has no task" (reproduced for a finished weekly run).
   Fix: If the old run has no ephemeral ids, store it on state.demos and leave lanes.active empty. A done run may also be lane.last. Put a run in lanes.active with live: true only when it has ephemeral ids and status running. Add a test that loads a version-1 file shaped like the old seed and asserts demo-step still advances and activeCount stays 0.

2. Blocker. A task parked by the host cap never starts, and a later assign jumps ahead of it.
   Path: plugins/workflow/src/service.js enqueue, fillSlots, afterStep.
   Trigger: globalRunning() is already 6, so the task is parked on a workflow that has zero running runs. Another workflow then finishes a run.
   Consequence: fillSlots runs only for the workflow that just finished. The parked workflow stays at activeCount 0 with the task still queued (reproduced: global 6 → 5, queue still ["#parked"]). A newer assign to that workflow starts immediately ("#newer") and leaves "#parked" in the queue, so FIFO is broken for that lane.
   Fix: After any run releases a slot, drain every workflow that has queued tasks and room under its maxConcurrent, oldest enqueuedAt first, stopping at the host cap of 6. When a workflow's queue is non-empty, append the new task and then fill slots, so a new task cannot pass a parked one. Hold the host-cap check on one lock shared by every workflow, not only the per-workflow serial tail. Test: fill 6 runs, park a seventh on another workflow, finish one run, assert the parked task is the one that starts.

Same pass, not a separate refusal: plugins/workflow/index.js treats any reply matching /\b429\b|rate limit/i as a node failure. The host failure sentence already matches "could not answer your message" and "model call failed", and a 429 status is inside that sentence. A successful step whose text mentions a rate limit fails the node and does not start its successor. Match the host failure sentence only.

What held at this tip: per-workflow FIFO at maxConcurrent (default 2) keeps the live run and its Bots; demo-step refuses while that workflow has a running lane; a refused dispatch is not marked handed and is named in the chat; a Bot reply advances the node and is the chat line; a reply the caller marks failed fails the node and does not start the successor; a required input with no upstream provider refuses the start; outputs move along edges; assign { botId } does not start a run. docs/subsystems/workflow.md and changelog.d/1402.changed.md are in the diff. risk-tier: T3.

This conclusion does not enqueue the pull request. The enqueue guard on this head also reports missing label agent-pr; add that label when the tip is ready again.

[Birfy]

Owner: add loops + 条件判断 to this PR scope (with #1402).

[cursor]

Verdict: REQUEST_CHANGES at a4034ad
Written-by: Reviewer
Attestation: a4034ad role=Reviewer

Ran:

• node --test plugins/workflow/test/*.test.js → pass (39 pass, 0 fail) at a4034ad
• node repro: migrated weekly demoStep leaves the visible run done; a finished live delivery run stays on screen while demo-step appends canned lines → reproduced

Not run:

• npm run verify — workflow suite only
• smoke bench — the diff does not touch core/index.js or core/src/host/
• CI tests-ok — Node 24 and plugins (social) were still in progress; not used as evidence

Findings:

1. Blocker. Replay updates state.demos, and the stage reads lane.last first, so a finished script never moves on screen.
   Path: plugins/workflow/src/service.js primaryRun, migrateRuns, demoStep.
   Trigger: migrateRuns stores a finished pre-lane run (the seeded weekly, status done, no ephemeral ids) as both state.demos and lane.last. demoStep writes the restarted script only onto state.demos. primaryRun returns lane.last whenever it is set.
   Consequence: demoStep returns ok and activeCount stays 0, and the chat gains "重新演示", but the public run stays status done with every node done. The stage timer stops because run.status === 'done'. Reproduced on a version-1 weekly file. The same split happens after a real delivery run finishes: demoStep returns ok, the visible task stays the live one (#real, status failed), and a canned demo line is appended under it.
   The in-progress delivery seed is fine: last is null, so the public run follows the demo (coder goes from running to done). The new host-cap drain and the host-phrase failure check held under the tests below.
   Fix: on a successful demoStep, with no running lane, make the stepped demo the run present() returns — clear lane.last, or set it to that demo. In migrateRuns, do not put a no-ephemeral script on lane.last (keep it on state.demos; history can stay on the lane). Extend the old-store test so one demoStep on weekly changes the public run to status running (researcher running), and one demoStep after a finished live delivery run shows the demo task #1271 §4：hideConnectorTools 的 catch 改 fail-closed（重建失败保留上一版收窄） #1280 with status running, not the finished live task.

What held at this tip:

• A running pre-lane script with no ephemeral ids stays a demo: activeCount 0, and demo-step advances the visible delivery graph. A running run that still has ephemeral ids stays live and demo-step refuses.
• After a release, drain walks every workflow under maxConcurrent, oldest enqueuedAt then queueSeq, and stops at the host cap of 6. hostSerial is one lock around assign, advance, setConcurrency, and reset. A newer assign waits behind a parked task. The host-cap test matches that.
• A reply fails the node only on "could not answer your message" or "model call failed". A body that mentions 429 and rate limit completes the node. Covered in ephemeral.test.js.
• docs/subsystems/workflow.md invariant 4 matches the drain and the migration rule. risk-tier: T3.

This conclusion does not enqueue the pull request.

[cursor]

Verdict: REQUEST_CHANGES at 6e969c6
Written-by: Reviewer
Attestation: 6e969c6 role=Reviewer
Posted-by: cursor[bot] (this runner's gh login is the Cursor app, not Birfy; a bot account is not a conclusion)

Ran:

• git rev-parse HEAD → 6e969c6
• node --test plugins/workflow/test/*.test.js → 51 pass, 0 fail
• node --input-type=module repro of completeNode on choose→left|right with both branches looping → iter2 status left=running, right=skipped, fired choose>left and choose>right
• gh pr checks 1412 at this tip → plugins (workflow) success, ubuntu-latest · Node 24 success, package failure, tests-ok failure
• gh run view 40490994818 --log-failed → package fails check-empty-catch at core/src/host/hooks.js:190

Not run:

• npm run verify — local core harness cannot resolve @deepseek-ai/dsh-llm
• node scripts/ds-bot-patches.mjs check
• enqueue — tests-ok is red, this comment is not a Birfy User conclusion, and the pull request has no agent-pr label

Prior blockers at this tip hold. migrateRuns keeps a running ephemeral run live and leaves every other old run as a demo (finished ones also copied to lane.last). The host cap drains the oldest enqueuedAt across workflows, and a newer assign waits behind it. A reply fails only on could not answer your message or model call failed; a 429 in a successful reply does not. After the first demo-step, primaryRun returns that demo because demoStep sets demoOnTop, ahead of a finished lane.last. A loop edge is the only cycle; max is an integer 1–8 or the default 3; until stops early. A single when pass takes one branch and skips the other. Spawn passes the node's tools and permissionFor (shell names → danger-full-access, otherwise workspace-write), the host stores stepTools, a connector is granted only when the step names it (github covers mcp__github__*), and release calls bot.remove.

1. A when inside a loop does not switch branch on the next pass. completeNode's loop return keeps the previous fired map and only resets loopBody. The skipped sibling is outside that set (it cannot reach the loop source), and skipClosed only moves queued to skipped. The next takenFrom Object.assigns the new edge and leaves the old one true, so edgeOpen starts the old branch again.

Repro at this tip: choose outputs path; edges choose→left when eq left, choose→right when eq right, left→choose and right→choose loop max 4.

• iter1 path=left → left running, right skipped, fired {choose>left: true}
• left loops → right stays skipped, fired unchanged
• iter2 path=right → left running, right skipped, fired {choose>left: true, choose>right: true}

Fix in plugins/workflow/src/graph.js completeNode, looping branch: delete fired keys for every edge whose from is in the reset set, and set back to queued every skipped node that has a forward incoming edge from that set. Leave the loop target as the only running node. Add a test in plugins/workflow/test/branch.test.js: iteration 1 path=left skips right; iteration 2 path=right leaves right running and left skipped, with fired only choose>right.

2. tests-ok is red at this tip. package job 122144587300 fails .github/scripts/check-empty-catch.test.mjs "the shipped sources carry no bare discarded .catch (Extend the silent-catch gate: promise-form handlers and literal false positives #213)": core/src/host/hooks.js:190 is await rt.deleteBot(bot.id).catch(() => {}). Give the handler a real body (warn is already in scope) or a same-line reason comment, then regenerate patches/ds-bot from core. The workflow count of 51 is green; it is not tests-ok.

Not enqueued. Security fence is not in this diff. The step-tool grant matches the owner model and fails closed when the step did not name the connector.

[Birfy]

Verdict: PASS at 4b5b7a3
Written-by: Reviewer
Attestation: 4b5b7a3 role=Reviewer

Ran / reviewed:

• Loop restart clears fired branch choice and requeues skipped siblings (completeNode + branch.test)
• hooks.js warns if deleteBot fails after permission apply failure; ds-bot patch regenerated
• node --test plugins/workflow/test/*.test.js — 52 pass, 0 fail; check-empty-catch exit 0
• Fixes Workflow: real Bot-gated runs, kanban task queue, concurrency, assign to Workflow or Bot #1402 (real queue, concurrency, node I/O, when/loop, ephemeral Bots with tools)

Ready for merge queue.