Do not count a denied or held workflow step as handed over

changelog.d/1407-workflow-dispatch-deny.fixed.md

@@ -0,0 +1 @@
+A workflow step the host refuses is no longer counted as delivered. When `fb.bot.send` answers `deny` (an edge outside the tree, a social limit, a middleware that threw) or `hold` (the message waits in the owner's approval queue), the step now stays unmarked and the next settle sends it again to the same Bot, with one line in the log naming which of the two refused it; an `allow` is unchanged. ([#1407](https://github.com/ThinkFlowLab/FlyBot/issues/1407))

plugins/workflow/index.js

@@ -61,7 +61,13 @@ export function apply(ctx, config = {}) {
       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' })
+      const answer = await fb.bot.send({ from, to: step.botId, text, role: 'request' })
+      // The host refuses with { action: 'deny' | 'hold' } and no ok key, which
+      // handOff would read as delivered. Answer in its own vocabulary instead.
+      if (answer?.action === 'deny' || answer?.action === 'hold') {
+        return { ok: false, code: answer.action, reason: answer.reason }
+      }
+      return answer
     },
   } : {}
   const service = createWorkflowService({

plugins/workflow/test/ephemeral.test.js

@@ -388,3 +388,71 @@ test('the plugin load itself reaps a run the restart left running (#1388)', asyn
     rmSync(dir, { recursive: true, force: true })
   }
 })
+
+// #1407: the host answers a refused hand-off with { action: 'deny' | 'hold' },
+// never with { ok: false }. `dispatch` has to normalise that shape, or the run
+// welds the node shut exactly like #1389 did through { ok: false }.
+function hosted(answerFor) {
+  const created = []
+  const sent = []
+  const warnings = []
+  const fb = {
+    bot: {
+      list: () => roster,
+      async create() { created.push({}); return `bot-${created.length}` },
+      async remove() { return true },
+      async send(message) { sent.push(message); return answerFor(message) },
+    },
+    readOnly: () => false,
+    api: { handle() { return () => {} } },
+    tools: { register() { return () => {} } },
+    warn: (...args) => warnings.push(args.join(' ')),
+  }
+  const started = apply({ flybot: { scope: () => fb }, effect(fn) { fn() } }, { persist: false })
+  return { service: started.service, sent, warnings }
+}
+
+function pairNodes() {
+  return [
+    { id: 'plan', name: 'Planner', role: 'plan', prompt: 'Write the plan.' },
+    { id: 'code', name: 'Coder', role: 'code', prompt: 'Implement it.' },
+  ]
+}
+
+for (const refused of [
+  { action: 'deny', reason: 'DENY_OUTSIDE', by: { bot: 'tree' } },
+  { action: 'hold', approvalId: 'approval-1', reason: 'Owner approval is required', by: { bot: 'policy' } },
+]) {
+  test(`#1407: a ${refused.action} answer is not marked as handed and is dispatched again`, async () => {
+    const { service, sent, warnings } = hosted(() => refused)
+    const made = await service.define({ name: 'Pair', nodes: pairNodes(), edges: [] })
+    const workflowId = made.workflows.find(item => item.name === 'Pair').id
+    const started = await service.runNamed({ workflow: 'Pair', brief: 'Ship the note' })
+    assert.equal(started.ok, true)
+    const run = started.workflows.find(item => item.id === workflowId).run
+    assert.equal(run.status, 'running')
+    assert.equal(sent.length, 2)
+    assert.match(warnings.join('\n'), new RegExp(`was not handed over: ${refused.action}`))
+
+    // The refused step is not welded shut: the next settle sends it again, to the same Bot.
+    assert.deepEqual(sent.map(message => message.to).sort(), ['bot-1', 'bot-2'])
+    const codeTo = sent[1].to
+    const moved = await service.advance({ workflowId, nodeId: 'plan', outcome: 'done' })
+    assert.equal(moved.ok, true)
+    const resent = sent.filter(message => message.to === codeTo)
+    assert.equal(resent.length, 2)
+    assert.equal(warnings.length, 3)
+  })
+}
+
+test('#1407: an allow answer is still marked as handed', async () => {
+  const { service, sent, warnings } = hosted(() => ({ action: 'allow' }))
+  const made = await service.define({ name: 'Pair', nodes: pairNodes(), edges: [] })
+  const workflowId = made.workflows.find(item => item.name === 'Pair').id
+  const started = await service.runNamed({ workflow: 'Pair', brief: 'Ship the note' })
+  assert.equal(started.ok, true)
+  assert.deepEqual(warnings, [])
+  const moved = await service.advance({ workflowId, nodeId: 'plan', outcome: 'done' })
+  assert.equal(moved.ok, true)
+  assert.equal(sent.length, 2)
+})