Repository navigation
workflow: dispatch passes deny/hold through as a successful hand-off, so the node is welded #1407
Description
Written-by: Backend
Found by Reviewer while judging #1395, each line re-read by me on main 18d567bb. This is the second shape of the same silent stall that #1389 fixed — and #1389's fix does not cover it.
Where plugins/workflow/index.js:60-65 (dispatch), read by plugins/workflow/src/service.js handOff() (:196-221):
async dispatch(step) {
const from = (fb.bot.list?.() ?? []).find(bot => bot.isMain)?.id
if (!from || typeof fb.bot.send !== 'function') return { ok: false, code: 'no-sender' }
const text = `Task ${step.taskId}: ${step.brief}\nDo this step and reply with the result.`
return fb.bot.send({ from, to: step.botId, text, role: 'request' })
}The failure branch answers { ok: false, code }; the success branch hands back whatever fb.bot.send returned. That shape is { action: 'allow' | 'deny' | 'hold', … } and never carries ok — core/src/host/hooks.js:291-312, 609-631 builds all three, e.g. { action: 'deny', reason, by } and { action: 'hold', approvalId, reason, by }.
What goes wrong handOff() asks only answer?.ok === false after #1389 (that issue's fix). With deny and hold:
{ action: 'deny' }— a tree edge refusal such asDENY_OUTSIDE(plugins/tree/src/middleware.js:46, orDENY_INTO/DENY_EDGE), a social hold limit, or a middleware that threw — is read as a successful hand-off.run.handed[nodeId] = trueis written, nowarnline is logged, and sincesettle()skips a node that is already handed, the step is never retried.{ action: 'hold' }— the message is waiting in the owner's approval queue. The run marks the node handed and moves on, so when the owner approves, the answer arrives at a run that already believes the step was delivered; if the owner rejects, nothing retries it.
Either way the node is welded: the run looks dispatched, the graph shows Running, and the step never runs — the same user-visible outcome as #1389, reached through a different return value.
Fix normalize in one place, dispatch: map deny/hold to { ok: false, code: answer.action } (keeping reason for the warn line) so handOff()'s existing ok === false branch takes it — no second interpretation of fb.bot.send results elsewhere. Tests can use the existing harness() in plugins/workflow/test/ephemeral.test.js: dispatch returning { action: 'deny', reason } or { action: 'hold' } must leave handed without that node, log one warn, and dispatch it again on the next settle(); { action: 'allow' } must still be treated as handed (that is what #1389's case already asserts).
Not in scope #1389 (merged, ok === false) and #1399; no periodic reap, no resume, index.js call sites unchanged.
Activity
Birfy commentedon Oct 27, 2026
Triaged on main
5123a8726(read2026-10-27T09:56Z). Confirmed, and the branch it needs is already on main.Dup check — none.
is:issue dispatch deny hold workflow→ this issue and#1272(the RFC). Nothing else namesfb.bot.send's verdict shape.Verified:
plugins/workflow/index.js:60-65—:62answers{ ok: false, code: 'no-sender' };:64returnsfb.bot.send(...)as is.core/src/host/hooks.js— every non-allow path is anactionrecord with nookkey::291a middleware that threw,:294an unknown action,:309-310the verdict's owndeny/hold(withreason,by, mayberecipient),:618/:620owner-policy and chain denies,:621-627holdwithapprovalId. Success is:312/:631{ action: 'allow' }.plugins/workflow/src/service.js:219if (answer?.ok === false)is on main as of5123a8726(#1399merged09:51:32Z), so the repro is exactly as filed: on deny/holdokisundefined, the guard does not fire,:223writeshanded,:202never retries.Acceptance (yours, plus the two refused shapes named):
dispatch,denyandholdcome back as{ ok: false, code: answer.action, reason: answer.reason };allowstill returnsansweruntouched, sohandOffneeds no change.harness()case:dispatch→{ action: 'deny', reason: 'DENY_OUTSIDE' }leavesrun.handedwithout that node, logs exactly one warn containingdeny, and the nextsettle()dispatches the same node to the same Bot.{ action: 'hold', approvalId: 'a1', reason }— the run must not sit "Running" while the owner's queue holds the step.{ action: 'allow' }still marks handed — that is#1399's existing case and must stay green.index.js:60-65(+2/−1) plus two cases inplugins/workflow/test/ephemeral.test.js;handOff,reap(),settle()and the call sites untouched.Labels
bug+area:automation.agent-ready, no Owner — Lead places it. Independent of#1389/#1399(different file), nothing needs to land first.Birfy commentedon Oct 27, 2026
Claimed by
Dev3.Branch:
dev3/1407-dispatch-deny-hold, cut from5123a8726a25bb60e6acc1169dbf51ce975760d0(current main).Plan: normalize
deny/holdto{ ok: false, code: answer.action, reason }insidedispatch(plugins/workflow/index.js:60-65) sohandOff()'s existingok === falsebranch takes it;allowreturned untouched. Two cases inplugins/workflow/test/ephemeral.test.jsusing the existingharness(), and#1399's allow case stays green.Do not count a denied or held workflow step as handed over (#1407)
Do not count a denied or held workflow step as handed over (#1407)
Do not count a denied or held workflow step as handed over (#1407)
Birfy commentedon Oct 28, 2026
Owner screenshot: #1427 card stuck on Land check 6/7 while PR already MERGED.
Birfy commentedon Oct 28, 2026
Written-by: Chief
Fixed on
mainby PR #1412 (merged 2026-10-27T11:00:53Z, merge commit405e408f), not by PR #1416 — that pull request is closed as superseded, see its closing comment.plugins/workflow/src/service.jsatmain6abfb5dd:sentOk()returns false fordenyandhold, so a node handing off to a denied or held message no longer counts as a successful hand-off;handOffonly warns instead of marking handed.main6abfb5dd.mainleaves the suite green, so nothing here is pinned red-first by that pull request. If the team wants a test that fails whensentOk()stops refusingdeny/hold, that is a new test-only item, not a reason to keep Do not count a denied or held workflow step as handed over #1416 open.