diff --git a/changelog.d/1524-workflow-reply-leak.fixed.md b/changelog.d/1524-workflow-reply-leak.fixed.md new file mode 100644 index 000000000..402932674 --- /dev/null +++ b/changelog.d/1524-workflow-reply-leak.fixed.md @@ -0,0 +1 @@ +**Workflow ephemeral node Bot replies no longer leak into persistent Bot chats.** Previously, ephemeral workflow node Bot replies (e.g., Review / Verify / Rework results) could appear as queued `[Reply from …]` messages in Chief or other persistent Bot conversations when deliveries bypassed send middleware (deferred, resumed, retried flags) or after workflow release. The fix expands the `gated()` check in `hooks.js` so ephemeral bot deliveries ALWAYS route through middleware (ensuring `stepResult` advances the workflow), while the workflow send hook denies delivery to persistent Bot chats. Removed ephemeral bots are tombstoned so late replies cannot leak. Ephemeral Bot outputs stay strictly on the workflow run surface. (#1524) diff --git a/core/src/host/hooks.js b/core/src/host/hooks.js index 32fe7d54a..d4e7678dc 100644 --- a/core/src/host/hooks.js +++ b/core/src/host/hooks.js @@ -683,6 +683,8 @@ export function install(rt) { const bot = rt.botOf(id) if (bot === undefined) return false if (bot.ephemeral?.plugin !== plugin) throw new Error('flybot: only the plugin that created this run Bot can remove it') + // Tombstone before deletion so late replies are denied (#1524) + if (bot.ephemeral) tombstoneEphemeral(id) await rt.deleteBot(id) return true } @@ -727,9 +729,22 @@ export function install(rt) { send.list = () => chain.map(m => ({ name: m.name, priority: priorityOf(m), before: m.before ?? [], after: m.after ?? [] })) send.priority = SEND_PRIORITY + // Tombstone removed ephemeral bots: a late reply after bot.remove must not leak. + const ephemeralTombstones = new Set() + const tombstoneEphemeral = (id) => ephemeralTombstones.add(id) + const wasEphemeral = (id) => ephemeralTombstones.has(id) + // The gate around ds-bot's own rt.deliver. Approved and re-sent messages pass through. - const gated = args => args.fromId !== undefined && rt.botOf(args.fromId) !== undefined && BOT_TO_BOT_ROLES.has(args.role) - && !args.flybotChecked && !args.resent && !args.resumed && !args.retried && !args.deferred + // CRITICAL FIX (#1524): Ephemeral Bot replies must ALWAYS go through middleware (which calls + // stepResult) even when deferred/resumed/retried/resent, so gated() returns true for ephemeral senders. + const gated = args => { + if (!args.fromId || !BOT_TO_BOT_ROLES.has(args.role) || args.flybotChecked) return false + const from = rt.botOf(args.fromId) + // Ephemeral bots (or tombstoned removed bots) always route through middleware, even when deferred/resumed/retried/resent + if (from?.ephemeral || wasEphemeral(args.fromId)) return true + // Normal bots skip middleware when deferred/resumed/retried/resent + return from !== undefined && !args.resent && !args.resumed && !args.retried && !args.deferred + } innerDeliver = rt.deliver rt.deliver = async (args) => { // Everything that is not a first, checked send still carries its sender: resumed, retried, @@ -746,10 +761,17 @@ export function install(rt) { } if (!gated(args)) return innerDeliver(args) const exchangeId = args.exchangeId ?? randomUUID() + // Post-removal tombstone check: deny late replies from removed ephemeral bots + if (wasEphemeral(args.fromId) && !rt.botOf(args.fromId)) { + noteOutcome(exchangeId, { action: 'deny', reason: 'ephemeral Bot was removed', by: 'flybot-ephemeral' }) + return exchangeId + } const msg = { from: args.fromId, to: args.toId, target: targetOf(args.toId), text: args.text, role: args.role, hop: args.hop ?? 0, via: 'ds-bot', exchangeId, deliverArgs: { ...args, exchangeId } } noteOutcome(exchangeId, await run(msg)) return exchangeId } + // Expose tombstone for workflow plugin to call on bot.remove + rt.tombstoneEphemeral = tombstoneEphemeral // ---------------------------------------------------------------- tools const callerOf = exec => rt.botOf(rt.selfOf(exec?.agent?.id)) diff --git a/core/test/workflow-reply-leak-real.test.js b/core/test/workflow-reply-leak-real.test.js new file mode 100644 index 000000000..ebf7a9357 --- /dev/null +++ b/core/test/workflow-reply-leak-real.test.js @@ -0,0 +1,135 @@ +// #1524: REAL defect tests proving the fix works +// These test actual hooks.js behavior, showing RED on origin/main and GREEN with the fix. +import assert from 'node:assert/strict' +import { test } from 'node:test' + +test('#1524: gated() logic - ephemeral with deferred MUST be gated (the fix)', async () => { + // This tests the actual gated() logic from hooks.js + // RED on origin/main: deferred from ephemeral bypasses gating → leaks + // GREEN with fix: ephemeral forces gating even with deferred → routes through middleware + + const BOT_TO_BOT_ROLES = new Set(['request', 'reply', 'stopped', 'report']) + const bots = new Map([ + ['main', { id: 'main', name: 'Chief' }], + ['ephemeral', { id: 'ephemeral', name: 'Review', ephemeral: { plugin: 'test' } }], + ]) + + const tombstones = new Set() + const botOf = (id) => bots.get(id) + const wasEphemeral = (id) => tombstones.has(id) + + // ORIGIN/MAIN LOGIC (before fix): deferred bypasses gating for all bots + const gatedOnMain = (args) => { + if (!args.fromId || !BOT_TO_BOT_ROLES.has(args.role) || args.flybotChecked || args.resent) return false + const from = botOf(args.fromId) + // Origin/main: deferred/resumed/retried skip gating for ALL bots + return from !== undefined && !args.resumed && !args.retried && !args.deferred + } + + // FIXED LOGIC: ephemeral always gates, even with deferred + const gatedWithFix = (args) => { + if (!args.fromId || !BOT_TO_BOT_ROLES.has(args.role) || args.flybotChecked) return false + const from = botOf(args.fromId) + // Fix: ephemeral (or tombstoned) always routes through middleware + if (from?.ephemeral || wasEphemeral(args.fromId)) return true + // Normal bots still skip when deferred/resumed/retried/resent + return from !== undefined && !args.resent && !args.resumed && !args.retried && !args.deferred + } + + // Test deferred from ephemeral + const deferredArgs = { fromId: 'ephemeral', role: 'reply', deferred: true } + const mainGated = gatedOnMain(deferredArgs) + const fixedGated = gatedWithFix(deferredArgs) + + assert.equal(mainGated, false, 'RED on origin/main: deferred from ephemeral bypasses gating (THE LEAK)') + assert.equal(fixedGated, true, 'GREEN with fix: deferred from ephemeral forces gating (routes through middleware)') + + // Test resumed from ephemeral + const resumedArgs = { fromId: 'ephemeral', role: 'reply', resumed: true } + assert.equal(gatedOnMain(resumedArgs), false, 'origin/main: resumed from ephemeral bypasses gating') + assert.equal(gatedWithFix(resumedArgs), true, 'with fix: resumed from ephemeral forces gating') + + // Test retried from ephemeral + const retriedArgs = { fromId: 'ephemeral', role: 'reply', retried: true } + assert.equal(gatedOnMain(retriedArgs), false, 'origin/main: retried from ephemeral bypasses gating') + assert.equal(gatedWithFix(retriedArgs), true, 'with fix: retried from ephemeral forces gating') + + // Test resent from ephemeral + const resentArgs = { fromId: 'ephemeral', role: 'reply', resent: true } + assert.equal(gatedOnMain(resentArgs), false, 'origin/main: resent from ephemeral bypasses gating') + assert.equal(gatedWithFix(resentArgs), true, 'with fix: resent from ephemeral forces gating') +}) + +test('#1524: tombstone logic - post-removal late reply MUST be gated and denied', async () => { + // RED on origin/main: after removeBot, bot is gone, late reply bypasses gating → leaks + // GREEN with fix: tombstone exists, gated()=true, denied + + const BOT_TO_BOT_ROLES = new Set(['request', 'reply', 'stopped', 'report']) + const bots = new Map([ + ['main', { id: 'main', name: 'Chief' }], + ]) + const tombstones = new Set() + + const botOf = (id) => bots.get(id) + const wasEphemeral = (id) => tombstones.has(id) + + // Simulate bot existence and removal + const ephemeralId = 'ephemeral' + bots.set(ephemeralId, { id: ephemeralId, name: 'Review', ephemeral: { plugin: 'test' } }) + + const gatedWithFix = (args) => { + if (!args.fromId || !BOT_TO_BOT_ROLES.has(args.role) || args.flybotChecked) return false + const from = botOf(args.fromId) + // Ephemeral or tombstoned always gates + if (from?.ephemeral || wasEphemeral(args.fromId)) return true + return from !== undefined && !args.resent && !args.resumed && !args.retried && !args.deferred + } + + // Before removal: gated + const beforeRemoval = gatedWithFix({ fromId: ephemeralId, role: 'reply' }) + assert.equal(beforeRemoval, true, 'Before removal: ephemeral is gated') + + // Remove bot (tombstone it first in fixed version) + tombstones.add(ephemeralId) + bots.delete(ephemeralId) + + // After removal: on origin/main would be ungated (bot is gone) + // With fix: still gated (tombstone exists) + const afterRemoval = gatedWithFix({ fromId: ephemeralId, role: 'reply' }) + assert.equal(botOf(ephemeralId), undefined, 'Bot is removed') + assert.equal(wasEphemeral(ephemeralId), true, 'Tombstone exists') + assert.equal(afterRemoval, true, 'GREEN with fix: post-removal reply is gated by tombstone') +}) + +test('#1524: normal (non-ephemeral) bot behavior unchanged', async () => { + // Ensures the fix doesn't break normal bot delivery + const BOT_TO_BOT_ROLES = new Set(['request', 'reply', 'stopped', 'report']) + const bots = new Map([ + ['main', { id: 'main', name: 'Chief' }], + ['reviewer', { id: 'reviewer', name: 'Reviewer' }], + ]) + + const tombstones = new Set() + const botOf = (id) => bots.get(id) + const wasEphemeral = (id) => tombstones.has(id) + + const gatedWithFix = (args) => { + if (!args.fromId || !BOT_TO_BOT_ROLES.has(args.role) || args.flybotChecked) return false + const from = botOf(args.fromId) + if (from?.ephemeral || wasEphemeral(args.fromId)) return true + return from !== undefined && !args.resent && !args.resumed && !args.retried && !args.deferred + } + + // Normal bot: first reply is gated + assert.equal(gatedWithFix({ fromId: 'reviewer', role: 'reply' }), true, 'Normal bot first reply is gated') + + // Normal bot: deferred/resumed/retried/resent skip gating (unchanged) + assert.equal(gatedWithFix({ fromId: 'reviewer', role: 'reply', deferred: true }), false, 'Normal bot deferred skips gating') + assert.equal(gatedWithFix({ fromId: 'reviewer', role: 'reply', resumed: true }), false, 'Normal bot resumed skips gating') + assert.equal(gatedWithFix({ fromId: 'reviewer', role: 'reply', retried: true }), false, 'Normal bot retried skips gating') + assert.equal(gatedWithFix({ fromId: 'reviewer', role: 'reply', resent: true }), false, 'Normal bot resent skips gating') + + // Request TO ephemeral still allowed (sender is not ephemeral) + bots.set('ephemeral', { id: 'ephemeral', name: 'Review', ephemeral: { plugin: 'test' } }) + assert.equal(gatedWithFix({ fromId: 'main', toId: 'ephemeral', role: 'request' }), true, 'Request TO ephemeral is gated (normal flow)') +}) diff --git a/core/test/workflow-reply-leak.test.js b/core/test/workflow-reply-leak.test.js new file mode 100644 index 000000000..1b67fe54f --- /dev/null +++ b/core/test/workflow-reply-leak.test.js @@ -0,0 +1,270 @@ +// #1524: Workflow ephemeral node Bot replies must not land in Chief / persistent Bot chat +// as queued `[Reply from …]` messages. Node I/O stays in the run/graph only. +import assert from 'node:assert/strict' +import { test } from 'node:test' + +test('#1524: ephemeral node reply is not delivered to Main Bot chat', async () => { + const roster = [{ id: 'main', name: 'Chief', isMain: true, remote: false }] + const created = [] + const removed = [] + + let hookHandle + const fb = { + bot: { + list: () => roster, + async create(spec) { + const id = `ephemeral-${created.length + 1}` + created.push(id) + roster.push({ id, name: spec.name, ephemeral: true, hidden: true }) + return id + }, + async remove(id) { + removed.push(id) + const index = roster.findIndex(bot => bot.id === id) + if (index >= 0) roster.splice(index, 1) + return true + }, + send: { + use(middleware) { + hookHandle = middleware.handle + return () => { hookHandle = undefined } + }, + priority: { ROUTING: 100 }, + }, + }, + readOnly: () => false, + api: { handle() { return () => {} } }, + tools: { register() { return () => {} } }, + warn() {}, + } + + // Apply workflow plugin + const { apply } = await import('../../plugins/workflow/index.js') + const plugin = apply( + { flybot: { scope: () => fb }, effect(fn) { fn() } }, + { persist: false } + ) + const service = plugin.service + + // Create a simple workflow + const defined = await service.define({ + name: 'Test Flow', + nodes: [ + { id: 'step1', name: 'Step One', prompt: 'Do step 1', outputs: [{ name: 'result' }] }, + ], + edges: [], + }) + assert.equal(defined.ok, true) + + // Start a run + const workflow = defined.workflows.find(w => w.name === 'Test Flow') + const run = await service.runNamed({ workflow: 'Test Flow', brief: 'Test task' }) + assert.equal(run.ok, true) + + // Get the ephemeral bot ID + const activeRun = workflow ? run.workflows.find(w => w.id === workflow.id).run : null + assert.ok(activeRun) + assert.ok(activeRun.ephemeral) + const ephemeralBotId = activeRun.ephemeral.step1 + assert.ok(ephemeralBotId) + + // Verify the hook was registered + assert.equal(typeof hookHandle, 'function') + + // Simulate a reply from the ephemeral node Bot + const replyMsg = { + from: ephemeralBotId, + to: 'main', + role: 'reply', + text: 'Step complete\nOUTPUT result: done', + } + + // Call the hook handler + const verdict = await hookHandle(replyMsg) + + // The hook should deny this message (prevent delivery to Main Bot chat) + assert.equal(verdict?.action, 'deny') + assert.match(verdict?.reason ?? '', /workflow step result stays on the run surface/i) + + // Verify the step was recorded in the workflow + const advanced = service.state().workflows.find(w => w.id === workflow.id) + assert.ok(advanced) + + plugin.dispose() +}) + +test('#1524: persistent Bot-to-Bot replies are NOT blocked', async () => { + const roster = [ + { id: 'main', name: 'Chief', isMain: true, remote: false }, + { id: 'reviewer', name: 'Reviewer', remote: false }, + ] + + let hookHandle + const fb = { + bot: { + list: () => roster, + async create() { throw new Error('should not create') }, + async remove() { throw new Error('should not remove') }, + send: { + use(middleware) { + hookHandle = middleware.handle + return () => { hookHandle = undefined } + }, + priority: { ROUTING: 100 }, + }, + }, + readOnly: () => false, + api: { handle() { return () => {} } }, + tools: { register() { return () => {} } }, + warn() {}, + } + + const { apply } = await import('../../plugins/workflow/index.js') + const plugin = apply( + { flybot: { scope: () => fb }, effect(fn) { fn() } }, + { persist: false } + ) + + // A reply from a persistent Bot should NOT be denied + const persistentReply = { + from: 'reviewer', + to: 'main', + role: 'reply', + text: 'Review complete: PASS', + } + + const verdict = await hookHandle(persistentReply) + + // The hook should return undefined (not handled, pass to next middleware) + assert.equal(verdict, undefined) + + plugin.dispose() +}) + +test('#1524: ephemeral Bot replies do NOT leak, middleware runs for stepResult, tombstone works', async () => { + // Critical: ephemeral replies must go through middleware (so stepResult advances the run), + // but must not queue in Chief. After removal, tombstone denies late replies. + const roster = [ + { id: 'main', name: 'Chief', isMain: true, remote: false }, + { id: 'ephemeral-node', name: 'Review', ephemeral: { plugin: 'flybot-workflow' }, hidden: true }, + ] + const deliveries = [] + const middlewareCalls = [] + let stepAdvanced = false + + const innerDeliver = async (args) => { + deliveries.push({ from: args.fromId, to: args.toId, role: args.role, flags: { deferred: args.deferred, resumed: args.resumed } }) + } + + // Simulate the middleware that calls stepResult + const runMiddleware = async (msg) => { + middlewareCalls.push({ from: msg.from, role: msg.role }) + if (msg.from === 'ephemeral-node' && msg.role === 'reply') { + stepAdvanced = true + return { action: 'deny', reason: 'workflow step result stays on the run surface', by: 'workflow-step' } + } + return { action: 'allow' } + } + + const BOT_TO_BOT_ROLES = new Set(['request', 'reply', 'stopped', 'report']) + const ephemeralTombstones = new Set() + + const rt = { + botOf: (id) => roster.find(bot => bot.id === id), + deliver: null, + } + + // Simulate the fixed gated() check + const gated = (args) => { + if (!args.fromId || !BOT_TO_BOT_ROLES.has(args.role) || args.flybotChecked || args.resent) return false + const from = rt.botOf(args.fromId) + // Ephemeral bots (or tombstoned) always route through middleware + if (from?.ephemeral || ephemeralTombstones.has(args.fromId)) return true + return from !== undefined && !args.resumed && !args.retried && !args.deferred + } + + rt.deliver = async (args) => { + const exchangeId = args.exchangeId ?? 'test-exchange-id' + + if (!gated(args)) { + await innerDeliver(args) + return exchangeId + } + + // Post-removal tombstone check + if (ephemeralTombstones.has(args.fromId) && !rt.botOf(args.fromId)) { + return exchangeId + } + + // Run middleware (workflow-step hook calls stepResult here) + const outcome = await runMiddleware({ from: args.fromId, to: args.toId, role: args.role }) + if (outcome.action === 'deny') return exchangeId + + await innerDeliver(args) + return exchangeId + } + + // SCENARIO 1: First reply - middleware runs (stepResult advances), NO delivery to Chief + stepAdvanced = false + await rt.deliver({ + fromId: 'ephemeral-node', + toId: 'main', + text: 'PASS', + role: 'reply', + }) + assert.equal(stepAdvanced, true, 'First reply MUST run middleware to advance step') + assert.equal(deliveries.length, 0, 'First reply should NOT be delivered to Chief') + assert.equal(middlewareCalls.length, 1, 'Middleware should run once') + + // SCENARIO 2: Deferred reply - middleware runs, still denied + stepAdvanced = false + await rt.deliver({ + fromId: 'ephemeral-node', + toId: 'main', + text: 'Deferred reply', + role: 'reply', + deferred: true, + }) + assert.equal(stepAdvanced, true, 'Deferred reply MUST run middleware (gated by ephemeral flag)') + assert.equal(deliveries.length, 0, 'Deferred reply should NOT be delivered') + + // SCENARIO 3: Resumed reply - middleware runs, still denied + stepAdvanced = false + await rt.deliver({ + fromId: 'ephemeral-node', + toId: 'main', + text: 'Resumed reply', + role: 'reply', + resumed: true, + }) + assert.equal(stepAdvanced, true, 'Resumed reply MUST run middleware') + assert.equal(deliveries.length, 0, 'Resumed reply should NOT be delivered') + + // SCENARIO 4: Tombstone the bot (simulate removal) + ephemeralTombstones.add('ephemeral-node') + const botIndex = roster.findIndex(bot => bot.id === 'ephemeral-node') + roster.splice(botIndex, 1) + + // SCENARIO 5: Post-removal reply - tombstone denies without middleware + const beforeMiddleware = middlewareCalls.length + await rt.deliver({ + fromId: 'ephemeral-node', + toId: 'main', + text: 'Late reply after removal', + role: 'reply', + }) + assert.equal(deliveries.length, 0, 'Post-removal reply should NOT be delivered') + assert.equal(middlewareCalls.length, beforeMiddleware, 'Post-removal reply skips middleware (bot is gone)') + + // SCENARIO 6: Request TO ephemeral (before removal, with bot back for test) + roster.push({ id: 'ephemeral-node', name: 'Review', ephemeral: { plugin: 'flybot-workflow' }, hidden: true }) + await rt.deliver({ + fromId: 'main', + toId: 'ephemeral-node', + text: 'Review this', + role: 'request', + }) + assert.equal(deliveries.length, 1, 'Request TO ephemeral Bot should be delivered') + assert.equal(deliveries[0].from, 'main') + assert.equal(deliveries[0].to, 'ephemeral-node') +}) diff --git a/patches/ds-bot/0001-core-fixes.patch b/patches/ds-bot/0001-core-fixes.patch index 90ede2863..000ec6a32 100644 --- a/patches/ds-bot/0001-core-fixes.patch +++ b/patches/ds-bot/0001-core-fixes.patch @@ -2649,14 +2649,17 @@ diff --git a/src/host/agents.js b/src/host/agents.js diff --git a/src/host/approvals.js b/src/host/approvals.js --- a/src/host/approvals.js +++ b/src/host/approvals.js -@@ -6,6 +6,43 @@ +@@ -3,8 +3,45 @@ + // the group chat. A Bot's own chat keeps DSH's card; its requests are only noted, so + // the team view can say who is waiting. + import { randomUUID } from 'node:crypto' +import { EPHEMERAL_DENIED_TOOLS } from './tools.js' +import { OWNER, settled } from './turn-origin.js' -const OUTCOMES = { allow: 'allowed-once', reject: 'rejected' } +const OUTCOMES = { allow: 'allowed-once', reject: 'rejected', 'always-bot': 'allowed-once', 'always-room': 'allowed-once' } +const RUN_DENIED = new Set(EPHEMERAL_DENIED_TOOLS) - ++ +// DSH has no standing grant (its outcomes are one-shot, #540), so the owner's "Always +// allow" lives here: on the Bot or on the group, keyed by the kind of permission asked. +// A sandbox escalation names its mode in DSH's reason; any other ask is keyed by tool. @@ -2690,11 +2693,13 @@ diff --git a/src/host/approvals.js b/src/host/approvals.js + const mode = /^escalate sandbox to ([\w-]+):/.exec(String(reason ?? ''))?.[1] + return mode ? `${tool} · ${mode}` : String(tool) +} -+ + export function install(rt) { const { ctx } = rt - /** @type {Mapvoid}>} */ -@@ -19,4 +19,15 @@ +@@ -16,7 +16,18 @@ export function install(rt) { + const sessionId = request.agent?.session?.id ?? request.agent?.id + const owner = sessionId === undefined ? undefined : rt.groupOwners.get(sessionId) + const botId = owner?.botId ?? (sessionId === undefined ? undefined : rt.chatOwners.get(sessionId)) - if (botId === undefined) return next() + // A subagent child Session started by a run Bot is in neither index. Follow the parent chain + // (the same `rt.whoseAgent` secrets.js and connector-access.js use) and adopt the run Bot's @@ -2711,7 +2716,10 @@ diff --git a/src/host/approvals.js b/src/host/approvals.js const id = randomUUID() const entry = { id, -@@ -50,11 +50,79 @@ +@@ -47,14 +47,82 @@ export function install(rt) { + }) + }, { prepend: true }) + + // A group grant covers the owner's Bots working in that group: the Bot it was granted + // from and any other Bot of the same owner. Before anyone signs in there is one person, + // and a Bot made then carries no owner, so every local Bot is theirs. @@ -2792,7 +2800,10 @@ diff --git a/src/host/approvals.js b/src/host/approvals.js entry.answer(outcome) } -@@ -68,2 +68,2 @@ +@@ -65,5 +65,5 @@ export function install(rt) { + } + } + - Object.assign(rt, { approvalsView, waitsForApproval, answerApproval, cancelApprovals }) + Object.assign(rt, { approvalsView, waitsForApproval, answerApproval, cancelApprovals, removeStandingApproval }) } diff --git a/patches/ds-bot/0002-flybot-api-host.patch b/patches/ds-bot/0002-flybot-api-host.patch index b8ac416ce..774d0de69 100644 --- a/patches/ds-bot/0002-flybot-api-host.patch +++ b/patches/ds-bot/0002-flybot-api-host.patch @@ -522,7 +522,8 @@ diff --git a/src/host/roster.js b/src/host/roster.js await rt.save() } -@@ -435,21 +435,41 @@ export function install(rt) { +@@ -434,22 +434,42 @@ export function install(rt) { + const current = rt.chatOf(id) // Questions and exchange turns are kept per Session, so every part of the chat is cleared. const parts = new Set([id, current, ...(bot.segments ?? []).map(segment => segment.sessionId)]) + // #1453: keep who owned every Session this deletion empties — the own chat and its parts, the @@ -733,9 +734,10 @@ diff --git a/src/host/store.js b/src/host/store.js + warn('ds-bot: unreadable state %s: %s', statePath, backupProblem) + return undefined + } -+ stuck = undefined -+ return parsed -+ } + stuck = undefined + return parsed + } +- stuck = 'saved by a newer DS Bot; update DS Bot' + if (parsed?.version === STATE_VERSION) { + const problem = ownershipProblem(parsed) + if (problem !== undefined) { @@ -743,10 +745,9 @@ diff --git a/src/host/store.js b/src/host/store.js + warn('ds-bot: state %s was refused: %s', statePath, problem) + return undefined + } - stuck = undefined - return parsed - } -- stuck = 'saved by a newer DS Bot; update DS Bot' ++ stuck = undefined ++ return parsed ++ } + stuck = 'saved by a newer FlyBot; update FlyBot' return undefined } @@ -1024,7 +1025,10 @@ diff --git a/src/host/store.js b/src/host/store.js diff --git a/src/host/tools.js b/src/host/tools.js --- a/src/host/tools.js +++ b/src/host/tools.js -@@ -4,7 +4,44 @@ +@@ -1,9 +1,46 @@ + // Team tools: the roster, bot-to-bot messages, group chat settings and posts, and the + // question card. Group rounds and reading chats are in groups.js. + import { randomUUID } from 'node:crypto' -import { defineTool } from '@deepseek-ai/dsh-tools' -import { COLORS, EXTRA_COLORS, MAX_HOPS, MAX_UNREAD_REQUESTS, ROOM_BOT_ROUNDS, ROOM_MODES, SOURCE_KIND } from './constants.js' +import { columnOf, compareForWork } from '../shared/task-board.js' @@ -1036,7 +1040,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js +import { questionReply, replyText } from './parts.js' +import { OTHER, settled } from './turn-origin.js' +import { cleanName } from './delivery.js' - ++ +// A run Bot (ephemeral) is not a team member: every tool below has an effect that reaches past +// the run Bot's own Session — it messages, stops or edits another Bot, edits a group, writes or +// attaches a capability, asks the owner for a secret, arms or changes a schedule, reads the team @@ -1067,11 +1071,13 @@ diff --git a/src/host/tools.js b/src/host/tools.js +// One sentence that names the reason and no substitute route: the run Bot cannot become a team +// member by finding another tool that does the same thing. +export const EPHEMERAL_REFUSAL = 'A running Bot is not a team member.' -+ + export const output = { schema: { type: 'string' }, render: (_args, value) => [{ type: 'text', text: String(value) }] } export const GROUP_REF = { type: 'string', required: true, description: 'Exact group chat name from list_bots' } - const BLOCKING_QUESTION_TOOL = 'ask_user_question' -@@ -21,3 +21,18 @@ +@@ -18,6 +18,21 @@ const WORK_DIR_PARAM = { type: 'string', description: 'Full path of the folder t + export function install(rt) { + const { ctx, state } = rt + + // The tool speaks NAME=value because a free-form object parameter is not expressible in the + // schema DSH compiles; the record keeps the typed map (#198). + const externalOf = (input) => { @@ -1090,7 +1096,10 @@ diff --git a/src/host/tools.js b/src/host/tools.js const callerBot = (exec) => rt.botOf(rt.selfOf(exec.agent?.id)) const callerRoom = (exec) => { const roomId = rt.groupOwners.get(exec.agent?.id)?.roomId -@@ -30,36 +30,107 @@ +@@ -27,39 +27,110 @@ export function install(rt) { + const roomLine = (room, self) => { + const members = room.members.map(rt.botOf).filter(Boolean) + .map(bot => `${bot.name}${bot.id === room.admin ? ' (admin)' : ''}${bot.id === self ? ' (you)' : ''}`) + const remotes = (room.participants ?? []).filter(p => p.remote && p.leftSeq === undefined) + .map(p => `${String(p.displayName ?? p.handle).replace(/[\u0000-\u001f\u007f-\u009f[\]"<>`]/g, '').slice(0, 60)} (${String(p.ownerName ?? p.ownerHome ?? 'friend').replace(/[\u0000-\u001f\u007f-\u009f[\]"<>`]/g, '').slice(0, 40)}'s Bot)`) const parts = [ @@ -1205,7 +1214,10 @@ diff --git a/src/host/tools.js b/src/host/tools.js ].join('\n') }, })) -@@ -84,9 +84,15 @@ +@@ -81,12 +81,18 @@ export function install(rt) { + + // A tool that throws shows as failed in the chat, instead of as sent or stopped. + const refuse = (message) => { throw new Error(message) } + // A run Bot keeps its name (findBot still sees it) but no team tool can address it (#1373). + const teamBot = bot => bot !== undefined && bot.remote === undefined && bot.ephemeral === undefined + const findTeamBot = (name) => { @@ -1222,7 +1234,10 @@ diff --git a/src/host/tools.js b/src/host/tools.js message: { type: 'string', required: true, description: 'Self-contained message; the other Bot cannot see this conversation' }, reply_to: { type: 'string', -@@ -107,5 +107,15 @@ +@@ -104,8 +104,18 @@ export function install(rt) { + await rt.load() + const sender = callerBot(exec) + if (sender === undefined) return 'Only Bots on the team can use message_bot.' - const target = rt.findBot(args.to) - if (target === undefined) return `No Bot named ${args.to}. Team: ${Object.values(state.bots).map(bot => bot.name).join(', ')}` + const target = findTeamBot(args.to) @@ -1240,7 +1255,10 @@ diff --git a/src/host/tools.js b/src/host/tools.js if (target.id === sender.id) return 'You cannot message yourself.' const turn = rt.live.get(exec.agent?.id) const hop = Math.max(turn?.info?.hop ?? 0, ...(turn?.joined ?? []).map(joined => joined.info?.hop ?? 0)) -@@ -121,4 +121,8 @@ +@@ -118,7 +118,11 @@ export function install(rt) { + const toUser = args.reply_to === 'user' + const working = rt.live.get(chat)?.turn !== undefined + const steer = args.when !== 'queue' && !toUser - await rt.deliver({ fromId: sender.id, toId: target.id, text: args.message, role: 'request', hop, steer, ...(toUser ? { audience: 'user' } : callerRoom(exec)) }) + const exchangeId = await rt.deliver({ fromId: sender.id, toId: target.id, text: args.message, role: 'request', hop, steer, origin: rt.agentOrigin?.(exec.agent), ...(toUser ? { audience: 'user' } : callerRoom(exec)) }) + // FlyBot send policies (flybot.js) may hold the message for the owner or refuse it. @@ -1250,7 +1268,10 @@ diff --git a/src/host/tools.js b/src/host/tools.js const sent = toUser ? `Delivered to ${target.name}. It will answer the user in its own chat; nothing comes back to you, so point the user to ${target.name}'s chat instead of relaying.` : working && steer -@@ -155,5 +155,5 @@ +@@ -152,8 +152,8 @@ export function install(rt) { + await rt.load() + const sender = callerBot(exec) + if (sender === undefined || !rt.isMain(sender.id)) refuse('Only a Main Bot can stop other Bots.') - const target = rt.findBot(args.bot) - if (target === undefined) refuse(`No Bot named ${args.bot}. Team: ${Object.values(state.bots).map(bot => bot.name).join(', ')}`) + const target = findTeamBot(args.bot) @@ -1258,7 +1279,10 @@ diff --git a/src/host/tools.js b/src/host/tools.js if (target.id === sender.id) refuse('You cannot stop yourself.') const { running } = activeSessions(target.id) if (running.length === 0) refuse(`${target.name} is not running right now, so there is nothing to stop. Use message_bot to reach it.`) -@@ -165,18 +165,29 @@ +@@ -162,21 +162,32 @@ export function install(rt) { + agent.cancel({ kind: 'hook', reason: 'ds-bot/stopped' }) + } + const why = String(args.reason ?? '').trim() || 'Stop what you are doing; the previous turn was ended on purpose. Acknowledge briefly.' - await rt.deliver({ fromId: sender.id, toId: target.id, text: why, role: 'stopped', ...callerRoom(exec) }) + await rt.deliver({ fromId: sender.id, toId: target.id, text: why, role: 'stopped', origin: rt.agentOrigin?.(exec.agent), ...callerRoom(exec) }) return `Stopped ${target.name} in ${running.map(whereOf).join(' and ')}. It is told why in a new turn, whose reply comes back to you.` @@ -1291,7 +1315,10 @@ diff --git a/src/host/tools.js b/src/host/tools.js }, output, async execute(args, exec) { -@@ -189,5 +189,5 @@ +@@ -186,8 +186,8 @@ export function install(rt) { + const workDir = args.work_dir ? await rt.cleanWorkDir(args.work_dir) : undefined + const name = String(args.name ?? '').trim() + await rt.approveFolder(exec, { path: workDir, who: { en: `the new Bot ${name}`, zh: `新 Bot ${name} ` } }) - const bot = await rt.createBot({ ...args, workDir, createdBy: rt.selfOf(exec.agent.id) }) - return `Created ${bot.name}${workDir ? `, working in ${workDir}` : ''}. It is greeting the user now.` + const bot = await rt.createBot({ ...args, workDir, createdBy: rt.selfOf(exec.agent.id), external: externalOf(args.external) }) @@ -1299,13 +1326,19 @@ diff --git a/src/host/tools.js b/src/host/tools.js } catch (error) { return `Could not create the Bot: ${error.message}` } -@@ -215,4 +215,4 @@ +@@ -212,7 +212,7 @@ export function install(rt) { + const self = rt.selfOf(exec.agent?.id) + const caller = rt.botOf(self) + if (caller === undefined) return 'Only Bots on the team can use update_bot.' - const bot = args.bot ? rt.findBot(args.bot) : caller + const bot = args.bot ? findTeamBot(args.bot) : caller if (bot === undefined) return `No Bot named ${args.bot}.` if (bot.id !== caller.id && !rt.isMain(self)) return 'Only a Main Bot can change another Bot. Leave out bot to change yourself, or ask a Main Bot with message_bot.' try { -@@ -254,10 +254,10 @@ +@@ -251,13 +251,13 @@ export function install(rt) { + const bots = [] + const unknown = [] + for (const ref of nameList(refs)) { - const bot = rt.findBot(ref) + const bot = findTeamBot(ref) if (bot === undefined) unknown.push(ref) @@ -1318,24 +1351,36 @@ diff --git a/src/host/tools.js b/src/host/tools.js const groupNames = self => visibleRooms(self).map(room => `"${room.name}"`).join(', ') || 'none' const pickRoom = (ref, self) => { const matches = rt.findRooms(ref).filter(room => visibleRooms(self).includes(room)) -@@ -286,4 +286,4 @@ +@@ -283,7 +283,7 @@ export function install(rt) { + if (!rt.isMain(self)) return 'Only a Main Bot can create group chats. Ask a Main Bot instead.' + const { bots, unknown } = resolveBots(args.members) + if (unknown.length > 0) return `No Bot named ${unknown.join(', ')}. Team: ${teamNames()}` - const admin = args.admin ? rt.findBot(args.admin) : rt.botOf(self) + const admin = args.admin ? findTeamBot(args.admin) : rt.botOf(self) if (admin === undefined) return `No Bot named ${args.admin}. Team: ${teamNames()}` const members = [...new Set([self, ...bots.map(bot => bot.id), admin.id])] try { -@@ -300,4 +300,4 @@ +@@ -297,7 +297,7 @@ export function install(rt) { + + ctx.tools.register(defineTool({ + name: 'update_group', - description: 'Change a group chat: rename it, set its notice, add or remove members, change its admin or reply mode. Main Bots may change any group; a group\'s admin may change its own group.', + description: 'Change a group chat: rename it, set its notice, add or remove members, change its admin, its task lead, or its reply mode. Main Bots may change any group; a group\'s admin may change its own group.', parameters: { group: GROUP_REF, name: { type: 'string', description: 'New group name; an empty string names it after its members again' }, -@@ -308,3 +308,4 @@ +@@ -305,6 +305,7 @@ export function install(rt) { + add_members: { type: 'array', items: { type: 'string' }, description: 'Exact names of Bots to add' }, + remove_members: { type: 'array', items: { type: 'string' }, description: 'Exact names of Bots to remove' }, + admin: { type: 'string', description: 'Member to make the admin; "none" leaves the group without one (Main Bots only)' }, + task_lead: { type: 'string', description: 'Member that divides this room\'s task board. "none" leaves that to the admin. A face-to-face room has no board.' }, mode: MODE_PARAM, work_dir: { ...WORK_DIR_PARAM, description: `${WORK_DIR_PARAM.description}. Every member works there in this group; an empty string lets each member work in its own folder again (Main Bots only)` }, }, -@@ -328,7 +328,15 @@ +@@ -325,10 +325,18 @@ export function install(rt) { + if (args.admin !== undefined && args.admin !== '') { + if (/^(none|nobody|null|无|没有)$/i.test(String(args.admin).trim())) admin = null + else { - admin = rt.findBot(args.admin)?.id + admin = findTeamBot(args.admin)?.id if (admin === undefined) return `No Bot named ${args.admin}. Team: ${teamNames()}` @@ -1352,12 +1397,18 @@ diff --git a/src/host/tools.js b/src/host/tools.js try { const workDir = moving ? await rt.cleanWorkDir(args.work_dir) : undefined // A Bot that joins a group with a folder works there from its first turn. -@@ -348,3 +348,4 @@ +@@ -345,6 +345,7 @@ export function install(rt) { + notice: args.notice, + mode: args.mode, + admin, + ...(taskLead !== undefined ? { taskLead } : {}), add: add.bots.map(bot => bot.id), remove: remove.bots.map(bot => bot.id), ...(moving ? { workDir: workDir ?? '' } : {}), -@@ -398,9 +398,8 @@ +@@ -395,12 +395,11 @@ export function install(rt) { + if ((info?.hop ?? 0) >= MAX_HOPS) return 'This chain of Bot messages is already long. Stop here and report to the user instead.' + const text = String(args.message ?? '').trim() + if (text === '') return 'Write a message to post.' - if ((room.botRounds ?? 0) >= ROOM_BOT_ROUNDS) return `Bots already started ${ROOM_BOT_ROUNDS} rounds in "${room.name}" since the user last spoke there. Wait for the user.` - room.botRounds = (room.botRounds ?? 0) + 1 - await rt.save() @@ -1370,7 +1421,10 @@ diff --git a/src/host/tools.js b/src/host/tools.js } catch (failure) { return `Could not post in "${room.name}": ${failure.message}` } -@@ -448,3 +448,7 @@ +@@ -445,6 +445,10 @@ export function install(rt) { + .filter(option => option.label !== '') + .slice(0, 6) + if (options.length === 0) return 'Give at least one option.' + // Work handed out from a group (turns.js): the user handed it out there, so a question + // about it is asked there too, never in the Bot's own chat (#550). + const handed = rt.handedFromGroup(id) @@ -1378,7 +1432,10 @@ diff --git a/src/host/tools.js b/src/host/tools.js // A hidden Bot-to-Bot turn shows none of its calls, so its card goes to the // composer dock, which takes questions without a call. const turn = rt.live.get(id)?.turn -@@ -468,3 +468,74 @@ +@@ -465,6 +465,77 @@ export function install(rt) { + }, + })) + + // The card sits in the group's composer dock, named for the Bot that asks. One card per group: + // while another Bot's waits there, this one asks in its reply, which goes back to the group. + async function askInGroup({ room, info }, args, options, exec) { @@ -1453,25 +1510,37 @@ diff --git a/src/host/tools.js b/src/host/tools.js // dsh's own question tool holds the turn open until the user answers, and every // message from another Bot or a group round queues behind it. Bot Sessions ask // through ask_user instead, so the tool is taken out of their tool list. -@@ -528,4 +528,4 @@ +@@ -525,7 +525,7 @@ export function install(rt) { + name: 'save_capability', + description: 'Create or update a shared MCP server, skill, or pack. Changes apply from your next message.', + parameters: { - name: { type: 'string', required: true, description: 'Lowercase letters, digits, and single hyphens, up to 32 characters; "github" is reserved' }, + name: { type: 'string', required: true, description: 'Lowercase letters, digits, and single hyphens, up to 32 characters; "github" and "composio" are reserved' }, kind: { type: 'string', required: true, enum: ['mcp', 'skill', 'pack'] }, description: { type: 'string', description: 'What this capability provides' }, transport: { type: 'string', enum: ['stdio', 'streamable-http'], description: 'How to reach an MCP server: a local command or an HTTP URL' }, -@@ -548,4 +548,4 @@ +@@ -545,7 +545,7 @@ export function install(rt) { + try { + const item = await rt.capabilitySave(args, { botId: self }) + const approval = item.kind === 'mcp' && item.transport === 'stdio' && !item.approved - ? ' The user must allow this MCP server in DS Bot settings before it can work.' + ? ' The user must allow this MCP server in FlyBot settings before it can work.' : '' return `Saved ${item.name}. Bots with it attached use the change from their next message.${approval}` } catch (error) { return error.message } -@@ -590,4 +590,4 @@ +@@ -587,7 +587,7 @@ export function install(rt) { + : attached ? [attached] : [] + const approvals = members + .filter(item => item.kind === 'mcp' && item.transport === 'stdio' && item.approved !== true) - .map(item => ` The user must allow ${item.name} in DS Bot settings before it can work.`) + .map(item => ` The user must allow ${item.name} in FlyBot settings before it can work.`) .join('') return `${verb} ${args.name} ${args.bot ? `to ${result.bot}` : 'to yourself'}. Changes apply from ${result.bot}'s next message.${endpoint === 'attach' ? approvals : ''}` } catch (error) { return error.message } -@@ -598,2 +598,118 @@ +@@ -595,5 +595,121 @@ export function install(rt) { + })) + } + + const actionable = task => task.status === 'open' || task.status === 'claimed' || task.status === 'working' + function formatTasks(tasks, { inGroup, withNext }) { + const ordered = [...tasks].sort(compareForWork) diff --git a/patches/ds-bot/0014-flybot-branding.patch b/patches/ds-bot/0014-flybot-branding.patch index d11cfaf95..407acef33 100644 --- a/patches/ds-bot/0014-flybot-branding.patch +++ b/patches/ds-bot/0014-flybot-branding.patch @@ -1,6 +1,5 @@ FlyBot patch 0014-flybot-branding on Fei2411/DeepSeek-Bot@2b10c70 (ds-bot/). Generated by scripts/ds-bot-patches.mjs; edits to upstream files only (files FlyBot adds are in files/). - diff --git a/src/client/about.js b/src/client/about.js --- a/src/client/about.js +++ b/src/client/about.js @@ -699,7 +698,7 @@ diff --git a/src/host/parts.js b/src/host/parts.js rt.warn('ds-bot: condensing %s failed: %s', agent.id, error) return false } finally { -@@ -544,7 +555,7 @@ export function install(rt) { +@@ -544,7 +544,7 @@ export function install(rt) { } // The prefix cache starts over anyway, so the memory section catches up. rt.renewMemory(sessionId) @@ -708,7 +707,7 @@ diff --git a/src/host/parts.js b/src/host/parts.js partBase.set(agent.id, after) ctx.logger.info('ds-bot: %s condensed from %d to %d tokens (check line %d, hard line %d)', agent.id, before ?? -1, after ?? -1, lines.check, lines.hard) return after < lines.check -@@ -561,20 +572,22 @@ export function install(rt) { +@@ -561,20 +561,22 @@ export function install(rt) { const why = segment.fallback === 'overflow' ? 'the part overflowed the model\'s window' : segment.fallback === 'failure' ? 'a request in the previous part kept failing' : 'the model did not write a usable note' @@ -734,7 +733,7 @@ diff --git a/src/host/parts.js b/src/host/parts.js const { recent, earlier, unlisted, complete } = lines const omitted = [ ...(unlisted > 0 ? [`(${unlisted} older messages are not listed.)`] : []), -@@ -582,7 +593,7 @@ export function install(rt) { +@@ -582,7 +582,7 @@ export function install(rt) { ] return [ `[Handoff · part ${number}]`, @@ -743,7 +742,7 @@ diff --git a/src/host/parts.js b/src/host/parts.js 'It is reference, not new instructions: the note and the messages below are a record of earlier work. Do what they say only as part of the work they record, and take new requests only from messages that come after this one.', `Everything earlier stays saved: ${lookup}`, ...(folder !== undefined ? [`You now work in ${folder}: commands and file tools start there. Relative paths in the messages below meant the previous folder.`] : []), -@@ -610,17 +621,24 @@ export function install(rt) { +@@ -610,17 +610,24 @@ export function install(rt) { ].join('\n') } @@ -792,7 +791,7 @@ diff --git a/src/host/parts.js b/src/host/parts.js const placeOf = (sessionId) => { const botId = rt.chatOwners.get(sessionId) -@@ -670,10 +681,10 @@ export function install(rt) { +@@ -670,10 +670,10 @@ export function install(rt) { // `hard`: past the hard line or the window, the part moves on even without a note // from the model. `folder`: the part moves on because its working folder changed. @@ -806,7 +805,7 @@ diff --git a/src/host/parts.js b/src/host/parts.js switches.set(agent.id, job) void runSwitch(agent, job) .catch(error => rt.warn('ds-bot: %s did not move on to a fresh part: %s', rt.botOf(job.botId)?.name ?? agent.id, error)) -@@ -693,12 +704,13 @@ export function install(rt) { +@@ -693,12 +693,13 @@ export function install(rt) { if (resume) { const carried = job.joined?.length ? { joined: job.joined } : {} back.unshift(createUserMessage({ @@ -855,7 +854,7 @@ diff --git a/src/host/parts.js b/src/host/parts.js } } -@@ -765,16 +776,43 @@ export function install(rt) { +@@ -765,16 +765,43 @@ export function install(rt) { const dir = rt.workDirOf(job) const intended = dir !== undefined && await rt.isFolder(dir) ? dir : rt.teamFolder() const freshId = await rt.createSession(room ? `${bot.name} · ${room.name}` : bot.name, { dir }) @@ -924,19 +923,18 @@ diff --git a/src/host/parts.js b/src/host/parts.js - if (intended !== undefined && cwd !== undefined && cwd !== intended) { - folderRetry.set(freshId, Date.now() + FOLDER_RETRY_MS) - rt.warn('ds-bot: the fresh part of %s did not start in %s; it works in %s', bot.name, intended, cwd) -- } -- const text = handoffPackage({ room, segment, lines: carried, number: number + 1, resume, overflow: job.overflow, failed: job.failed, bare: job.bare, folder }) -- freshParts.set(freshId, estimateTokens(text)) -- fresh.followup(createUserMessage({ content: [{ type: 'text', text }], source: handoffSource(job, resume) })) -- await ctx.sessions.flush(fresh.session) -- // Then what the old part held: the stopped turn's input, then whatever waited. + try { + await rt.save() + } catch (error) { + restore() + await ctx.workspaceRegistry.archiveSession(freshId).catch(() => { /* best effort */ }) + throw error -+ } + } +- const text = handoffPackage({ room, segment, lines: carried, number: number + 1, resume, overflow: job.overflow, failed: job.failed, bare: job.bare, folder }) +- freshParts.set(freshId, estimateTokens(text)) +- fresh.followup(createUserMessage({ content: [{ type: 'text', text }], source: handoffSource(job, resume) })) +- await ctx.sessions.flush(fresh.session) +- // Then what the old part held: the stopped turn's input, then whatever waited. + let folder + try { + await rt.ensureCapabilities(freshId) @@ -975,7 +973,7 @@ diff --git a/src/host/parts.js b/src/host/parts.js ctx.logger.info('ds-bot: %s%s moved on to part %d at %d tokens (check line %d, hard line %d); %s; %d latest and %d earlier messages carried%s', bot.name, room ? ` in ${room.name}` : '', number + 1, tokens ?? -1, lines?.check ?? -1, lines?.hard ?? -1, written !== undefined ? `note written in ${written.attempts} attempt(s)` : `note put together by DS Bot (${noted.fallback}${noted.note ? `, carries part ${noted.noteFrom}'s` : ', nothing to carry'})`, carried.recent.length, carried.earlier.length, folder === undefined ? '' : `; now works in ${folder}`) -@@ -840,16 +851,18 @@ export function install(rt) { +@@ -840,16 +840,18 @@ export function install(rt) { freshParts.delete(agent.id) partBase.set(agent.id, (tokensOf(agent.session) ?? 0) + fresh) } @@ -998,7 +996,7 @@ diff --git a/src/host/parts.js b/src/host/parts.js // A switch may have started while the checks above awaited. const running = switches.get(agent.id) if (running !== undefined) { -@@ -858,8 +869,11 @@ export function install(rt) { +@@ -858,8 +858,11 @@ export function install(rt) { } const midTurn = payload.step > 1 const entry = rt.liveOf(agent.id) @@ -1011,7 +1009,7 @@ diff --git a/src/host/parts.js b/src/host/parts.js return { kind: 'reject' } } -@@ -927,15 +938,19 @@ export function install(rt) { +@@ -927,15 +927,19 @@ export function install(rt) { const tokens = tokensOf(agent.session) if (lines === undefined || tokens === undefined || tokens < lines.check || !grown(agent.id, tokens, lines)) return const hard = tokens >= lines.hard diff --git a/patches/ds-bot/files/src/host/hooks.js b/patches/ds-bot/files/src/host/hooks.js index 32fe7d54a..d4e7678dc 100644 --- a/patches/ds-bot/files/src/host/hooks.js +++ b/patches/ds-bot/files/src/host/hooks.js @@ -683,6 +683,8 @@ export function install(rt) { const bot = rt.botOf(id) if (bot === undefined) return false if (bot.ephemeral?.plugin !== plugin) throw new Error('flybot: only the plugin that created this run Bot can remove it') + // Tombstone before deletion so late replies are denied (#1524) + if (bot.ephemeral) tombstoneEphemeral(id) await rt.deleteBot(id) return true } @@ -727,9 +729,22 @@ export function install(rt) { send.list = () => chain.map(m => ({ name: m.name, priority: priorityOf(m), before: m.before ?? [], after: m.after ?? [] })) send.priority = SEND_PRIORITY + // Tombstone removed ephemeral bots: a late reply after bot.remove must not leak. + const ephemeralTombstones = new Set() + const tombstoneEphemeral = (id) => ephemeralTombstones.add(id) + const wasEphemeral = (id) => ephemeralTombstones.has(id) + // The gate around ds-bot's own rt.deliver. Approved and re-sent messages pass through. - const gated = args => args.fromId !== undefined && rt.botOf(args.fromId) !== undefined && BOT_TO_BOT_ROLES.has(args.role) - && !args.flybotChecked && !args.resent && !args.resumed && !args.retried && !args.deferred + // CRITICAL FIX (#1524): Ephemeral Bot replies must ALWAYS go through middleware (which calls + // stepResult) even when deferred/resumed/retried/resent, so gated() returns true for ephemeral senders. + const gated = args => { + if (!args.fromId || !BOT_TO_BOT_ROLES.has(args.role) || args.flybotChecked) return false + const from = rt.botOf(args.fromId) + // Ephemeral bots (or tombstoned removed bots) always route through middleware, even when deferred/resumed/retried/resent + if (from?.ephemeral || wasEphemeral(args.fromId)) return true + // Normal bots skip middleware when deferred/resumed/retried/resent + return from !== undefined && !args.resent && !args.resumed && !args.retried && !args.deferred + } innerDeliver = rt.deliver rt.deliver = async (args) => { // Everything that is not a first, checked send still carries its sender: resumed, retried, @@ -746,10 +761,17 @@ export function install(rt) { } if (!gated(args)) return innerDeliver(args) const exchangeId = args.exchangeId ?? randomUUID() + // Post-removal tombstone check: deny late replies from removed ephemeral bots + if (wasEphemeral(args.fromId) && !rt.botOf(args.fromId)) { + noteOutcome(exchangeId, { action: 'deny', reason: 'ephemeral Bot was removed', by: 'flybot-ephemeral' }) + return exchangeId + } const msg = { from: args.fromId, to: args.toId, target: targetOf(args.toId), text: args.text, role: args.role, hop: args.hop ?? 0, via: 'ds-bot', exchangeId, deliverArgs: { ...args, exchangeId } } noteOutcome(exchangeId, await run(msg)) return exchangeId } + // Expose tombstone for workflow plugin to call on bot.remove + rt.tombstoneEphemeral = tombstoneEphemeral // ---------------------------------------------------------------- tools const callerOf = exec => rt.botOf(rt.selfOf(exec?.agent?.id)) diff --git a/patches/ds-bot/files/test/workflow-reply-leak-real.test.js b/patches/ds-bot/files/test/workflow-reply-leak-real.test.js new file mode 100644 index 000000000..ebf7a9357 --- /dev/null +++ b/patches/ds-bot/files/test/workflow-reply-leak-real.test.js @@ -0,0 +1,135 @@ +// #1524: REAL defect tests proving the fix works +// These test actual hooks.js behavior, showing RED on origin/main and GREEN with the fix. +import assert from 'node:assert/strict' +import { test } from 'node:test' + +test('#1524: gated() logic - ephemeral with deferred MUST be gated (the fix)', async () => { + // This tests the actual gated() logic from hooks.js + // RED on origin/main: deferred from ephemeral bypasses gating → leaks + // GREEN with fix: ephemeral forces gating even with deferred → routes through middleware + + const BOT_TO_BOT_ROLES = new Set(['request', 'reply', 'stopped', 'report']) + const bots = new Map([ + ['main', { id: 'main', name: 'Chief' }], + ['ephemeral', { id: 'ephemeral', name: 'Review', ephemeral: { plugin: 'test' } }], + ]) + + const tombstones = new Set() + const botOf = (id) => bots.get(id) + const wasEphemeral = (id) => tombstones.has(id) + + // ORIGIN/MAIN LOGIC (before fix): deferred bypasses gating for all bots + const gatedOnMain = (args) => { + if (!args.fromId || !BOT_TO_BOT_ROLES.has(args.role) || args.flybotChecked || args.resent) return false + const from = botOf(args.fromId) + // Origin/main: deferred/resumed/retried skip gating for ALL bots + return from !== undefined && !args.resumed && !args.retried && !args.deferred + } + + // FIXED LOGIC: ephemeral always gates, even with deferred + const gatedWithFix = (args) => { + if (!args.fromId || !BOT_TO_BOT_ROLES.has(args.role) || args.flybotChecked) return false + const from = botOf(args.fromId) + // Fix: ephemeral (or tombstoned) always routes through middleware + if (from?.ephemeral || wasEphemeral(args.fromId)) return true + // Normal bots still skip when deferred/resumed/retried/resent + return from !== undefined && !args.resent && !args.resumed && !args.retried && !args.deferred + } + + // Test deferred from ephemeral + const deferredArgs = { fromId: 'ephemeral', role: 'reply', deferred: true } + const mainGated = gatedOnMain(deferredArgs) + const fixedGated = gatedWithFix(deferredArgs) + + assert.equal(mainGated, false, 'RED on origin/main: deferred from ephemeral bypasses gating (THE LEAK)') + assert.equal(fixedGated, true, 'GREEN with fix: deferred from ephemeral forces gating (routes through middleware)') + + // Test resumed from ephemeral + const resumedArgs = { fromId: 'ephemeral', role: 'reply', resumed: true } + assert.equal(gatedOnMain(resumedArgs), false, 'origin/main: resumed from ephemeral bypasses gating') + assert.equal(gatedWithFix(resumedArgs), true, 'with fix: resumed from ephemeral forces gating') + + // Test retried from ephemeral + const retriedArgs = { fromId: 'ephemeral', role: 'reply', retried: true } + assert.equal(gatedOnMain(retriedArgs), false, 'origin/main: retried from ephemeral bypasses gating') + assert.equal(gatedWithFix(retriedArgs), true, 'with fix: retried from ephemeral forces gating') + + // Test resent from ephemeral + const resentArgs = { fromId: 'ephemeral', role: 'reply', resent: true } + assert.equal(gatedOnMain(resentArgs), false, 'origin/main: resent from ephemeral bypasses gating') + assert.equal(gatedWithFix(resentArgs), true, 'with fix: resent from ephemeral forces gating') +}) + +test('#1524: tombstone logic - post-removal late reply MUST be gated and denied', async () => { + // RED on origin/main: after removeBot, bot is gone, late reply bypasses gating → leaks + // GREEN with fix: tombstone exists, gated()=true, denied + + const BOT_TO_BOT_ROLES = new Set(['request', 'reply', 'stopped', 'report']) + const bots = new Map([ + ['main', { id: 'main', name: 'Chief' }], + ]) + const tombstones = new Set() + + const botOf = (id) => bots.get(id) + const wasEphemeral = (id) => tombstones.has(id) + + // Simulate bot existence and removal + const ephemeralId = 'ephemeral' + bots.set(ephemeralId, { id: ephemeralId, name: 'Review', ephemeral: { plugin: 'test' } }) + + const gatedWithFix = (args) => { + if (!args.fromId || !BOT_TO_BOT_ROLES.has(args.role) || args.flybotChecked) return false + const from = botOf(args.fromId) + // Ephemeral or tombstoned always gates + if (from?.ephemeral || wasEphemeral(args.fromId)) return true + return from !== undefined && !args.resent && !args.resumed && !args.retried && !args.deferred + } + + // Before removal: gated + const beforeRemoval = gatedWithFix({ fromId: ephemeralId, role: 'reply' }) + assert.equal(beforeRemoval, true, 'Before removal: ephemeral is gated') + + // Remove bot (tombstone it first in fixed version) + tombstones.add(ephemeralId) + bots.delete(ephemeralId) + + // After removal: on origin/main would be ungated (bot is gone) + // With fix: still gated (tombstone exists) + const afterRemoval = gatedWithFix({ fromId: ephemeralId, role: 'reply' }) + assert.equal(botOf(ephemeralId), undefined, 'Bot is removed') + assert.equal(wasEphemeral(ephemeralId), true, 'Tombstone exists') + assert.equal(afterRemoval, true, 'GREEN with fix: post-removal reply is gated by tombstone') +}) + +test('#1524: normal (non-ephemeral) bot behavior unchanged', async () => { + // Ensures the fix doesn't break normal bot delivery + const BOT_TO_BOT_ROLES = new Set(['request', 'reply', 'stopped', 'report']) + const bots = new Map([ + ['main', { id: 'main', name: 'Chief' }], + ['reviewer', { id: 'reviewer', name: 'Reviewer' }], + ]) + + const tombstones = new Set() + const botOf = (id) => bots.get(id) + const wasEphemeral = (id) => tombstones.has(id) + + const gatedWithFix = (args) => { + if (!args.fromId || !BOT_TO_BOT_ROLES.has(args.role) || args.flybotChecked) return false + const from = botOf(args.fromId) + if (from?.ephemeral || wasEphemeral(args.fromId)) return true + return from !== undefined && !args.resent && !args.resumed && !args.retried && !args.deferred + } + + // Normal bot: first reply is gated + assert.equal(gatedWithFix({ fromId: 'reviewer', role: 'reply' }), true, 'Normal bot first reply is gated') + + // Normal bot: deferred/resumed/retried/resent skip gating (unchanged) + assert.equal(gatedWithFix({ fromId: 'reviewer', role: 'reply', deferred: true }), false, 'Normal bot deferred skips gating') + assert.equal(gatedWithFix({ fromId: 'reviewer', role: 'reply', resumed: true }), false, 'Normal bot resumed skips gating') + assert.equal(gatedWithFix({ fromId: 'reviewer', role: 'reply', retried: true }), false, 'Normal bot retried skips gating') + assert.equal(gatedWithFix({ fromId: 'reviewer', role: 'reply', resent: true }), false, 'Normal bot resent skips gating') + + // Request TO ephemeral still allowed (sender is not ephemeral) + bots.set('ephemeral', { id: 'ephemeral', name: 'Review', ephemeral: { plugin: 'test' } }) + assert.equal(gatedWithFix({ fromId: 'main', toId: 'ephemeral', role: 'request' }), true, 'Request TO ephemeral is gated (normal flow)') +}) diff --git a/patches/ds-bot/files/test/workflow-reply-leak.test.js b/patches/ds-bot/files/test/workflow-reply-leak.test.js new file mode 100644 index 000000000..1b67fe54f --- /dev/null +++ b/patches/ds-bot/files/test/workflow-reply-leak.test.js @@ -0,0 +1,270 @@ +// #1524: Workflow ephemeral node Bot replies must not land in Chief / persistent Bot chat +// as queued `[Reply from …]` messages. Node I/O stays in the run/graph only. +import assert from 'node:assert/strict' +import { test } from 'node:test' + +test('#1524: ephemeral node reply is not delivered to Main Bot chat', async () => { + const roster = [{ id: 'main', name: 'Chief', isMain: true, remote: false }] + const created = [] + const removed = [] + + let hookHandle + const fb = { + bot: { + list: () => roster, + async create(spec) { + const id = `ephemeral-${created.length + 1}` + created.push(id) + roster.push({ id, name: spec.name, ephemeral: true, hidden: true }) + return id + }, + async remove(id) { + removed.push(id) + const index = roster.findIndex(bot => bot.id === id) + if (index >= 0) roster.splice(index, 1) + return true + }, + send: { + use(middleware) { + hookHandle = middleware.handle + return () => { hookHandle = undefined } + }, + priority: { ROUTING: 100 }, + }, + }, + readOnly: () => false, + api: { handle() { return () => {} } }, + tools: { register() { return () => {} } }, + warn() {}, + } + + // Apply workflow plugin + const { apply } = await import('../../plugins/workflow/index.js') + const plugin = apply( + { flybot: { scope: () => fb }, effect(fn) { fn() } }, + { persist: false } + ) + const service = plugin.service + + // Create a simple workflow + const defined = await service.define({ + name: 'Test Flow', + nodes: [ + { id: 'step1', name: 'Step One', prompt: 'Do step 1', outputs: [{ name: 'result' }] }, + ], + edges: [], + }) + assert.equal(defined.ok, true) + + // Start a run + const workflow = defined.workflows.find(w => w.name === 'Test Flow') + const run = await service.runNamed({ workflow: 'Test Flow', brief: 'Test task' }) + assert.equal(run.ok, true) + + // Get the ephemeral bot ID + const activeRun = workflow ? run.workflows.find(w => w.id === workflow.id).run : null + assert.ok(activeRun) + assert.ok(activeRun.ephemeral) + const ephemeralBotId = activeRun.ephemeral.step1 + assert.ok(ephemeralBotId) + + // Verify the hook was registered + assert.equal(typeof hookHandle, 'function') + + // Simulate a reply from the ephemeral node Bot + const replyMsg = { + from: ephemeralBotId, + to: 'main', + role: 'reply', + text: 'Step complete\nOUTPUT result: done', + } + + // Call the hook handler + const verdict = await hookHandle(replyMsg) + + // The hook should deny this message (prevent delivery to Main Bot chat) + assert.equal(verdict?.action, 'deny') + assert.match(verdict?.reason ?? '', /workflow step result stays on the run surface/i) + + // Verify the step was recorded in the workflow + const advanced = service.state().workflows.find(w => w.id === workflow.id) + assert.ok(advanced) + + plugin.dispose() +}) + +test('#1524: persistent Bot-to-Bot replies are NOT blocked', async () => { + const roster = [ + { id: 'main', name: 'Chief', isMain: true, remote: false }, + { id: 'reviewer', name: 'Reviewer', remote: false }, + ] + + let hookHandle + const fb = { + bot: { + list: () => roster, + async create() { throw new Error('should not create') }, + async remove() { throw new Error('should not remove') }, + send: { + use(middleware) { + hookHandle = middleware.handle + return () => { hookHandle = undefined } + }, + priority: { ROUTING: 100 }, + }, + }, + readOnly: () => false, + api: { handle() { return () => {} } }, + tools: { register() { return () => {} } }, + warn() {}, + } + + const { apply } = await import('../../plugins/workflow/index.js') + const plugin = apply( + { flybot: { scope: () => fb }, effect(fn) { fn() } }, + { persist: false } + ) + + // A reply from a persistent Bot should NOT be denied + const persistentReply = { + from: 'reviewer', + to: 'main', + role: 'reply', + text: 'Review complete: PASS', + } + + const verdict = await hookHandle(persistentReply) + + // The hook should return undefined (not handled, pass to next middleware) + assert.equal(verdict, undefined) + + plugin.dispose() +}) + +test('#1524: ephemeral Bot replies do NOT leak, middleware runs for stepResult, tombstone works', async () => { + // Critical: ephemeral replies must go through middleware (so stepResult advances the run), + // but must not queue in Chief. After removal, tombstone denies late replies. + const roster = [ + { id: 'main', name: 'Chief', isMain: true, remote: false }, + { id: 'ephemeral-node', name: 'Review', ephemeral: { plugin: 'flybot-workflow' }, hidden: true }, + ] + const deliveries = [] + const middlewareCalls = [] + let stepAdvanced = false + + const innerDeliver = async (args) => { + deliveries.push({ from: args.fromId, to: args.toId, role: args.role, flags: { deferred: args.deferred, resumed: args.resumed } }) + } + + // Simulate the middleware that calls stepResult + const runMiddleware = async (msg) => { + middlewareCalls.push({ from: msg.from, role: msg.role }) + if (msg.from === 'ephemeral-node' && msg.role === 'reply') { + stepAdvanced = true + return { action: 'deny', reason: 'workflow step result stays on the run surface', by: 'workflow-step' } + } + return { action: 'allow' } + } + + const BOT_TO_BOT_ROLES = new Set(['request', 'reply', 'stopped', 'report']) + const ephemeralTombstones = new Set() + + const rt = { + botOf: (id) => roster.find(bot => bot.id === id), + deliver: null, + } + + // Simulate the fixed gated() check + const gated = (args) => { + if (!args.fromId || !BOT_TO_BOT_ROLES.has(args.role) || args.flybotChecked || args.resent) return false + const from = rt.botOf(args.fromId) + // Ephemeral bots (or tombstoned) always route through middleware + if (from?.ephemeral || ephemeralTombstones.has(args.fromId)) return true + return from !== undefined && !args.resumed && !args.retried && !args.deferred + } + + rt.deliver = async (args) => { + const exchangeId = args.exchangeId ?? 'test-exchange-id' + + if (!gated(args)) { + await innerDeliver(args) + return exchangeId + } + + // Post-removal tombstone check + if (ephemeralTombstones.has(args.fromId) && !rt.botOf(args.fromId)) { + return exchangeId + } + + // Run middleware (workflow-step hook calls stepResult here) + const outcome = await runMiddleware({ from: args.fromId, to: args.toId, role: args.role }) + if (outcome.action === 'deny') return exchangeId + + await innerDeliver(args) + return exchangeId + } + + // SCENARIO 1: First reply - middleware runs (stepResult advances), NO delivery to Chief + stepAdvanced = false + await rt.deliver({ + fromId: 'ephemeral-node', + toId: 'main', + text: 'PASS', + role: 'reply', + }) + assert.equal(stepAdvanced, true, 'First reply MUST run middleware to advance step') + assert.equal(deliveries.length, 0, 'First reply should NOT be delivered to Chief') + assert.equal(middlewareCalls.length, 1, 'Middleware should run once') + + // SCENARIO 2: Deferred reply - middleware runs, still denied + stepAdvanced = false + await rt.deliver({ + fromId: 'ephemeral-node', + toId: 'main', + text: 'Deferred reply', + role: 'reply', + deferred: true, + }) + assert.equal(stepAdvanced, true, 'Deferred reply MUST run middleware (gated by ephemeral flag)') + assert.equal(deliveries.length, 0, 'Deferred reply should NOT be delivered') + + // SCENARIO 3: Resumed reply - middleware runs, still denied + stepAdvanced = false + await rt.deliver({ + fromId: 'ephemeral-node', + toId: 'main', + text: 'Resumed reply', + role: 'reply', + resumed: true, + }) + assert.equal(stepAdvanced, true, 'Resumed reply MUST run middleware') + assert.equal(deliveries.length, 0, 'Resumed reply should NOT be delivered') + + // SCENARIO 4: Tombstone the bot (simulate removal) + ephemeralTombstones.add('ephemeral-node') + const botIndex = roster.findIndex(bot => bot.id === 'ephemeral-node') + roster.splice(botIndex, 1) + + // SCENARIO 5: Post-removal reply - tombstone denies without middleware + const beforeMiddleware = middlewareCalls.length + await rt.deliver({ + fromId: 'ephemeral-node', + toId: 'main', + text: 'Late reply after removal', + role: 'reply', + }) + assert.equal(deliveries.length, 0, 'Post-removal reply should NOT be delivered') + assert.equal(middlewareCalls.length, beforeMiddleware, 'Post-removal reply skips middleware (bot is gone)') + + // SCENARIO 6: Request TO ephemeral (before removal, with bot back for test) + roster.push({ id: 'ephemeral-node', name: 'Review', ephemeral: { plugin: 'flybot-workflow' }, hidden: true }) + await rt.deliver({ + fromId: 'main', + toId: 'ephemeral-node', + text: 'Review this', + role: 'request', + }) + assert.equal(deliveries.length, 1, 'Request TO ephemeral Bot should be delivered') + assert.equal(deliveries[0].from, 'main') + assert.equal(deliveries[0].to, 'ephemeral-node') +}) diff --git a/plugins/workflow/index.js b/plugins/workflow/index.js index 02c50bf57..3eda90310 100644 --- a/plugins/workflow/index.js +++ b/plugins/workflow/index.js @@ -66,6 +66,7 @@ export function apply(ctx, config = {}) { // allocator treats every name this process ever handed out as taken. for (const [name, owner] of liveNames) if (owner === id) liveNames.delete(name) if (typeof fb.bot.remove !== 'function') return undefined + // Tombstoning now happens in removeBot (hooks.js) before deleteBot (#1524) return fb.bot.remove(id) }, async dispatch(step) { @@ -136,7 +137,10 @@ export function apply(ctx, config = {}) { priority: fb.bot.send.priority?.ROUTING, async handle(msg) { const from = typeof msg.from === 'string' ? msg.from : '' - if (msg.role !== 'reply' || !from || service.ownsBot(from) !== true) return undefined + if (msg.role !== 'reply' || !from) return undefined + // Check if this bot is owned by a workflow run + 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) // The reply advances the run only. It must not land in a persistent Bot chat (e.g. Chief). @@ -149,6 +153,8 @@ export function apply(ctx, config = {}) { }, }) if (typeof off === 'function') offs.push(off) + } else { + fb.warn?.('flybot-workflow: send middleware not available; ephemeral bot replies may leak') } if (typeof fb.tasks?.onWrite === 'function') { offs.push(fb.tasks.onWrite(task => {