Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion core/src/host/connector-access.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.`
})

Expand Down
21 changes: 21 additions & 0 deletions core/test/connector-access.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
3 changes: 2 additions & 1 deletion core/test/workflow-node-connectors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
9 changes: 8 additions & 1 deletion patches/ds-bot/files/src/host/connector-access.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.`
})

Expand Down
21 changes: 21 additions & 0 deletions patches/ds-bot/files/test/connector-access.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
3 changes: 2 additions & 1 deletion patches/ds-bot/files/test/workflow-node-connectors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
Loading