diff --git a/core/src/host/connector-access.js b/core/src/host/connector-access.js index 53b6e0b45..6f39e7345 100644 --- a/core/src/host/connector-access.js +++ b/core/src/host/connector-access.js @@ -126,7 +126,14 @@ 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.` + // #1543: a run Bot's own switch is the honest reason when the step did name the connector. The + // settings sentence below is wrong for it — a run Bot is hidden, so its owner has no page to flip + // the switch on — and the tools list cannot either; say both instead of blaming the graph. + if (bot.ephemeral !== undefined) { + return turnedOff(bot, id) + ? `The ${id} connector is turned off for ${bot.name}. A step's tools list can narrow what a run may use, but it cannot turn a connector back on.` + : `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/test/connector-access.test.js b/core/test/connector-access.test.js index 939e533b6..8f7f405fd 100644 --- a/core/test/connector-access.test.js +++ b/core/test/connector-access.test.js @@ -572,3 +572,24 @@ 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/) }) + +// #1543: when a run Bot's own connector switch is off, the refusal must name that switch. The step's +// tools list did name the connector, so blaming it sends the reader to fix the graph instead of the +// toggle. The sentence also has to say the tools list cannot turn it back on — a run Bot is +// hidden: true and the owner cannot find it on the settings page to flip the switch there. +test('a run Bot names the switch that turned a connector off, not the step tools', async (t) => { + const state = team() + state.bots.switched = { + id: 'switched', sessionId: 'switched', name: 'Coder Off', role: 'Run', instructions: '', createdAt: 4, + hidden: true, ephemeral: { plugin: 'flybot-workflow' }, stepTools: ['github'], connectors: { github: false }, + } + const { agentFor, guard, stop } = await setup({ state }) + t.after(stop) + await agentFor('switched') + const reason = guard(GITHUB[0], 'switched') + assert.ok(reason, 'the call is still refused: this fixes the reason, not the decision') + assert.match(reason, /The github connector is turned off for Coder Off\./, 'the Bot whose switch is off is named') + assert.match(reason, /A step's tools list can narrow what a run may use, but it cannot turn a connector back on\./) + assert.doesNotMatch(reason, /not granted to this run/, 'the step tools did name it, so that is not the reason') + assert.doesNotMatch(reason, /settings/, 'a run Bot is hidden from the settings page, so do not send the reader there') +}) diff --git a/core/test/workflow-node-connectors.test.js b/core/test/workflow-node-connectors.test.js index 7fd3f4dd4..742a4e278 100644 --- a/core/test/workflow-node-connectors.test.js +++ b/core/test/workflow-node-connectors.test.js @@ -168,6 +168,7 @@ test('a node Bot still keeps full file/shell rights and the team tools stay refu ctx.emit('tools/change') await new Promise(resolve => setImmediate(resolve)) await new Promise(resolve => setImmediate(resolve)) - assert.match(guardFor(id, GITHUB[0]), /not granted to this run/) + assert.match(guardFor(id, GITHUB[0]), /The github connector is turned off for Runner ab\./) + assert.match(guardFor(id, GITHUB[0]), /cannot turn a connector back on/) assert.deepEqual(listedNow(), [], 'both connectors are hidden once the Bot switch is off') }) diff --git a/patches/ds-bot/files/src/host/connector-access.js b/patches/ds-bot/files/src/host/connector-access.js index 53b6e0b45..6f39e7345 100644 --- a/patches/ds-bot/files/src/host/connector-access.js +++ b/patches/ds-bot/files/src/host/connector-access.js @@ -126,7 +126,14 @@ 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.` + // #1543: a run Bot's own switch is the honest reason when the step did name the connector. The + // settings sentence below is wrong for it — a run Bot is hidden, so its owner has no page to flip + // the switch on — and the tools list cannot either; say both instead of blaming the graph. + if (bot.ephemeral !== undefined) { + return turnedOff(bot, id) + ? `The ${id} connector is turned off for ${bot.name}. A step's tools list can narrow what a run may use, but it cannot turn a connector back on.` + : `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/test/connector-access.test.js b/patches/ds-bot/files/test/connector-access.test.js index 939e533b6..8f7f405fd 100644 --- a/patches/ds-bot/files/test/connector-access.test.js +++ b/patches/ds-bot/files/test/connector-access.test.js @@ -572,3 +572,24 @@ 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/) }) + +// #1543: when a run Bot's own connector switch is off, the refusal must name that switch. The step's +// tools list did name the connector, so blaming it sends the reader to fix the graph instead of the +// toggle. The sentence also has to say the tools list cannot turn it back on — a run Bot is +// hidden: true and the owner cannot find it on the settings page to flip the switch there. +test('a run Bot names the switch that turned a connector off, not the step tools', async (t) => { + const state = team() + state.bots.switched = { + id: 'switched', sessionId: 'switched', name: 'Coder Off', role: 'Run', instructions: '', createdAt: 4, + hidden: true, ephemeral: { plugin: 'flybot-workflow' }, stepTools: ['github'], connectors: { github: false }, + } + const { agentFor, guard, stop } = await setup({ state }) + t.after(stop) + await agentFor('switched') + const reason = guard(GITHUB[0], 'switched') + assert.ok(reason, 'the call is still refused: this fixes the reason, not the decision') + assert.match(reason, /The github connector is turned off for Coder Off\./, 'the Bot whose switch is off is named') + assert.match(reason, /A step's tools list can narrow what a run may use, but it cannot turn a connector back on\./) + assert.doesNotMatch(reason, /not granted to this run/, 'the step tools did name it, so that is not the reason') + assert.doesNotMatch(reason, /settings/, 'a run Bot is hidden from the settings page, so do not send the reader there') +}) diff --git a/patches/ds-bot/files/test/workflow-node-connectors.test.js b/patches/ds-bot/files/test/workflow-node-connectors.test.js index 7fd3f4dd4..742a4e278 100644 --- a/patches/ds-bot/files/test/workflow-node-connectors.test.js +++ b/patches/ds-bot/files/test/workflow-node-connectors.test.js @@ -168,6 +168,7 @@ test('a node Bot still keeps full file/shell rights and the team tools stay refu ctx.emit('tools/change') await new Promise(resolve => setImmediate(resolve)) await new Promise(resolve => setImmediate(resolve)) - assert.match(guardFor(id, GITHUB[0]), /not granted to this run/) + assert.match(guardFor(id, GITHUB[0]), /The github connector is turned off for Runner ab\./) + assert.match(guardFor(id, GITHUB[0]), /cannot turn a connector back on/) assert.deepEqual(listedNow(), [], 'both connectors are hidden once the Bot switch is off') })