Fix workflow step failure detection on quoted notice text

changelog.d/2677-quoted-failure.fixed.md

@@ -0,0 +1 @@
+A workflow step no longer fails because its own answer quotes `could not answer your message` while auditing that sentence: the host marks its real failure notice structurally, and an old host without the marker is read only from the notice shape at the start of the reply ([#2677](https://github.com/ThinkFlowLab/FlyBot/issues/2677)).

core/src/host/turns.js

@@ -711,6 +711,8 @@ export function install(rt) {
               hop: (source.hop ?? 0) + 1,
               replyTo: source.exchangeId,
               origin,
+              // #2677: the same marker the remote notice uses, so a reader never parses the sentence.
+              failure: true,
             }).catch(error => rt.warn('ds-bot: failure notice delivery failed: %s', error))
           } else if (reply !== '' || reason?.kind === 'aborted') {
             // A turn stopped before it could answer still owes the sender a line,

core/test/failure-notice-marker.test.js

@@ -0,0 +1,56 @@
+// #2677: a workflow step used to be failed whenever its own answer quoted the failure
+// sentence. The host's notice now says structurally that the turn failed, so a reader does
+// not have to parse the text it may be auditing.
+import assert from 'node:assert/strict'
+import { test } from 'node:test'
+import { install as installTurns } from '../src/host/turns.js'
+import { SOURCE_KIND } from '../src/host/constants.js'
+
+function turnsHost() {
+  let listener
+  const delivered = []
+  const rt = {
+    ctx: { on: (name, fn) => { if (name === 'session/event') listener = fn } },
+    state: { exchangeTurns: {}, questions: {} },
+    bots: { a: { id: 'a', name: 'Chief' }, b: { id: 'b', name: 'Writer' } },
+    isBotSession: () => true,
+    roomOf: () => undefined,
+    groupOwners: new Map(),
+    chatOwners: new Map(),
+    botOf: id => rt.bots[id],
+    selfOf: id => id,
+    save: async () => {},
+    deliver: async (args) => { delivered.push(args) },
+    noteFailure() {},
+    warn() {},
+    switchJobs: () => [],
+  }
+  installTurns(rt)
+  let seq = 0
+  const emit = (id, type, data) => listener({ id }, { type, seq: ++seq, data })
+  return { emit, delivered }
+}
+
+const replySource = (extra = {}) => ({ kind: SOURCE_KIND, role: 'request', senderSessionId: 'a', senderName: 'Chief', exchangeId: 'e1', hop: 0, ...extra })
+const textBlock = content => [{ type: 'text', text: content }]
+const assistantText = text => ({ message: { content: textBlock(text) } })
+const joined = host => {
+  host.emit('b', 'turn/start', { turn: 1 })
+  host.emit('b', 'user/message', { content: textBlock('write the letter'), source: { kind: 'user' } })
+  host.emit('b', 'assistant/message', assistantText('first half'))
+  host.emit('b', 'user/message', { content: textBlock('[Message from Chief]\nmore'), source: replySource() })
+}
+
+test('#2677: the failure notice carries the structural marker; a normal reply does not', () => {
+  const failed = turnsHost()
+  joined(failed)
+  failed.emit('b', 'turn/end', { turn: 1, reason: { kind: 'error', error: { code: 'SERVER', message: 'upstream busy', status: 503 } } })
+  assert.equal(failed.delivered.length, 1)
+  assert.equal(failed.delivered[0].failure, true)
+
+  const answered = turnsHost()
+  joined(answered)
+  answered.emit('b', 'turn/end', { turn: 1, reason: { kind: 'completed' } })
+  assert.equal(answered.delivered.length, 1)
+  assert.equal(answered.delivered[0].failure, undefined)
+})

docs/subsystems/core-host.md

@@ -440,7 +440,7 @@ Work a member hands a Bot with `message_bot` from its group Session runs in a hi
 ### Untrusted fences in every shared room
 
 - `authoredRoomLine` fences peer-controlled text by the line's origin, not the room kind (#98): a remote participant, a local remote-chat record (`state.bots[id].remote`), a botId that is no team Bot, or `line.remote`. A friend's Bot speaking in a non-face group is fenced the same way as in a face room. A complete wrap is left as-is so outbound fanout does not nest residue; a mention of the marker in the body still fences. `stripUntrusted` is one fixed-point pass over both `untrusted-message` and `untrusted-file`, and its pattern allows whitespace inside the tag (`< / untrusted-message>`), since a parser that ignores whitespace reads the same marker; `stripMarkers` does the same for one tag. `core/test/fence-property.test.js` fuzzes a corpus of reconstitution attempts to keep this a fixed point. Inbound-share lines from the other instance are `remote: true` so owner-typed text is fenced too. Face join/leave notices stay unfenced platform copy. Channel room previews use `unwrapUntrusted` so the sidebar never shows the wrap. A human line whose `who` is `User` is rewritten from the signed-in display name before it is stored or fenced.
-- A late group model failure posts a short friendly line into the room (`This Bot could not answer right now.` / `This Bot has no model set up yet.`); `failureLine` stays on Bot-to-Bot deliver and in diagnostics, not in the human transcript.
+- A late group model failure posts a short friendly line into the room (`This Bot could not answer right now.` / `This Bot has no model set up yet.`); `failureLine` stays on Bot-to-Bot deliver and in diagnostics, not in the human transcript. That Bot-to-Bot failure notice carries `failure: true` (#2677), the same field the remote notice uses, so a consumer reads the outcome from the message instead of parsing the sentence.
 
 ### Remote chats and mentions
 

patches/ds-bot/0001-core-fixes.patch

@@ -8505,11 +8505,13 @@ diff --git a/src/host/turns.js b/src/host/turns.js
            if (!expectsReply(source) || answersUser(source)) return
            const name = rt.botOf(rt.selfOf(id))?.name ?? 'The other Bot'
            // A sender that wrote from a group gets the reply in its Session there, while
-@@ -434,6 +434,7 @@ export function install(rt) {
+@@ -434,6 +434,9 @@ export function install(rt) {
                role: 'reply',
                hop: (source.hop ?? 0) + 1,
                replyTo: source.exchangeId,
 +              origin,
++              // #2677: the same marker the remote notice uses, so a reader never parses the sentence.
++              failure: true,
              }).catch(error => rt.warn('ds-bot: failure notice delivery failed: %s', error))
            } else if (reply !== '' || reason?.kind === 'aborted') {
              // A turn stopped before it could answer still owes the sender a line,

patches/ds-bot/files/test/failure-notice-marker.test.js

@@ -0,0 +1,56 @@
+// #2677: a workflow step used to be failed whenever its own answer quoted the failure
+// sentence. The host's notice now says structurally that the turn failed, so a reader does
+// not have to parse the text it may be auditing.
+import assert from 'node:assert/strict'
+import { test } from 'node:test'
+import { install as installTurns } from '../src/host/turns.js'
+import { SOURCE_KIND } from '../src/host/constants.js'
+
+function turnsHost() {
+  let listener
+  const delivered = []
+  const rt = {
+    ctx: { on: (name, fn) => { if (name === 'session/event') listener = fn } },
+    state: { exchangeTurns: {}, questions: {} },
+    bots: { a: { id: 'a', name: 'Chief' }, b: { id: 'b', name: 'Writer' } },
+    isBotSession: () => true,
+    roomOf: () => undefined,
+    groupOwners: new Map(),
+    chatOwners: new Map(),
+    botOf: id => rt.bots[id],
+    selfOf: id => id,
+    save: async () => {},
+    deliver: async (args) => { delivered.push(args) },
+    noteFailure() {},
+    warn() {},
+    switchJobs: () => [],
+  }
+  installTurns(rt)
+  let seq = 0
+  const emit = (id, type, data) => listener({ id }, { type, seq: ++seq, data })
+  return { emit, delivered }
+}
+
+const replySource = (extra = {}) => ({ kind: SOURCE_KIND, role: 'request', senderSessionId: 'a', senderName: 'Chief', exchangeId: 'e1', hop: 0, ...extra })
+const textBlock = content => [{ type: 'text', text: content }]
+const assistantText = text => ({ message: { content: textBlock(text) } })
+const joined = host => {
+  host.emit('b', 'turn/start', { turn: 1 })
+  host.emit('b', 'user/message', { content: textBlock('write the letter'), source: { kind: 'user' } })
+  host.emit('b', 'assistant/message', assistantText('first half'))
+  host.emit('b', 'user/message', { content: textBlock('[Message from Chief]\nmore'), source: replySource() })
+}
+
+test('#2677: the failure notice carries the structural marker; a normal reply does not', () => {
+  const failed = turnsHost()
+  joined(failed)
+  failed.emit('b', 'turn/end', { turn: 1, reason: { kind: 'error', error: { code: 'SERVER', message: 'upstream busy', status: 503 } } })
+  assert.equal(failed.delivered.length, 1)
+  assert.equal(failed.delivered[0].failure, true)
+
+  const answered = turnsHost()
+  joined(answered)
+  answered.emit('b', 'turn/end', { turn: 1, reason: { kind: 'completed' } })
+  assert.equal(answered.delivered.length, 1)
+  assert.equal(answered.delivered[0].failure, undefined)
+})

plugins/workflow/index.js

@@ -15,6 +15,25 @@
   return `${stem} ${randomBytes(3).toString('hex')}`.slice(0, 40)
 }
 
+// #2677: a step's answer is its own text, so quoting the host's failure sentence in an
+// audit or a pasted log must not fail the node. The host marks a real notice on the
+// message; an old host without that marker is read only in the notice shape at the start
+// of the text (`<name> could not answer your message: …`).
+const LEGACY_FAILURE_NOTICE = [
+  /^[^\n:`"'<>]{1,80} could not answer your message:/i,
+  /^could not answer your message$/i,
+  /^[^\n:`"'<>]{0,80}model call failed\.?$/i,
+]
+const legacyFailureNotice = text => LEGACY_FAILURE_NOTICE.some(pattern => pattern.test(text))
+// `rt.deliver` hands the middleware its own args under `deliverArgs`, where the notice
+// carries the marker; a direct `bot.send` message carries it at the top level.
+function failureMarker(msg) {
+  const marked = msg.failure ?? msg.deliverArgs?.failure
+  if (marked !== undefined) return marked === true
+  if (msg.ok !== undefined) return msg.ok === false
+  return undefined
+}
+
 export const name = 'flybot-workflow'
 export const inject = ['flybot']
 
@@ -142,7 +161,7 @@
         task: { type: 'string', description: 'Task id, such as #1280. Omitted ids are generated.' },
       },
       // fb.tools.register hands the calling Bot to execute() as the second argument's botId (#1544).
       execute: (args, ctx) => service.runNamed({ ...(args ?? {}), ...(ctx?.botId ? { callerBotId: ctx.botId } : {}) }),
     }))
     offs.push(fb.tools.register({
       name: 'define_workflow',
@@ -154,7 +173,7 @@
         edges: { type: 'array', required: true, description: 'Hand-offs { from, to }. A fork is two edges out; a join is two edges in. A condition is when: { output, eq } or { else: true }. A loop is loop: true with max (1–8, default 3) and optional until: { output, eq }.' },
         stepTimeoutMinutes: { type: 'number', description: 'How long one step may wait for its Bot before the run fails, 1–120. Omit for the 45-minute default.' },
       },
       execute: (args, ctx) => service.define({ ...(args ?? {}), ...(ctx?.botId ? { callerBotId: ctx.botId } : {}) }),
     }))
   }
   if (typeof fb.bot?.send?.use === 'function') {
@@ -168,7 +187,8 @@
         const owned = service.ownsBot(from)
         if (owned !== true) return undefined
         const text = String(msg.text ?? '')
-        const failed = /could not answer your message|model call failed/i.test(text)
+        const marked = failureMarker(msg)
+        const failed = marked === undefined ? legacyFailureNotice(text) : marked === true
         // The reply advances the run only. It must not land in a persistent Bot chat (e.g. Chief).
         try {
           await Promise.resolve(service.stepResult({ botId: from, text, ok: !failed }))

plugins/workflow/test/step-body.test.js

@@ -116,3 +116,66 @@ test('#2669: the run walks the branch once the node prints the OUTPUT it was tol
   assert.equal(run.nodeStatus.escalate, 'skipped', 'the else route is not taken')
   assert.equal(run.status, 'running')
 })
+
+// #2677: a step's answer is its own text, so an audit that quotes the host's failure
+// sentence, a pasted log, or a retold failure must not fail the node. The host marks the
+// real notice on the message; only an old host falls back to the sentence, and only when
+// it is the shape at the start of the text.
+function replyMiddleware() {
+  let handle
+  const seen = []
+  const fb = {
+    bot: {
+      list: () => [],
+      send: Object.assign(async () => ({ action: 'allow' }), {
+        use(spec) {
+          handle = spec.handle
+          return () => {}
+        },
+        priority: { ROUTING: 10 },
+      }),
+    },
+    readOnly: () => false,
+    api: { handle: () => () => {} },
+    tools: { register: () => () => {} },
+    warn: () => {},
+  }
+  const started = apply({ flybot: { scope: () => fb }, effect(fn) { fn() } }, { persist: false })
+  started.service.ownsBot = () => true
+  started.service.stepResult = async body => { seen.push(body); return {} }
+  return { handle, seen }
+}
+
+async function stepOk(message) {
+  const { handle, seen } = replyMiddleware()
+  const verdict = await handle({ from: 'bot-step', role: 'reply', text: '', ...message })
+  assert.equal(verdict?.action, 'deny')
+  assert.equal(seen.length, 1)
+  return seen[0].ok
+}
+
+const QUOTED_FAILURE = '候选 3：turns.js:709 的正文是 `<name> could not answer your message: its model call failed (…)`'
+
+test('#2677: an answer that quotes the failure sentence still advances the step', async () => {
+  assert.equal(await stepOk({ text: QUOTED_FAILURE }), true, 'a quoted failure inside the answer is not the step failing')
+  assert.equal(await stepOk({ text: '查完了，3 条候选。' }), true, 'an ordinary answer advances the step')
+})
+
+test('#2677: the host marks a real failure notice, and the marker decides', async () => {
+  assert.equal(await stepOk({ text: 'Scout abc could not answer your message: its model call failed (RATE_LIMIT 429).', failure: true }), false)
+  assert.equal(await stepOk({ text: 'anything at all', ok: false }), false)
+  // `rt.deliver` hands the middleware its own args under `deliverArgs`, which is where
+  // the host's notice carries the marker on a live run.
+  assert.equal(await stepOk({ text: 'Scout abc could not answer your message: its model call failed (RATE_LIMIT 429).', deliverArgs: { failure: true } }), false)
+  // A marker that says the answer is fine wins over text that looks like the notice.
+  assert.equal(await stepOk({ text: 'Scout abc could not answer your message: its model call failed (RATE_LIMIT 429).', failure: false }), true)
+})
+
+test('#2677: an old host without the marker is read only at the start of the text', async () => {
+  assert.equal(await stepOk({ text: 'Scout abc could not answer your message:' }), false, 'the notice shape at the start still fails')
+  assert.equal(await stepOk({ text: 'Scout abc could not answer your message: its model call failed (RATE_LIMIT 429).' }), false)
+  // These two are what the middleware caught before #2677; the anchored fallback keeps them.
+  assert.equal(await stepOk({ text: 'could not answer your message' }), false)
+  assert.equal(await stepOk({ text: 'The model call failed.' }), false)
+  assert.equal(await stepOk({ text: 'The log said "`<name> could not answer your message: its model call failed`".' }), true, 'a quoted notice after the start is not the step failing')
+})