From 465fc62876bab556a12d26f6c9520a9318ffaeda Mon Sep 17 00:00:00 2001 From: Dev-4 Date: Wed, 28 Oct 2026 04:25:01 +0000 Subject: [PATCH] fix(workflow): a switched-off connector stays off for a run (#1544) A Workflow node names the connectors its run Bot may call (#1535), so the graph could hand a Bot back a connector its owner turned off. The run now records who started it (createdBy) and every Bot whose switches cap it (cappedBy: the caller and the Bot that saved the graph), and connector-access.js refuses a connector turned off for any of them and masks it out of the run Bot's list. No caller (the board's ingest, the canvas, the workflow/* API) is capped by the graph's author; a save with no Bot behind it records no author and leaves no ceiling, and a graph saved before this keeps today's behaviour. --- changelog.d/1544.fixed.md | 1 + core/src/host/connector-access.js | 27 ++++- core/src/host/hooks.js | 21 +++- core/test/connector-access.test.js | 37 ++++++ docs/subsystems/core-host.md | 2 +- docs/subsystems/plugin-runtime.md | 2 +- docs/subsystems/workflow.md | 5 + .../ds-bot/files/src/host/connector-access.js | 27 ++++- patches/ds-bot/files/src/host/hooks.js | 21 +++- .../files/test/connector-access.test.js | 37 ++++++ plugins/workflow/index.js | 9 +- plugins/workflow/src/ephemeral.js | 15 ++- plugins/workflow/src/service.js | 39 ++++++- plugins/workflow/test/ephemeral.test.js | 108 ++++++++++++++++++ 14 files changed, 332 insertions(+), 19 deletions(-) create mode 100644 changelog.d/1544.fixed.md diff --git a/changelog.d/1544.fixed.md b/changelog.d/1544.fixed.md new file mode 100644 index 000000000..3bf457644 --- /dev/null +++ b/changelog.d/1544.fixed.md @@ -0,0 +1 @@ +A connector a Bot switched off can no longer be reopened by a Workflow. The run Bot a node spawns records who started the run (`createdBy`) and which Bots' switches cap it (`cappedBy`: the caller and the Bot that saved the graph), and the host refuses a connector turned off for any of them — with `turned off for ; the step's tools list cannot turn it back on` — and masks it out of the run Bot's list, so naming `github` on a node cannot turn a switch back on. A run started from the board, the canvas or the API has no caller and is capped by the graph's author; a graph saved with no Bot behind it has no ceiling, and an older graph keeps today's behaviour. ([#1544](https://github.com/ThinkFlowLab/FlyBot/issues/1544)) diff --git a/core/src/host/connector-access.js b/core/src/host/connector-access.js index 53b6e0b45..1c1ca4ab1 100644 --- a/core/src/host/connector-access.js +++ b/core/src/host/connector-access.js @@ -53,7 +53,24 @@ export function install(rt) { const tools = bot?.stepTools ?? [] return tools.includes(id) || tools.includes('*') || tools.some(name => name === id || String(name).startsWith(`mcp__${id}__`)) } - const allowed = (bot, id) => bot.remote === undefined && !turnedOff(bot, id) && (bot.ephemeral === undefined || stepGrants(bot, id)) + // #1544: a run Bot must not reopen a connector a Bot switched off. The record names the Bots whose + // switches cap it — the caller that started the run (createdBy), and the Bot that saved the graph + // (cappedBy), which is the ceiling when the board, the canvas or the API started the run. A Bot that + // no longer exists, and a record with neither field (an older run), leave the answer as it was: + // there is no switch list to intersect with. + const capsOf = (bot) => { + const ids = [bot?.createdBy, ...(Array.isArray(bot?.cappedBy) ? bot.cappedBy : [])] + .filter(id => typeof id === 'string' && id !== '') + const caps = [] + for (const id of ids) { + const cap = rt.botOf(id) + if (cap !== undefined && !caps.includes(cap)) caps.push(cap) + } + return caps + } + const capOff = (bot, id) => capsOf(bot).find(cap => turnedOff(cap, id)) + const allowed = (bot, id) => bot.remote === undefined && !turnedOff(bot, id) + && (bot.ephemeral === undefined || (stepGrants(bot, id) && capOff(bot, id) === undefined)) // The Bot an Agent works for: its own chat, a group Session and an earlier part resolve through // selfOf; a sub-agent Session is followed up to its parent. The answer has three shapes, and only @@ -126,7 +143,13 @@ export function install(rt) { const { bot } = who if (allowed(bot, id)) return undefined if (bot.remote !== undefined) return `The ${id} connector is not available to a friend's Bot.` - if (bot.ephemeral !== undefined) return `The ${id} connector is not granted to this run. The step's tools list is what allows it.` + if (bot.ephemeral !== undefined) { + // #1544: the step did name it — a Bot's switch is what refuses it, and naming it again in the + // graph cannot turn it back on. + const cap = capOff(bot, id) + if (cap !== undefined) return `The ${id} connector is turned off for ${cap.name}; the step's tools list cannot turn it back on.` + return `The ${id} connector is not granted to this run. The step's tools list is what allows it.` + } return `The ${id} connector is turned off for ${bot.name}. Its owner can turn it on in this Bot's settings.` }) diff --git a/core/src/host/hooks.js b/core/src/host/hooks.js index d4e7678dc..6d27281e7 100644 --- a/core/src/host/hooks.js +++ b/core/src/host/hooks.js @@ -168,10 +168,26 @@ export function install(rt) { return tools } - async function create({ name, systemPrompt, fields, role, instructions, createdBy, plugin = SHARED, hidden, ephemeral, model, tools, permission } = {}) { + // #1544: the Bots whose connector switches cap an ephemeral Bot (a Workflow run names the Bot that + // started it and the Bot that saved the graph). Only the ids are stored; each is read at call time, + // so a switch flipped later applies at once and a Bot that is gone stops capping. + function cleanCappedBy(value) { + if (!Array.isArray(value)) return [] + const ids = [] + for (const item of value) { + const id = String(item ?? '').trim() + if (id === '' || ids.includes(id)) continue + ids.push(id) + if (ids.length === 8) break + } + return ids + } + + async function create({ name, systemPrompt, fields, role, instructions, createdBy, cappedBy, plugin = SHARED, hidden, ephemeral, model, tools, permission } = {}) { checkName('plugin', plugin) await rt.load() const stepTools = ephemeral === true ? cleanStepTools(tools) : [] + const caps = ephemeral === true ? cleanCappedBy(cappedBy) : [] const preset = ephemeral === true ? String(permission ?? '').trim() : '' const bot = await rt.createBot({ name, role, instructions, createdBy, @@ -184,6 +200,7 @@ export function install(rt) { }) const record = rt.botOf(bot.id) if (record && stepTools.length > 0) record.stepTools = stepTools + if (record && caps.length > 0) record.cappedBy = caps if (record && preset) { try { await rt.setPermission(record, preset) @@ -192,7 +209,7 @@ export function install(rt) { throw error } } - if (record && (stepTools.length > 0 || preset)) await rt.save() + if (record && (stepTools.length > 0 || caps.length > 0 || preset)) await rt.save() for (const [key, value] of Object.entries(fields ?? {})) fire('change', { botId: bot.id, plugin, key, value: clone(value), prev: undefined }) return bot.id } diff --git a/core/test/connector-access.test.js b/core/test/connector-access.test.js index 939e533b6..e7479823a 100644 --- a/core/test/connector-access.test.js +++ b/core/test/connector-access.test.js @@ -572,3 +572,40 @@ test('a run Bot gets a connector only when its step tools name it', async (t) => assert.deepEqual(sorted(hiddenFrom('bare')), sorted([...COMPOSIO, ...GITHUB]), 'no stepTools fails closed') assert.match(guard(GITHUB[0], 'bare'), /not granted to this run/) }) + +// #1544: a Workflow node names the connectors it may call, and since #1535 that names enough to reach +// them. That must not become a way around a switch: a connector turned off for the Bot that started +// the run (or that saved the graph) stays off for the run Bot, and the tool is masked out of its list +// (denied()/restrict()), not only refused when it is called. +test('a run Bot does not reopen a connector the Bot that started the run switched off', async (t) => { + const state = team() + state.bots.writer.connectors = { github: false } + const runBot = (id, { createdBy, cappedBy, stepTools = ['github'] } = {}) => { + state.bots[id] = { + id, sessionId: id, name: id, role: 'Run', instructions: '', createdAt: 4, + hidden: true, ephemeral: { plugin: 'flybot-workflow' }, stepTools, + ...(createdBy === undefined ? {} : { createdBy }), + ...(cappedBy === undefined ? {} : { cappedBy }), + } + } + runBot('caller-off', { createdBy: 'writer' }) + runBot('caller-on', { createdBy: 'chief' }) + runBot('caller-gone', { createdBy: 'removed-bot' }) + runBot('author-off', { cappedBy: ['writer'] }) + runBot('both-on', { createdBy: 'chief', cappedBy: ['chief'] }) + const { agentFor, hiddenFrom, guard, stop } = await setup({ state }) + t.after(stop) + await Promise.all(['caller-off', 'caller-on', 'caller-gone', 'author-off', 'both-on'].map(id => agentFor(id))) + + assert.match(guard(GITHUB[0], 'caller-off'), /turned off for Writer; the step's tools list cannot turn it back on/, 'the switch is the reason, not the step list') + assert.deepEqual(sorted(hiddenFrom('caller-off')), sorted([...COMPOSIO, ...GITHUB]), 'and the tool is out of the run Bot\'s list too') + assert.equal(guard(GITHUB[0], 'caller-on'), undefined, 'a caller that still has github keeps the #1535 answer') + assert.deepEqual(sorted(hiddenFrom('caller-on')), sorted(COMPOSIO)) + // A run nobody's tool call started (the board, the canvas, the API) has no caller; the Bot that + // saved the graph is the ceiling then. + assert.match(guard(GITHUB[0], 'author-off'), /turned off for Writer; the step's tools list cannot turn it back on/) + assert.deepEqual(sorted(hiddenFrom('author-off')), sorted([...COMPOSIO, ...GITHUB])) + assert.equal(guard(GITHUB[0], 'both-on'), undefined) + // A Bot that no longer exists has no switch list to read: the answer stays what it was. + assert.equal(guard(GITHUB[0], 'caller-gone'), undefined) +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 57308f714..c12d4dc21 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -334,7 +334,7 @@ Work a member hands a Bot with `message_bot` from its group Session runs in a hi - New Bot model (#410): `roster.js` `createBot` fixes a Bot's model at creation. A Bot made by another Bot (`create_bot`, a tree child via `flybot.bot.create` with `createdBy`) and given no model starts on its maker's effective model (`desiredModel`) and its thinking level resolved against that model (dropped when that model does not serve it), so its first request never goes to an unconfigured profile default. A model the caller names wins; a Bot the user makes in the UI keeps the client's start model (Main Bot's, #230). Covered by `core/test/roster.test.js`. - Silent creation (#645): `createBot` and `create-room` send nothing by themselves. A Bot's model is applied without a turn, and a new group posts no opening round, so making a team costs no model turn. A line the user typed in the new-Bot card is their first request: `new-chat.js` sends it to the new Bot as one user turn and the card carries no `brief`; `roster.js` turns a `brief` into `Why you exist: …` instructions only when a Bot created the Bot (`create_bot`, a tree child), where the brief is a parent's context. The `kickoff` role, the `[Bot team setup]` text and the group-created cue are gone (an older transcript's kickoff line is still skipped by `historyEntry`), and `create_bot` answers that the new Bot starts no turn. Covered by `core/test/roster.test.js`, `core/test/rounds.test.js` and `core/test/new-chat.test.js`. - Every new Bot is announced (`bot.created`) through the `rt.createBot` wrapper, including the first Main Bot `bootstrap` creates on the first `state` request and a `duplicate-bot` copy. Creating those through the module-local `createBot` used to skip the event, so a plugin that adopts new Bots (the tree root) could miss the first Chief or leave a copy outside the tree until a restart. A run Bot is the exception: `createBot` stores `hidden` and `ephemeral: { plugin }` before the first save, and the wrapper does not fire `bot.created` for it. Covered by `core/test/flybot.test.js`. -- A run Bot (#1373) is left out of the channel `state` view, so the sidebar, search and Settings → Bots never list it. `list_bots`, `message_bot`, `stop_bot`, `update_bot` and group member lookup skip it too. `findBot` still sees the name, so a later Bot cannot reuse it. It is not a permissionless shell: `flybot.bot.create` may pass `tools` and `permission` for an ephemeral Bot only. `stepTools` is what connector-access and the approval listener read. `github` grants `mcp__github__*`; a connector the step did not name is hidden and a call is refused (`not granted to this run`). A hidden run Bot cannot show an approval card, so a granted call is `allowed-once` and an ungranted connector or an ungranted full-access shell escalation is `rejected`. **It is not a team member either (#1445):** a node's `tools` covers connectors and the shell, not the team surface, so `EPHEMERAL_DENIED_TOOLS` in `tools.js` names every tool whose effect reaches past the run Bot's own Session — `message_bot`/`stop_bot`/`create_bot`/`update_bot`, groups and posts, `ask_user`, the capability writes, `request_secret`/`list_secrets`, the schedule writes, `send_message`, the social and workflow tools, `read_browser`, `recall` (its default scope reads the team memory) and `create_child_bot`. `connector-access.js` refuses each call with `A running Bot is not a team member.`, `approvals.js` answers `rejected` for the same names (also for a subagent child Session, by following the parent chain through `rt.whoseAgent`), and the run Bot's own list loses them through `restrict({ deny })`; a name the profile serves but the mask cannot enforce stays listed with a warn line, and the guard is what refuses it. `EPHEMERAL_ALLOWED_TOOLS` is the rest: the read-only tools (`list_bots`, `list_models`, `list_capabilities`, `bot_schedules`, `schedule_list`, `read_own_chat`, `read_group_chat`) and `remember`/`forget`, which `writeDenied` keeps inside the run Bot's own scope. Ordinary Bots, friend's Bots and group rooms are unchanged, and `plugins/workflow/**` is not edited: the node's `tools` still decides connectors and the shell. Covered by `core/test/flybot.test.js`, `core/test/connector-access.test.js` and `core/test/run-bot-tools.test.js`. +- A run Bot (#1373) is left out of the channel `state` view, so the sidebar, search and Settings → Bots never list it. `list_bots`, `message_bot`, `stop_bot`, `update_bot` and group member lookup skip it too. `findBot` still sees the name, so a later Bot cannot reuse it. It is not a permissionless shell: `flybot.bot.create` may pass `tools` and `permission` for an ephemeral Bot only. `stepTools` is what connector-access and the approval listener read. `github` grants `mcp__github__*`; a connector the step did not name is hidden and a call is refused (`not granted to this run`). A run Bot is also capped by the Bots whose connector switches apply to it (#1544): `createdBy` names the Bot whose tool call started the run, `cappedBy` names every Bot whose switches cap it (the caller and the Bot that saved the workflow graph), and `connector-access.js` refuses a connector turned off for any of them with `turned off for ; the step's tools list cannot turn it back on`, dropping it from the run Bot's own list as well. A Bot that no longer exists, and a run recorded before this with neither field, leaves the answer as it was: there is no switch list to intersect with. A save with no Bot behind it (the canvas, the API, a template copy) records no author, which is the owner's own edit and lifts that ceiling again. A hidden run Bot cannot show an approval card, so a granted call is `allowed-once` and an ungranted connector or an ungranted full-access shell escalation is `rejected`. **It is not a team member either (#1445):** a node's `tools` covers connectors and the shell, not the team surface, so `EPHEMERAL_DENIED_TOOLS` in `tools.js` names every tool whose effect reaches past the run Bot's own Session — `message_bot`/`stop_bot`/`create_bot`/`update_bot`, groups and posts, `ask_user`, the capability writes, `request_secret`/`list_secrets`, the schedule writes, `send_message`, the social and workflow tools, `read_browser`, `recall` (its default scope reads the team memory) and `create_child_bot`. `connector-access.js` refuses each call with `A running Bot is not a team member.`, `approvals.js` answers `rejected` for the same names (also for a subagent child Session, by following the parent chain through `rt.whoseAgent`), and the run Bot's own list loses them through `restrict({ deny })`; a name the profile serves but the mask cannot enforce stays listed with a warn line, and the guard is what refuses it. `EPHEMERAL_ALLOWED_TOOLS` is the rest: the read-only tools (`list_bots`, `list_models`, `list_capabilities`, `bot_schedules`, `schedule_list`, `read_own_chat`, `read_group_chat`) and `remember`/`forget`, which `writeDenied` keeps inside the run Bot's own scope. Ordinary Bots, friend's Bots and group rooms are unchanged, and `plugins/workflow/**` is not edited: the node's `tools` still decides connectors and the shell. Covered by `core/test/flybot.test.js`, `core/test/connector-access.test.js` and `core/test/run-bot-tools.test.js`. - Group Session models (#289, #1127): `applyModel` does not trust `appliedModel` while a Bot is `modelStale`. A failed model selection in a group Session (from `applyModel` or from `room-participant.js` `open()`) marks the Bot stale; the next apply (model-list change, restart, or the Bot's next turn in a known group Session) selects every Session again and clears the mark once all took it. A settled record (`appliedModel` matches and `modelStale` is unset) still re-selects a group Session whose last request used another provider or model, on that apply and before the Session's next turn; the Bot's own chat is left as recorded. A Session whose route cannot be read is left alone, so a settled Bot is not selected again on every refresh. A fresh part takes the Bot's model, not the previous request's route. Covered by `core/test/models.test.js`, `core/test/room-participant.test.js` and `core/test/fresh-part-model.test.js`. ### Face-to-face room lead (#476) diff --git a/docs/subsystems/plugin-runtime.md b/docs/subsystems/plugin-runtime.md index 446e6e999..f84207e15 100644 --- a/docs/subsystems/plugin-runtime.md +++ b/docs/subsystems/plugin-runtime.md @@ -65,7 +65,7 @@ FlyBot's plugin API surface on top of the patched core: host hooks, client slots ### Run Bots (#1373) -`flybot.bot.create({ ephemeral: true })` makes a run Bot for the calling plugin. The host stores `hidden: true` and `ephemeral: { plugin }` before the first save. The marker is set only there: `fields.set` cannot add it, and a spec cannot name another plugin. `bot.list`, `bot.find`, `flybot/bots` and `bot.created` omit it. `bot.get` still returns it (`hidden`, `systemPrompt`). `bot.remove(id)` deletes it only when `ephemeral.plugin` is the caller; any other Bot throws, and a missing id returns `false`. `CreateBotSpec` also takes `hidden`, `model`, and — for a run Bot only — `tools` and `permission`. `tools` is stored as `stepTools` (`github` grants `mcp__github__*`). `permission` is a DSH preset (`workspace-write` or `danger-full-access`) applied to that Bot's Session; a failed apply deletes the Bot. A team Bot create ignores both. A run Bot has no visible chat, so an approval card would stall it: a granted tool is allowed, a connector it did not name is refused, and a shell escalation to full access is allowed only when `bash`, `shell` or `danger-full-access` is in `stepTools`. Covered by `core/test/flybot.test.js`. +`flybot.bot.create({ ephemeral: true })` makes a run Bot for the calling plugin. The host stores `hidden: true` and `ephemeral: { plugin }` before the first save. The marker is set only there: `fields.set` cannot add it, and a spec cannot name another plugin. `bot.list`, `bot.find`, `flybot/bots` and `bot.created` omit it. `bot.get` still returns it (`hidden`, `systemPrompt`). `bot.remove(id)` deletes it only when `ephemeral.plugin` is the caller; any other Bot throws, and a missing id returns `false`. `CreateBotSpec` also takes `hidden`, `model`, and — for a run Bot only — `tools`, `permission`, `createdBy` and `cappedBy`. `createdBy` is the Bot whose tool call started the run; `cappedBy` lists the Bots whose connector switches cap it (the caller and the Bot that saved the workflow graph), and the host reads each id at call time, so a switch flipped later applies at once and a Bot that is gone stops capping (#1544). `tools` is stored as `stepTools` (`github` grants `mcp__github__*`). `permission` is a DSH preset (`workspace-write` or `danger-full-access`) applied to that Bot's Session; a failed apply deletes the Bot. A team Bot create ignores both. A run Bot has no visible chat, so an approval card would stall it: a granted tool is allowed, a connector it did not name is refused, and a shell escalation to full access is allowed only when `bash`, `shell` or `danger-full-access` is in `stepTools`. Covered by `core/test/flybot.test.js`. ### Chat-row pending badge diff --git a/docs/subsystems/workflow.md b/docs/subsystems/workflow.md index a0baeb606..8e6e51230 100644 --- a/docs/subsystems/workflow.md +++ b/docs/subsystems/workflow.md @@ -60,3 +60,8 @@ Everything in `plugins/workflow/`: the graph rules, the run store, the matcher, ## Tests `plugins/workflow/test/`: graph advance and join, graph edit, assign and persistence, the FIFO queue and concurrency cap, Bot-reply advance, a dispatch that fails without being marked handed, a bounded hand-off retry and a step deadline that fails the run and clears its timer (#1530), disposing releases the Bots before the record drops their ids and a released runner name is reusable again, assign-to-bot versus assign-to-workflow, inputs and outputs along edges, a loop that breaks and an if/else branch, failure recovery (onFail edges with max attempts, exhaustion behavior, and compatibility with failOnce and loop edges), templates, `define` / `runNamed`, the matcher, ephemeral spawn and cleanup, a stopped run with its settled node map (#1424), the reply middleware, the client canvas, the settings page (including a condition and a loop), rename and delete (including factory-id rows such as `delivery`), and a sidebar click that leaves the Workflows section in place. Run `npm test` in `plugins/workflow`. The host seam that applies `ownerBotId` is `core/test/task-route.test.js`. The host omission of run Bots, and the grant of `stepTools` plus the hidden-approval answer, is `core/test/flybot.test.js`. Which connectors a run Bot may call is `core/test/connector-access.test.js`, and the chain from a node's `tools` through `spawn` to that guard — the built-in delivery nodes included — is `core/test/workflow-node-connectors.test.js`. + +## Connector ceiling (#1544) + +A connector a Bot switched off is not reopened by a Workflow, whichever surface started the run. A run records the Bots whose switches cap it: the Bot whose `run_workflow` call started it (`createdBy`), and the Bot that saved the graph (`authoredBy` on the workflow record, set by `define_workflow` and by any save that names a Bot). Every node's run Bot carries `cappedBy` — the caller, else the graph's author — and `core/src/host/connector-access.js` intersects the node's `tools` with those switches, so a node that names a connector its caller or author turned off is refused with `turned off for ; the step's tools list cannot turn it back on`, and the tool is masked out of the run Bot's list too. A run nobody's tool call started (the board's `ingest`, the canvas, the `workflow/*` API) has no caller and is capped by the graph's author alone; a save with no Bot behind it records no author, which is the owner's own edit and leaves no ceiling; a graph saved before this has none either. `visibleTo` on `run_workflow`/`define_workflow` is unchanged: the ceiling is the fix, not limiting who may define a Workflow. + diff --git a/patches/ds-bot/files/src/host/connector-access.js b/patches/ds-bot/files/src/host/connector-access.js index 53b6e0b45..1c1ca4ab1 100644 --- a/patches/ds-bot/files/src/host/connector-access.js +++ b/patches/ds-bot/files/src/host/connector-access.js @@ -53,7 +53,24 @@ export function install(rt) { const tools = bot?.stepTools ?? [] return tools.includes(id) || tools.includes('*') || tools.some(name => name === id || String(name).startsWith(`mcp__${id}__`)) } - const allowed = (bot, id) => bot.remote === undefined && !turnedOff(bot, id) && (bot.ephemeral === undefined || stepGrants(bot, id)) + // #1544: a run Bot must not reopen a connector a Bot switched off. The record names the Bots whose + // switches cap it — the caller that started the run (createdBy), and the Bot that saved the graph + // (cappedBy), which is the ceiling when the board, the canvas or the API started the run. A Bot that + // no longer exists, and a record with neither field (an older run), leave the answer as it was: + // there is no switch list to intersect with. + const capsOf = (bot) => { + const ids = [bot?.createdBy, ...(Array.isArray(bot?.cappedBy) ? bot.cappedBy : [])] + .filter(id => typeof id === 'string' && id !== '') + const caps = [] + for (const id of ids) { + const cap = rt.botOf(id) + if (cap !== undefined && !caps.includes(cap)) caps.push(cap) + } + return caps + } + const capOff = (bot, id) => capsOf(bot).find(cap => turnedOff(cap, id)) + const allowed = (bot, id) => bot.remote === undefined && !turnedOff(bot, id) + && (bot.ephemeral === undefined || (stepGrants(bot, id) && capOff(bot, id) === undefined)) // The Bot an Agent works for: its own chat, a group Session and an earlier part resolve through // selfOf; a sub-agent Session is followed up to its parent. The answer has three shapes, and only @@ -126,7 +143,13 @@ export function install(rt) { const { bot } = who if (allowed(bot, id)) return undefined if (bot.remote !== undefined) return `The ${id} connector is not available to a friend's Bot.` - if (bot.ephemeral !== undefined) return `The ${id} connector is not granted to this run. The step's tools list is what allows it.` + if (bot.ephemeral !== undefined) { + // #1544: the step did name it — a Bot's switch is what refuses it, and naming it again in the + // graph cannot turn it back on. + const cap = capOff(bot, id) + if (cap !== undefined) return `The ${id} connector is turned off for ${cap.name}; the step's tools list cannot turn it back on.` + return `The ${id} connector is not granted to this run. The step's tools list is what allows it.` + } return `The ${id} connector is turned off for ${bot.name}. Its owner can turn it on in this Bot's settings.` }) diff --git a/patches/ds-bot/files/src/host/hooks.js b/patches/ds-bot/files/src/host/hooks.js index d4e7678dc..6d27281e7 100644 --- a/patches/ds-bot/files/src/host/hooks.js +++ b/patches/ds-bot/files/src/host/hooks.js @@ -168,10 +168,26 @@ export function install(rt) { return tools } - async function create({ name, systemPrompt, fields, role, instructions, createdBy, plugin = SHARED, hidden, ephemeral, model, tools, permission } = {}) { + // #1544: the Bots whose connector switches cap an ephemeral Bot (a Workflow run names the Bot that + // started it and the Bot that saved the graph). Only the ids are stored; each is read at call time, + // so a switch flipped later applies at once and a Bot that is gone stops capping. + function cleanCappedBy(value) { + if (!Array.isArray(value)) return [] + const ids = [] + for (const item of value) { + const id = String(item ?? '').trim() + if (id === '' || ids.includes(id)) continue + ids.push(id) + if (ids.length === 8) break + } + return ids + } + + async function create({ name, systemPrompt, fields, role, instructions, createdBy, cappedBy, plugin = SHARED, hidden, ephemeral, model, tools, permission } = {}) { checkName('plugin', plugin) await rt.load() const stepTools = ephemeral === true ? cleanStepTools(tools) : [] + const caps = ephemeral === true ? cleanCappedBy(cappedBy) : [] const preset = ephemeral === true ? String(permission ?? '').trim() : '' const bot = await rt.createBot({ name, role, instructions, createdBy, @@ -184,6 +200,7 @@ export function install(rt) { }) const record = rt.botOf(bot.id) if (record && stepTools.length > 0) record.stepTools = stepTools + if (record && caps.length > 0) record.cappedBy = caps if (record && preset) { try { await rt.setPermission(record, preset) @@ -192,7 +209,7 @@ export function install(rt) { throw error } } - if (record && (stepTools.length > 0 || preset)) await rt.save() + if (record && (stepTools.length > 0 || caps.length > 0 || preset)) await rt.save() for (const [key, value] of Object.entries(fields ?? {})) fire('change', { botId: bot.id, plugin, key, value: clone(value), prev: undefined }) return bot.id } diff --git a/patches/ds-bot/files/test/connector-access.test.js b/patches/ds-bot/files/test/connector-access.test.js index 939e533b6..e7479823a 100644 --- a/patches/ds-bot/files/test/connector-access.test.js +++ b/patches/ds-bot/files/test/connector-access.test.js @@ -572,3 +572,40 @@ test('a run Bot gets a connector only when its step tools name it', async (t) => assert.deepEqual(sorted(hiddenFrom('bare')), sorted([...COMPOSIO, ...GITHUB]), 'no stepTools fails closed') assert.match(guard(GITHUB[0], 'bare'), /not granted to this run/) }) + +// #1544: a Workflow node names the connectors it may call, and since #1535 that names enough to reach +// them. That must not become a way around a switch: a connector turned off for the Bot that started +// the run (or that saved the graph) stays off for the run Bot, and the tool is masked out of its list +// (denied()/restrict()), not only refused when it is called. +test('a run Bot does not reopen a connector the Bot that started the run switched off', async (t) => { + const state = team() + state.bots.writer.connectors = { github: false } + const runBot = (id, { createdBy, cappedBy, stepTools = ['github'] } = {}) => { + state.bots[id] = { + id, sessionId: id, name: id, role: 'Run', instructions: '', createdAt: 4, + hidden: true, ephemeral: { plugin: 'flybot-workflow' }, stepTools, + ...(createdBy === undefined ? {} : { createdBy }), + ...(cappedBy === undefined ? {} : { cappedBy }), + } + } + runBot('caller-off', { createdBy: 'writer' }) + runBot('caller-on', { createdBy: 'chief' }) + runBot('caller-gone', { createdBy: 'removed-bot' }) + runBot('author-off', { cappedBy: ['writer'] }) + runBot('both-on', { createdBy: 'chief', cappedBy: ['chief'] }) + const { agentFor, hiddenFrom, guard, stop } = await setup({ state }) + t.after(stop) + await Promise.all(['caller-off', 'caller-on', 'caller-gone', 'author-off', 'both-on'].map(id => agentFor(id))) + + assert.match(guard(GITHUB[0], 'caller-off'), /turned off for Writer; the step's tools list cannot turn it back on/, 'the switch is the reason, not the step list') + assert.deepEqual(sorted(hiddenFrom('caller-off')), sorted([...COMPOSIO, ...GITHUB]), 'and the tool is out of the run Bot\'s list too') + assert.equal(guard(GITHUB[0], 'caller-on'), undefined, 'a caller that still has github keeps the #1535 answer') + assert.deepEqual(sorted(hiddenFrom('caller-on')), sorted(COMPOSIO)) + // A run nobody's tool call started (the board, the canvas, the API) has no caller; the Bot that + // saved the graph is the ceiling then. + assert.match(guard(GITHUB[0], 'author-off'), /turned off for Writer; the step's tools list cannot turn it back on/) + assert.deepEqual(sorted(hiddenFrom('author-off')), sorted([...COMPOSIO, ...GITHUB])) + assert.equal(guard(GITHUB[0], 'both-on'), undefined) + // A Bot that no longer exists has no switch list to read: the answer stays what it was. + assert.equal(guard(GITHUB[0], 'caller-gone'), undefined) +}) diff --git a/plugins/workflow/index.js b/plugins/workflow/index.js index daa8018f0..db0e4b370 100644 --- a/plugins/workflow/index.js +++ b/plugins/workflow/index.js @@ -52,6 +52,10 @@ export function apply(ctx, config = {}) { ephemeral: true, permission: permissionFor(), tools: spec.tools, + // #1544: who started the run and whose switches cap its connectors; the host reads both + // and refuses a connector a Bot switched off however the node names it. + ...(spec.createdBy ? { createdBy: spec.createdBy } : {}), + ...(spec.cappedBy ? { cappedBy: spec.cappedBy } : {}), fields: { nodeId: spec.nodeId, tools: spec.tools ?? null }, }) liveNames.set(name, id) @@ -118,7 +122,8 @@ export function apply(ctx, config = {}) { brief: { type: 'string', required: true, description: 'What the run should do' }, task: { type: 'string', description: 'Task id, such as #1280. Omitted ids are generated.' }, }, - execute: args => service.runNamed(args ?? {}), + // 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', @@ -129,7 +134,7 @@ export function apply(ctx, config = {}) { nodes: { type: 'array', required: true, description: 'Nodes. Each may carry prompt (system prompt), role, model, tools (the connectors it may call: github, composio), inputs, outputs, and an optional local botId. Real work uses an ephemeral Bot per run, not a seated botId.' }, 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 }.' }, }, - execute: args => service.define(args ?? {}), + execute: (args, ctx) => service.define({ ...(args ?? {}), ...(ctx?.botId ? { callerBotId: ctx.botId } : {}) }), })) } if (typeof fb.bot?.send?.use === 'function') { diff --git a/plugins/workflow/src/ephemeral.js b/plugins/workflow/src/ephemeral.js index 483c3f9c4..d944bde74 100644 --- a/plugins/workflow/src/ephemeral.js +++ b/plugins/workflow/src/ephemeral.js @@ -32,12 +32,25 @@ export function nodeConfig(node, roster = [], workflowModel = '') { } } -export async function spawnAll(nodes, roster, spawn, workflowModel = '') { +// #1544: a connector a Bot switched off is not reopened by a Workflow. The run carries the Bots whose +// switches cap it — the Bot that started the run (a tool call), and the Bot that saved the graph, which +// is the ceiling when the board, the canvas or the API started it. The host intersects the node's +// `tools` with them; the graph cannot raise its own ceiling. +export function runCaps(callerBotId, authorBotId) { + const ids = [callerBotId, authorBotId].filter(id => typeof id === 'string' && id !== '') + return [...new Set(ids)] +} + +export async function spawnAll(nodes, roster, spawn, workflowModel = '', caps = {}) { const made = [] + const capping = runCaps(caps.callerBotId, caps.authorBotId) + const caller = typeof caps.callerBotId === 'string' && caps.callerBotId !== '' ? caps.callerBotId : undefined try { const ephemeral = {} for (const node of nodes) { const spec = nodeConfig(node, roster, workflowModel) + if (caller !== undefined) spec.createdBy = caller + if (capping.length > 0) spec.cappedBy = capping const id = await spawn(spec) if (typeof id !== 'string' || id === '') throw new Error('The run could not start a Bot') made.push(id) diff --git a/plugins/workflow/src/service.js b/plugins/workflow/src/service.js index 45f7109e3..bee48eda5 100644 --- a/plugins/workflow/src/service.js +++ b/plugins/workflow/src/service.js @@ -573,7 +573,7 @@ export function createWorkflowService({ } if (!best) return null const task = laneOf(best.item.id).queue.shift() - return { item: best.item, task: { id: task.id, title: task.title } } + return { item: best.item, task: { id: task.id, title: task.title, ...(task.callerBotId ? { callerBotId: task.callerBotId } : {}) } } } function noteUnstarted(item, error) { @@ -662,7 +662,12 @@ export function createWorkflowService({ const holes = missingProviders(item) if (holes.length > 0) return { ok: false, error: `Node ${holes[0].nodeId} input ${holes[0].name} has no upstream provider` } try { - ephemeral = await spawnAll(item.nodes, bots() ?? [], spawn, item.model ?? '') + // #1544: the caller that started the run and the Bot that saved the graph cap the connectors + // this run may use; the host intersects them with each node's `tools`. + ephemeral = await spawnAll(item.nodes, bots() ?? [], spawn, item.model ?? '', { + callerBotId: task.callerBotId, + authorBotId: item.authoredBy, + }) } catch (error) { await releaseAll(error.spawned ?? [], release, warn) return refuse(error) @@ -690,7 +695,14 @@ export function createWorkflowService({ function park(item, task) { const lane = laneOf(item.id) state.queueSeq = (state.queueSeq ?? 0) + 1 - lane.queue.push({ id: task.id, title: task.title, enqueuedAt: now(), queueSeq: state.queueSeq }) + lane.queue.push({ + id: task.id, + title: task.title, + enqueuedAt: now(), + queueSeq: state.queueSeq, + // #1544: a parked run starts later without its caller in hand; keep it, or the ceiling is lost. + ...(task.callerBotId ? { callerBotId: task.callerBotId } : {}), + }) push(item.id, [{ kind: 'system', text: `排队 · ${summarizeTask(task)}(前面 ${lane.queue.length - 1} 个)` }], now()) write() } @@ -865,6 +877,9 @@ export function createWorkflowService({ ...(Array.isArray(body.edges) ? { edges: body.edges } : { layers: body.layers }), }) if (!item.name) throw new Error('A workflow needs a name') + // #1544: the Bot that wrote the graph caps the connectors a run may reach from it. + const authoredBy = cleanLine(body.callerBotId ?? body.authoredBy, 80) || undefined + if (authoredBy) item.authoredBy = authoredBy const cap = Number(body.maxConcurrent) if (Number.isInteger(cap) && cap >= 1 && cap <= MAX_CONCURRENT) item.maxConcurrent = cap state.workflows.push(item) @@ -887,6 +902,10 @@ export function createWorkflowService({ if (!item) return { ok: false, error: 'No such workflow' } const task = { id: cleanLine(body.taskId ?? body.id, 80), title: cleanLine(body.title, 300) } if (!task.id || !task.title) return { ok: false, error: 'A task needs an id and a title' } + // #1544: the Bot whose tool call started this run, when a Bot started it. The board, the canvas + // and the API pass none, and the graph's author is the ceiling there. + const callerBotId = cleanLine(body.callerBotId, 80) + if (callerBotId) task.callerBotId = callerBotId if (typeof spawn !== 'function') return enqueue(item, task) return hostSerial(() => enqueue(item, task)) }, @@ -1090,6 +1109,11 @@ export function createWorkflowService({ // persist; a node that remains keeps its run status, a new one waits. saveGraph(body = {}) { return mutate(String(body.workflowId ?? ''), (item) => { + // #1544: a save from a Bot records it as the graph's author; a save with no Bot behind it (the + // canvas, the API, a template copy) is the owner's own edit and lifts the ceiling again. + const authoredBy = cleanLine(body.callerBotId ?? body.authoredBy, 80) || undefined + if (authoredBy) item.authoredBy = authoredBy + else delete item.authoredBy const seated = seat(body.nodes, bots() ?? []) const seenBots = new Set() for (const node of seated) { @@ -1121,9 +1145,12 @@ export function createWorkflowService({ }, // A Bot (or the owner) writes a graph. A name that already exists is replaced. define(body = {}) { + // #1544: the graph remembers the Bot that wrote it, so a run nobody's tool call started is still + // capped by the switches of the Bot that chose the node's `tools`. + const authoredBy = cleanLine(body.callerBotId, 80) || undefined const existing = body.workflow ? findWorkflow(body.workflow) : undefined if (existing) { - const saved = this.saveGraph({ workflowId: existing.id, nodes: body.nodes, edges: body.edges ?? [] }) + const saved = this.saveGraph({ workflowId: existing.id, nodes: body.nodes, edges: body.edges ?? [], ...(authoredBy ? { authoredBy } : {}) }) if (saved.ok === false) return saved const name = cleanLine(body.name, 80) if (name) { @@ -1132,7 +1159,7 @@ export function createWorkflowService({ } return present() } - return this.create({ name: body.name, nodes: body.nodes, edges: body.edges ?? [], description: body.description }) + return this.create({ name: body.name, nodes: body.nodes, edges: body.edges ?? [], description: body.description, ...(authoredBy ? { authoredBy } : {}) }) }, // Start a named workflow. `brief` is the task title; `task` is its id. runNamed(body = {}) { @@ -1141,7 +1168,7 @@ export function createWorkflowService({ const title = cleanLine(body.brief || body.title, 300) if (!title) return { ok: false, error: 'A run needs a brief' } const taskId = cleanLine(body.task || body.taskId, 80) || `task-${now().toString(36)}` - return this.assign({ workflowId: item.id, taskId, title }) + return this.assign({ workflowId: item.id, taskId, title, ...(body.callerBotId ? { callerBotId: body.callerBotId } : {}) }) }, // Auto-assign an open board task. A miss leaves the task untouched. ingest(task = {}) { diff --git a/plugins/workflow/test/ephemeral.test.js b/plugins/workflow/test/ephemeral.test.js index 7144a97dc..88b327447 100644 --- a/plugins/workflow/test/ephemeral.test.js +++ b/plugins/workflow/test/ephemeral.test.js @@ -763,3 +763,111 @@ test('the built-in delivery nodes hand their connector list to the host', async assert.deepEqual(byNode.get('planner').tools, [], 'Planner names no connector') for (const spec of created) assert.equal(spec.permission, 'danger-full-access', 'file and shell rights are a normal Bot\'s') }) + +// #1544: a connector a Bot switched off must not be reopened by a Workflow. The run records the Bot +// that started it (createdBy) and every Bot whose switches cap its connectors (cappedBy): the caller +// when a Bot's tool call started the run, and the Bot that saved the graph when the board, the canvas +// or the API did. `core/src/host/connector-access.js` intersects the node's `tools` with them. +test('#1544: the run Bot records the Bot that started the run, and the Bot that saved the graph caps it', async () => { + const { svc, spawned } = harness() + svc.define({ + name: 'Launder', + nodes: [{ id: 'code', name: 'Coder', prompt: 'Code.', tools: ['github'] }], + edges: [], + callerBotId: 'bot-planner', + }) + const started = await svc.runNamed({ workflow: 'Launder', brief: 'Ship the note', callerBotId: 'main' }) + assert.equal(started.ok, true) + const spec = spawned.at(-1).spec + assert.equal(spec.createdBy, 'main') + assert.deepEqual(spec.cappedBy, ['main', 'bot-planner']) + assert.deepEqual(spec.tools, ['github']) +}) + +test('#1544: a run with no calling Bot is capped by the Bot that saved the graph', async () => { + const { svc, spawned } = harness() + const made = svc.define({ + name: 'Board lane', + nodes: [{ id: 'code', name: 'Coder', prompt: 'Code.', tools: ['github'] }], + edges: [], + callerBotId: 'bot-planner', + }).workflows.find(item => item.name === 'Board lane') + // The board (ingest) and the canvas have no calling Bot; the graph's author is the ceiling then. + const started = await svc.assign({ workflowId: made.id, taskId: '#1544', title: 'From the board' }) + assert.equal(started.ok, true) + const spec = spawned.at(-1).spec + assert.equal('createdBy' in spec, false, 'no Bot started this run') + assert.deepEqual(spec.cappedBy, ['bot-planner']) + // A save with no Bot behind it is the owner's own edit: it lifts the ceiling again. + svc.saveGraph({ workflowId: made.id, nodes: made.nodes, edges: made.edges }) + await svc.reset({ workflowId: made.id, runId: started.workflows.find(item => item.id === made.id).run.id }) + await svc.assign({ workflowId: made.id, taskId: '#1544b', title: 'After the owner edit' }) + assert.equal('cappedBy' in spawned.at(-1).spec, false, 'the owner own edit leaves no Bot ceiling') + // The board's own path: a trigger match auto-starts the run from ingest, with no caller in hand. + const authored = svc.create({ + name: 'Trigger lane', + nodes: [{ id: 'code', name: 'Coder', prompt: 'Code.', tools: ['github'] }], + edges: [], + triggers: ['wash'], + callerBotId: 'bot-planner', + }).workflows.find(item => item.name === 'Trigger lane') + const ingested = await svc.ingest({ id: '#1544c', title: 'please wash this', status: 'open' }) + assert.equal(ingested.matched, true) + const boardRun = ingested.workflows.find(item => item.id === authored.id) + assert.equal(boardRun.run.task.id, '#1544c') + assert.deepEqual(spawned.at(-1).spec.cappedBy, ['bot-planner'], 'the graph author caps a run the board started') +}) + +test('#1544: a parked run keeps the caller and the author across the queue', async () => { + const { svc, spawned } = harness() + const made = svc.define({ + name: 'Parked', + nodes: [{ id: 'code', name: 'Coder', prompt: 'Code.', tools: ['github'] }], + edges: [], + callerBotId: 'bot-planner', + }).workflows.find(item => item.name === 'Parked') + svc.setConcurrency({ workflowId: made.id, maxConcurrent: 1 }) + const first = await svc.assign({ workflowId: made.id, taskId: '#a', title: 'First' }) + const parked = await svc.runNamed({ workflow: 'Parked', brief: 'Second', callerBotId: 'main' }) + assert.equal(parked.workflows.find(item => item.id === made.id).queueDepth, 1) + const before = spawned.length + await svc.reset({ workflowId: made.id, runId: first.workflows.find(item => item.id === made.id).run.id }) + const drained = spawned.slice(before).map(entry => entry.spec) + assert.deepEqual(drained.map(spec => spec.createdBy), ['main']) + assert.deepEqual(drained.map(spec => spec.cappedBy), [['main', 'bot-planner']]) +}) + +test('#1544: the workflow tools hand the calling Bot on to the run they start', async () => { + const boot = () => { + const specs = {} + const created = [] + async function send() { return { action: 'allow' } } + send.use = () => () => {} + send.priority = { ROUTING: 10 } + const fb = { + bot: { + list: () => roster, + async create(spec) { created.push(spec); return `bot-${created.length}` }, + async remove() { return true }, + send, + }, + readOnly: () => false, + api: { handle() { return () => {} } }, + tools: { register(spec) { specs[spec.name] = spec; return () => {} } }, + warn() {}, + } + const started = apply({ flybot: { scope: () => fb }, effect(fn) { fn() } }, { persist: false }) + return { started, specs, created } + } + const one = boot() + // The host wraps fb.tools.register's execute and passes the caller as the second argument's botId. + await one.specs.define_workflow.execute({ + name: 'Caller lane', + nodes: [{ id: 'code', name: 'Coder', prompt: 'Code.', tools: ['github'] }], + edges: [], + }, { botId: 'main' }) + await one.specs.run_workflow.execute({ workflow: 'Caller lane', brief: 'Ship the note' }, { botId: 'bot-planner' }) + const spawnedSpec = one.created.at(-1) + assert.equal(spawnedSpec.createdBy, 'bot-planner') + assert.deepEqual(spawnedSpec.cappedBy, ['bot-planner', 'main']) +})