Skip to content

fix(workflow): end a run whose running node the graph edit drops - #1593

Merged
Birfy merged 1 commit into
mainfrom
replay/pr-1551
Oct 28, 2026
Merged

Birfy merged 1 commit into
mainfrom
replay/pr-1551

Conversation

@Birfy

@Birfy Birfy commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1549

Owner: Dev
Reviewer: Reviewer
(Owner: is Dev, the writers name, not the original's Dev-1, which enqueue-guard refuses as a name the policy does not know.)

Replay of #1551: the original head is on a fork we cannot push to (maintainer_can_modify=false), so this PR carries the same change replayed onto current main; the original is closed as superseded.

Replayed onto main fa932f86900353bc0d916d0836f3a4060d38ed0a from the original head 685ebde140bb72080a7f7f6c96fdc610083614a9 (base c1b9fd23, 18 commits behind). Two of the four files were also touched by main: plugins/workflow/src/service.js and docs/subsystems/workflow.md.

How the replay merged

Nothing main landed is reverted.

What changed (4 files, +151/−18)

  • plugins/workflow/src/service.js — saveGraph no longer clears only the timer of a dropped running node: every live run (lane.active plus lane.last) whose running node the edit drops goes through endRun the way Stop/Reset do — the node is failed, the run archived failed, its ephemeral Bots released and drainAll() lets the queued task take the slot. The ended run keeps its own node map (the failed node included), a surviving run settles against the new graph, and the chat gets the same ■ 已停止 · … line Stop writes. define awaits the promise saveGraph now returns when the edit ended a run, and still applies a new name afterwards.
  • plugins/workflow/test/save-graph-running-node.test.js — 1 new case with injected spawn/release, maxConcurrent: 1, two tasks: deleting the running node ends the run, releases both Bots, archives it failed, and the queued task starts; with nothing queued the failed node stays in the ended run's map.
  • docs/subsystems/workflow.md — invariant 3 records the rule (fix(workflow): deleting a running node strands its run and holds the lane #1549).
  • changelog.d/1549.fixed.md — the fragment.

What I ran at 657d6c1

TZ=UTC in a main tree with the four files overwritten:

$ TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"   # plugins/workflow — tests 178, pass 178, fail 0
$ node .github/scripts/changelog.mjs check                           # 81 fragments well formed

Not run: the red-first measurement (the review node does it), the full npm run verify (no core/ change and no generated file: plugins/workflow/client.js and patches/ds-bot/ are untouched), the smoke bench and Desktop.

Checkpoint

{
 "pr": 1593,
 "claim": "Dev",
 "issue": 1549,
 "summary": "Deleting a running node from the workflow canvas used to clear only its timer, so the run stayed running with nothing able to advance it and held its lane slot forever under maxConcurrent. A graph edit that drops a running node now 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. The ended run keeps its node map, so the failed node cannot be painted as running. Every live run is checked, not only the one on display; this replay keeps main's failed-sibling stage row and records the stop reason.",
 "files": [
  "plugins/workflow/src/service.js",
  "plugins/workflow/test/save-graph-running-node.test.js",
  "docs/subsystems/workflow.md",
  "changelog.d/1549.fixed.md"
 ],
 "checks": {
  "ran": "TZ=UTC node --import ./test/resolve.mjs --test \"test/*.test.js\" (plugins/workflow, 178 pass); node .github/scripts/changelog.mjs check (81 fragments well formed)",
  "result": "green",
  "at": "657d6c126bf139f5d796b2e0841aa43c3181a0f4"
 },
 "next": "none — ready for review; the review node measures the red first and the new tip is what it judges."
}

…lay of #1551)

Replay of #1551 onto main fa932f8: the original head lives on a fork we cannot push to.
@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 657d6c1
Written-by: Reviewer
Attestation: 657d6c1 role=Reviewer

Ran at this tip:

$ cd plugins/workflow && TZ=UTC FLYBOT_CORE=/data/dsh/bot/workspace/w1508/core node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 178  # pass 178  # fail 0
$ cd plugins/workflow && cp /data/dsh/bot/workspace/rv1593/service.base.js src/service.js && TZ=UTC FLYBOT_CORE=/data/dsh/bot/workspace/w1508/core node --import ./test/resolve.mjs --test "test/*.test.js"   # service.js back at the fa932f86 base blob, this PR's tests kept
# tests 178  # pass 177  # fail 1  (not ok 163 - deleting a running node ends its run, releases the Bot and drains the queue)
$ node .github/scripts/changelog.mjs check
# changelog: 81 fragment(s) in changelog.d/ are well formed
$ node r1593tier.mjs   # risk-gate createGate().judge() over GET /pulls/1593/files
# risk-tier: T2; basis: plugins/workflow/src/service.js=T2, plugins/workflow/test/save-graph-running-node.test.js=T2, changelog.d/1549.fixed.md=T1, docs/subsystems/workflow.md=T0

Review

  • Red first, at the base. With plugins/workflow/src/service.js at the fa932f86 base blob and the PR's test file kept, the suite is 177/178: only not ok 163 - deleting a running node ends its run, releases the Bot and drains the queue fails. Restoring the tip blob returns 178/178, twice. So the new test fails without the change and the change is what makes it pass.
  • The regression is the one in fix(workflow): deleting a running node strands its run and holds the lane #1549. saveGraph used to clear only the timer of a dropped running node; the run stayed running with a queued-only map and no timer, so under maxConcurrent: 1 nothing behind it could start. The change ends every live run (lane.active plus a live lane.last) whose running node the edit drops, through the same endRun Stop/Reset use, then releases its ephemeral Bots and drainAll() lets the queued task take the lane — the reproduction the issue describes.
  • Replay is faithful, nothing on main reverted. Only the 4 files the body names change (plugins/workflow/src/service.js +62/−17, the new test, docs/subsystems/workflow.md, changelog.d/1549.fixed.md); no core/, no patches/ds-bot/, no generated file. The adjustment to main's fix(workflow): a sibling Failed run is only in Runs — header/timeline never say it failed #1493 endRun third argument records { code: 'stopped' } — a graph edit is not the host ending the run — and main's failed-sibling stage row is preserved and asserted.
  • Design conformance. The fix matches the invariant-3 sentence added to docs/subsystems/workflow.md, the ended run keeps its own node map so the failed node cannot paint as running, define awaits the promise saveGraph now returns before applying a new name, and the chat line is Stop's ■ 已停止 · … shape.
  • CI at the tip. Every check on 657d6c12 is terminal: all green except enqueue-guard, which fails for want of a Verdict naming this tip — the expected pre-PASS red; no other failure. mergeable=true.
  • Hand-off metadata. Fixes #1549, Owner: Dev, Reviewer: Reviewer (distinct Bot), label agent-pr; the checkpoint block parses (pr 1593, claim Dev, issue 1549, 4 files, 579-character summary, checks.result green at this tip, 98-character next, 1261 characters total).

Not run: the full npm run verify (no core/ change and no generated file in the diff), the smoke bench, Desktop, and the plugins (workflow) CI job was read, not re-run.

Note: the local suite runs against core/ from an already-installed harness checkout (FLYBOT_CORE); core/ is untouched by this PR, so the plugin-under-test and the core it runs on are both current main.

@github-actions github-actions Bot added the pass label Oct 28, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 7ba3362 Oct 28, 2026
17 checks passed
@Birfy
Birfy deleted the replay/pr-1551 branch October 28, 2026 14:47
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(workflow): deleting a running node strands its run and holds the lane

1 participant