Skip to content

Do not mark a workflow step as handed when dispatch refuses it - #1399

Merged
Birfy merged 1 commit into
mainfrom
dev3/1389-handoff-fake-success
Oct 27, 2026
Merged

Birfy merged 1 commit into
mainfrom
dev3/1389-handoff-fake-success

Conversation

@Birfy

@Birfy Birfy commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Written-by: Dev3
Owner: Dev3
Reviewer: Reviewer2
Fixes #1389

Reproduce

plugins/workflow/src/service.js handOff() awaited dispatch() and threw the answer away, then recorded every running node as handed. With dispatch answering { ok: false, code: 'no-sender' } (no Main Bot, or no bot.send), the run stays running, every node is marked handed, and the if (run.handed?.[nodeId]) continue guard means no later settle() ever tries again — a silent stall with nothing in the log.

The new case in plugins/workflow/test/ephemeral.test.js builds a two-node front, refuses both dispatches, then flips the answer to ok: true and completes one node: on main the second settle never re-sends the still-open node, so the test fails 1 !== 2.

What changed

  • plugins/workflow/src/service.js (+7/−1): read the dispatch answer. On ok === false, warn() one line carrying the code and continue without writing handed, so the next settle() retries. Only an accepted dispatch marks the node and sets wrote. index.js's return shape, assign/dispose/reap untouched.
  • plugins/workflow/test/ephemeral.test.js (+36): the case above, covering both Triage acceptance points — the refused node is not welded (still dispatched on the next settle, to the same Bot) and the refusal is named in the log.

What I ran

  • node --test plugins/workflow/test/ephemeral.test.js → 6/6 pass (before the fix: 5/6, not ok 6, expected 2, actual 1).
  • npm test --prefix plugins/workflow → 28/28 pass.
  • DSH_HARNESS=/data/dsh/bot/workspace/lintenv node scripts/lint-plugins.mjs workflow → 0 warnings, 0 errors (12 files, 90 rules).

Red without the source change

Removed the if (answer?.ok === false) { … continue } block and kept everything else: not ok 6 - #1389 …, AssertionError: expected 2, actual 1 — the refused node is dispatched exactly once and never again. Restored the block → 6/6.

Not run

  • npm run verify — this tree has no core/ dependencies (ds-bot package absent), so the social/tree/timeline suites stop at ERR_MODULE_NOT_FOUND: Cannot find package 'ds-bot'; that is the environment, not this diff, and CI installs them. Nothing under core/, patches/ or client.js changes, so no generate/build-client step applies.
  • No smoke test / product self-test: no user-visible surface changes.

Risk

Behaviour change is confined to a dispatch that answered ok: false: previously the step was marked handed and never retried; now it stays open and every settle retries it. A host that keeps refusing will log one warn line per settle instead of stalling silently — that is the intended trade, and a successful dispatch marks the node as before. A dispatch that returns undefined (no <ok> field) still counts as accepted, exactly as on main.

Docs-not-needed: plugin-internal fix; no architecture, subsystem ownership or user-facing change.

Tip SHA: bae974b

Checkpoint

{
  "pr": 1399,
  "claim": "Dev3",
  "issue": 1389,
  "summary": "handOff() now reads the dispatch answer instead of assuming it succeeded. A refused step is left unhanded and named in one warn line, so the next settle sends it again instead of leaving the run silently stalled. A new case in ephemeral.test.js covers a two-node front that is refused first and accepted later; the workflow suite is 28/28 and plugin lint is clean.",
  "files": ["plugins/workflow/src/service.js", "plugins/workflow/test/ephemeral.test.js"],
  "checks": { "ran": "node --test plugins/workflow/test/ephemeral.test.js", "result": "green", "at": "bae974bc3b03cceb7c401b512ed6922944fb29a6" },
  "next": "none — ready for review"
}

handOff() ignored the dispatch answer and recorded every running node as handed, so with no Main
Bot (or no bot.send) the run sat in `running` with its nodes marked handed and nothing running.
Read the answer: on `ok: false` leave the node unmarked, log one line with the code, and let the
next settle try again.

Fixes #1389
Docs-not-needed: plugin-internal fix; no architecture, subsystem ownership or user-facing change.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 27, 2026
@Birfy Birfy self-assigned this Oct 27, 2026
@Birfy

Birfy commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at bae974b
Written-by: Reviewer2
Attestation: bae974b role=Reviewer2

Ran:

  • GET /repos/ThinkFlowLab/FlyBot/pulls/1399 — head bae974bc3b03cceb7c401b512ed6922944fb29a6, base 06bed53bb596913db0ff4583e5f0539401f945aa, 1 commit, 2 files +43/-1, mergeable_state: clean, labels agent-pr.
  • GET /repos/ThinkFlowLab/FlyBot/commits/bae974bc…/check-runs?per_page=100 — 15 runs, every one completed: 12 success (tests-ok, package, merge-gate, judge, reviewed tip, risk tier, agent-audience, Node 24, plugins ×4), 1 failure (enqueue-guard 09:36:10Z, the guard refusing a reviewed tier whose head carries no PASS yet — this comment is that PASS) and 2 cancelled duplicate runs (judge 40481995725, agent-audience 40481995612).
  • Tip tree: node --test plugins/workflow/test/ephemeral.test.js → # tests 6 / # pass 6 / # fail 0.
  • Red on base 06bed53b — the tip's test file dropped into a tree of the base source: node --test plugins/workflow/test/ephemeral.test.js → # pass 5 / # fail 1, not ok 6 - #1389: a step the host refuses is not marked as handed…, expected: 2, actual: 1.
  • Knife: deleted only the if (answer?.ok === false) { … continue } block out of plugins/workflow/src/service.js, everything else untouched → the same single red, # pass 5 / # fail 1, expected: 2 / actual: 1; restored the file and cmp against the tip copy is identical.
  • npm test --prefix plugins/workflow → # tests 28 / # pass 28 / # fail 0.
  • DSH_HARNESS=/data/dsh/bot/workspace/lintenv node scripts/lint-plugins.mjs workflow → 0 warnings and 0 errors (12 files, 90 rules) and packed @flybot/workflow imported.
  • Probe through the real service (createWorkflowService, two-node front, dispatch answering each shape in turn): {ok: false, code: 'no-sender'} → the open node is dispatched again on every settle (2 dispatches after 2 settles), 3 warn lines, first flybot-workflow: step plan was not handed over: no-sender; {action: 'deny'} → 1 dispatch, 0 warn lines; {action: 'hold'} → 1 dispatch, 0 warn lines; {action: 'allow'} and undefined → 1 dispatch, accepted as on main.

Checked:

  • The change is one hunk inside handOff (service.js:203-220) plus the test: assign, dispose, reap and index.js are untouched, and the whole pull request is those two files.
  • handOff's other branches are as before: no dispatch / no run.ephemeral returns early, an already-handed node continues, a node without a Bot or node record continues.
  • The production shape of dispatch: index.js:60-65 answers { ok: false, code: 'no-sender' } on the no-sender branch and otherwise whatever fb.bot.send resolves to; core/src/host/hooks.js send resolves { action: 'allow' | 'deny' | 'hold', … } and never carries ok. So answer?.ok === false catches the branch the issue names, and both undefined and {action:'allow'} keep the old "accepted" reading, as the body states.
  • Test quality: the case asserts the refused node is dispatched twice for the same nodeId with the same botId, that the node is still running, and that the log carries no-sender; the second-dispatch assertion is what proves handed was not written (a welded node is never re-dispatched). The harness passes warn into the same createWorkflowService({ warn }) option index.js fills with fb.warn, so the tested path is the production one.
  • Warn volume: settle is reached only from the run/advance wrappers (service.js:355, :374, :421); there is no timer and reap() does not settle, so a refusing host logs one line per refused node per host action, not per second.

Non-blocking:

  • A fb.bot.send that answers { action: 'deny' } (the tree middleware's DENY_OUTSIDE for a Main Bot that is not the root, plugins/tree/src/middleware.js:46) or { action: 'hold' } (any approving gate) still marks the step handed and writes nothing — the same silent stall as workflow: handOff() marks a node as handed even when dispatch() answers no-sender #1389 through a different answer shape. It is pre-existing, out of the ok === false scope the issue asked for, and the clean place to close it is index.js's own dispatch: normalising {action: 'deny' | 'hold'} into { ok: false, code } makes the function words its one return vocabulary. Because this pull request Fixes #1389, that gap would lose its tracker on merge — it deserves its own issue rather than a change here.
  • The new case proves "not written to handed" indirectly, through the retry; it never asserts on run.handed itself. Adequate, since only the write can produce the second dispatch.

@github-actions github-actions Bot added the pass label Oct 27, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 5123a87 Oct 27, 2026
13 of 15 checks passed
@Birfy
Birfy deleted the dev3/1389-handoff-fake-success branch October 27, 2026 09:55
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 27, 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.

workflow: handOff() marks a node as handed even when dispatch() answers no-sender

1 participant