fix(workflow): a run Bot takes the profile model, not the model of the Bot that started the run

changelog.d/1582.fixed.md

@@ -0,0 +1 @@
+A Workflow node that names no model now runs on the profile default whichever surface started the run, so the same graph no longer changes model, cost and tool capability with whoever pressed start; the connector ceiling and the refusal of a run started by a friend's Bot still hold ([#1582](https://github.com/ThinkFlowLab/FlyBot/issues/1582)).

core/src/host/connector-access.js

@@ -54,11 +54,11 @@ export function install(rt) {
     const tools = bot?.stepTools ?? []
     return tools.includes(id) || tools.includes('*') || tools.some(name => name === id || String(name).startsWith(`mcp__${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.
+  // #1544: a run Bot must not reopen a connector a Bot switched off. `cappedBy` names the Bots whose
+  // switches cap it — the caller that started the run, and the Bot that saved the graph, which is the
+  // ceiling when the board, the canvas or the API started the run. `createdBy` still counts for a Bot
+  // made by another Bot (and for a run record written before #1582). 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.
   const capsOf = (bot) => {
     const ids = [bot?.createdBy, ...(Array.isArray(bot?.cappedBy) ? bot.cappedBy : [])]
       .filter(id => typeof id === 'string' && id !== '')
@@ -71,14 +71,12 @@ export function install(rt) {
   }
   const capOff = (bot, id) => capsOf(bot).find(cap => turnedOff(cap, id))
   // #1555: `run_workflow`/`define_workflow` have no `visibleTo`, so a friend's Bot can call them and a run
-  // Bot can name it as the one that started the run (`createdBy`). A remote Bot has no switch table of ours
-  // to intersect with, so it reads as "every connector switched off": the node's `tools` cannot hand the run
-  // Bot a connector. A record with no `createdBy` (the owner, the board, an older run) keeps the answer it
-  // had, and a `createdBy` that names a Bot we do not have leaves it as it was too.
-  const remoteCaller = (bot) => {
-    const id = bot?.createdBy
-    return (typeof id === 'string' && id !== '' ? rt.botOf(id) : undefined)?.remote !== undefined
-  }
+  // Bot can name it among the Bots whose switches cap the run (`cappedBy`; a record written before #1582
+  // named it in `createdBy`). A remote Bot has no switch table of ours to intersect with, so it reads as
+  // "every connector switched off": the node's `tools` cannot hand the run Bot a connector. A run with no
+  // cap (the owner, the board, an older run) keeps the answer it had, and a cap that names a Bot we do not
+  // have leaves it as it was too.
+  const remoteCaller = (bot) => capsOf(bot).some(cap => cap.remote !== undefined)
   const allowed = (bot, id) => bot.remote === undefined && !turnedOff(bot, id)
     && (bot.ephemeral === undefined || (stepGrants(bot, id) && capOff(bot, id) === undefined && !remoteCaller(bot)))
 

core/src/host/roster.js

@@ -77,7 +77,10 @@ export function install(rt) {
     // maker's effective model and thinking level (#410; the level resolved against that
     // model, as applyModel does for the maker), set before its first request, so it never
     // tries an unconfigured profile default first. A friend's Bot is never a maker here.
-    const maker = !model && !pickModel && !isRemote && createdBy ? rt.botOf(createdBy) : undefined
+    // A run Bot (#1582) is not a Bot's child either: `createdBy` on one names the caller whose
+    // connector switches cap it, not a maker, so it starts on the profile default like the board,
+    // the canvas and the API do. Its model must not change with whoever pressed start.
+    const maker = !model && !pickModel && !isRemote && !ephemeral && createdBy ? rt.botOf(createdBy) : undefined
     // A brief is a parent's context (`create_bot`), so it becomes instructions only when
     // another Bot made this one. A line the user typed in the new-Bot card is their first
     // request: the client sends it as a turn, and it is never saved as a standing rule (#645).

core/test/connector-access.test.js

@@ -588,21 +588,25 @@ test('a run Bot does not reopen a connector the Bot that started the run switche
       ...(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'] })
+  // #1582: the caller now travels in `cappedBy` with the graph's author, so the ceiling reads one
+  // field. A run record written before it named the caller in `createdBy` still gets the same answer.
+  runBot('caller-off', { cappedBy: ['writer'] })
+  runBot('caller-on', { cappedBy: ['chief'] })
+  runBot('caller-gone', { cappedBy: ['removed-bot'] })
+  runBot('author-off', { cappedBy: ['chief', 'writer'] })
+  runBot('both-on', { cappedBy: ['chief'] })
+  runBot('legacy-caller-off', { createdBy: 'writer' })
   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)))
+  await Promise.all(['caller-off', 'caller-on', 'caller-gone', 'author-off', 'both-on', 'legacy-caller-off'].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.match(guard(GITHUB[0], 'legacy-caller-off'), /turned off for Writer; the step's tools list cannot turn it back on/, 'a pre-#1582 record keeps its ceiling')
   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.
+  // saved the graph is the ceiling then. Both ids are caps in the same list.
   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)
@@ -611,29 +615,33 @@ test('a run Bot does not reopen a connector the Bot that started the run switche
 })
 
 // #1555: `run_workflow` / `define_workflow` have no `visibleTo`, so a friend's Bot can call them. A run
-// Bot then names that remote Bot as the one that started it, and a remote Bot has no switch table of ours
-// to intersect with — so the caller reads as "every connector switched off", and the step list cannot hand
-// the run Bot anything. Nothing changes for a team Bot: its own switch and the node's `tools` still decide.
+// Bot then names that remote Bot among the Bots that cap it (`cappedBy`, #1582; `createdBy` before it),
+// and a remote Bot has no switch table of ours to intersect with — so the cap reads as "every connector
+// switched off", and the step list cannot hand the run Bot anything. Nothing changes for a team Bot: its
+// own switch and the node's `tools` still decide.
 test('a run Bot started by a friend\'s Bot reaches no connector', async (t) => {
   const state = team()
   state.bots.friend = { id: 'friend', sessionId: 'friend', name: 'Bob', role: 'Bot', instructions: '', createdAt: 5, remote: { handle: '@bob.home' } }
-  const run = (id, { createdBy, stepTools = ['github'] } = {}) => {
+  const run = (id, { createdBy, cappedBy, stepTools = ['github'] } = {}) => {
     state.bots[id] = {
       id, sessionId: id, name: id, role: 'Run', instructions: '', createdAt: 6,
       hidden: true, ephemeral: { plugin: 'flybot-workflow' }, stepTools,
       ...(createdBy === undefined ? {} : { createdBy }),
+      ...(cappedBy === undefined ? {} : { cappedBy }),
     }
   }
-  run('remote-run', { createdBy: 'friend' })
-  run('team-run', { createdBy: 'chief' })
+  run('remote-run', { cappedBy: ['friend'] })
+  run('team-run', { cappedBy: ['chief'] })
+  run('legacy-remote-run', { createdBy: 'friend' })
   const { agentFor, hiddenFrom, guard, stop } = await setup({ state })
   t.after(stop)
-  await Promise.all(['remote-run', 'team-run'].map(id => agentFor(id)))
+  await Promise.all(['remote-run', 'team-run', 'legacy-remote-run'].map(id => agentFor(id)))
 
   // Red before this change: the node names github, so the step list alone let the run Bot through.
   assert.match(guard(GITHUB[0], 'remote-run'), /started by a friend's Bot/, 'the caller is the reason, not the step list')
   assert.deepEqual(sorted(hiddenFrom('remote-run')), sorted([...COMPOSIO, ...GITHUB]), 'and both connectors are out of its list too')
   assert.match(guard(COMPOSIO[0], 'remote-run'), /started by a friend's Bot/)
+  assert.match(guard(GITHUB[0], 'legacy-remote-run'), /started by a friend's Bot/, 'a pre-#1582 record keeps the refusal')
   // The no-over-refusal case: a team Bot started this run, its switch is on, the node names github.
   assert.equal(guard(GITHUB[0], 'team-run'), undefined, 'a team caller keeps what the node names')
   assert.deepEqual(sorted(hiddenFrom('team-run')), sorted(COMPOSIO), 'only the connector the node did not name stays out')

core/test/runbot-model.test.js

@@ -0,0 +1,79 @@
+// #1582: a run Bot must not take the model of the Bot that started the run. `createdBy` is how the
+// roster says "a Bot made this Bot, so with no model named it starts on its maker's model" (#410);
+// a run Bot is not a Bot's child, so the field must never reach that rule. The profile default is
+// what the board, the canvas and the API already use for a node that names no model.
+import assert from 'node:assert/strict'
+import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { test } from 'node:test'
+import { createRuntime, provideFlyBot } from '../index.js'
+import { fakeContext } from './fake-ctx.js'
+
+const team = () => ({
+  version: 1, mainBotIds: ['a'],
+  bots: { a: { id: 'a', name: 'Chief', role: 'Boss', sessionId: 'a', createdAt: 1, model: 'deepseek/deepseek-v4-pro' } },
+  rooms: {}, agents: {}, exchanges: [], exchangeTurns: {}, questions: {}, prefs: {}, revision: 0,
+})
+
+function start(teamState, { models = [] } = {}) {
+  const home = mkdtempSync(join(tmpdir(), 'ds-bot-runbot-model-'))
+  mkdirSync(home, { recursive: true })
+  writeFileSync(join(home, 'state.json'), JSON.stringify(teamState))
+  const ctx = fakeContext()
+  const sent = []
+  let sessions = 0
+  ctx.logger.warn = () => {}
+  Object.assign(ctx, {
+    llm: {
+      listProviders: () => (models.length > 0 ? [{ id: 'deepseek', name: 'DeepSeek' }] : []),
+      listModels: async () => models.map(id => ({ id })),
+      resolveModelInfo: async () => ({}),
+    },
+    workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} },
+    sessionController: {
+      create: async () => ({ sessionId: `s${++sessions}` }),
+      resolveAgent: async sessionId => ({ agent: { session: { id: sessionId }, followup: m => sent.push({ sessionId, message: m }) } }),
+      selectModel: async () => {},
+    },
+    sessionTitle: { rename: async () => {} },
+    sessions: { flush: async () => {} },
+  })
+  let route
+  const register = ctx.connection.fetch.register
+  ctx.connection.fetch.register = (options) => {
+    if (options.path.endsWith('bot')) route = options
+    return register(options)
+  }
+  const rt = createRuntime(ctx, { home })
+  provideFlyBot(ctx, rt.flybot)
+  const call = async (endpoint, payload) => (await route.fetch(new Request('http://localhost/api/bot', { method: 'POST', body: JSON.stringify({ endpoint, payload }) }))).json()
+  const stop = async () => { await ctx.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) }
+  return { rt, ctx, stop, call }
+}
+
+const settle = () => new Promise(resolve => setTimeout(resolve, 30))
+
+test('#1582: a run Bot with a caller and no model takes the profile default, not the caller model', async (t) => {
+  const { rt, ctx, stop, call } = start(team(), { models: ['deepseek-v4-flash', 'deepseek-v4-pro'] })
+  t.after(stop)
+  const teamView = (await call('state', {})).value
+  const chiefId = teamView.mainBotId ?? 'a'
+  await call('update-bot', { id: chiefId, model: 'deepseek/deepseek-v4-pro' })
+  await new Promise(resolve => setTimeout(resolve, 300))
+  assert.equal(await rt.desiredModel(rt.botOf('a')), 'deepseek/deepseek-v4-pro', 'the caller really is on the other model')
+  assert.equal(await rt.defaultModel(), 'deepseek/deepseek-v4-flash')
+  await settle()
+
+  const createRunBot = async (name, extra) => ctx.flybot.bot.create({
+    name, role: 'Run', instructions: 'Work.', hidden: true,
+    ephemeral: true, tools: ['github'],
+    ...extra,
+  })
+  const withCaller = await createRunBot('Probe WithCaller', { createdBy: 'a' })
+  const without = await createRunBot('Probe NoCaller', {})
+  const explicit = await createRunBot('Probe Explicit', { createdBy: 'a', model: 'deepseek/deepseek-v4-flash' })
+  assert.equal(rt.botOf(withCaller)?.model, 'deepseek/deepseek-v4-flash', 'a run Bot must not take the caller model')
+  assert.equal(rt.botOf(without)?.model, 'deepseek/deepseek-v4-flash')
+  assert.equal(rt.botOf(explicit)?.model, 'deepseek/deepseek-v4-flash', 'an explicit model still wins')
+})