diff --git a/changelog.d/refusals-are-error-results.fixed.md b/changelog.d/refusals-are-error-results.fixed.md new file mode 100644 index 000000000..3de0497e5 --- /dev/null +++ b/changelog.d/refusals-are-error-results.fixed.md @@ -0,0 +1 @@ +- A refusal now reaches the model as an error result (`isError: true`), so it can tell a refusal from a tool that ran. That covers `tune_out`'s refusals (an unknown channel, a cancel of a channel that isn't tuned out, and an enter that can't go through, such as on a channel already tuned out), `channel_open` on a tuned-out channel, the subconscious's refusals of a `deliver_summary` with no text, a `cancel_tuneout` of a channel that isn't tuned out, a `note_disposition` with no key and a `speak_in_channel` it can't make, and the api module's `Unknown tool`. Each came back unflagged (`isError: false`, or for the api module no `isError` at all), so the model read it as a tool's ordinary answer, in XML tool mode as its ``. `channel_open`'s refusal also showed the model only its `data`, `{"refusal":"tuned-out","channelId":"…"}`, and never the text saying to cancel the tune-out with `tune_out`'s `cancel` mode; the model now reads that text. A script's call to one of these tools now gets `Error: `, as for any failed call. An in-process caller of `channel_open` (`ctx.callTool`) or of the api module gets the same result as before with `isError` set, and the tuned-out refusal keeps its `data`. diff --git a/src/framework.ts b/src/framework.ts index 013277a59..823492835 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -18277,12 +18277,12 @@ export class AgentFramework { .find((e) => e.descriptor.id === channelId); let result: { success: boolean; data?: unknown; error?: string; isError?: boolean }; if (!entry) { - result = { success: false, error: `unknown channel: ${channelId}`, isError: false }; + result = { success: false, error: `unknown channel: ${channelId}`, isError: true }; } else if (input.mode === 'cancel') { const r = coordinator.cancel(entry.serverId, channelId, 'agent-tool', 'agent request'); result = r.ok ? { success: true, data: { cancelled: channelId } } - : { success: false, error: r.error, isError: false }; + : { success: false, error: r.error, isError: true }; } else { const r = coordinator.enter(entry.serverId, channelId, { cadenceSeconds: input.cadenceSeconds, @@ -18300,7 +18300,7 @@ export class AgentFramework { maxWakes: r.params.maxWakes, }, } - : { success: false, error: r.error, isError: false }; + : { success: false, error: r.error, isError: true }; } this.pushEvent({ type: 'tool-result', diff --git a/src/mcpl/channel-registry.ts b/src/mcpl/channel-registry.ts index 082e6ed9b..a66be48ef 100644 --- a/src/mcpl/channel-registry.ts +++ b/src/mcpl/channel-registry.ts @@ -2670,11 +2670,13 @@ export class ChannelRegistry { // A tuned-out channel is transport-open but attention-diverted; a plain // open would silently discard the epoch (and its pending backlog dump). // Cancelling is the tune-out coordinator's flow, reached via its own - // tool — refuse with the pointer rather than eat the state. + // tool — refuse with the pointer rather than eat the state. As an error + // result, the agent reads the pointer itself; `data` names the refusal + // for an in-process caller. if (this.getDesiredState(entry.serverId, input.channelId) === 'tuned-out') { return { success: false, - isError: false, + isError: true, error: `Channel ${input.channelId} is tuned out. Cancel the tune-out ` + `(tune_out with mode: 'cancel') to resume normal attention — ` + diff --git a/src/modules/api/index.ts b/src/modules/api/index.ts index 9e844a8d9..9d9543a3d 100644 --- a/src/modules/api/index.ts +++ b/src/modules/api/index.ts @@ -40,7 +40,7 @@ export class ApiModule implements Module { async handleToolCall(_call: ToolCall): Promise { // API module has no tools - return { success: false, error: 'Unknown tool' }; + return { success: false, error: 'Unknown tool', isError: true }; } async onProcess(event: ProcessEvent, _state: ProcessState): Promise { diff --git a/src/tune-out/coordinator.ts b/src/tune-out/coordinator.ts index dbfee7013..cbf6ad2f1 100644 --- a/src/tune-out/coordinator.ts +++ b/src/tune-out/coordinator.ts @@ -485,7 +485,7 @@ export class TuneOutCoordinator { case 'deliver_summary': { const text = String(input.text ?? '').trim(); const channelId = String(input.channelId ?? ''); - if (!text) return { success: false, error: 'empty summary', isError: false }; + if (!text) return { success: false, error: 'empty summary', isError: true }; const subName = this.hooks.subconsciousName(); if (!subName) return { success: false, error: 'subconscious not configured', isError: true }; // Verbatim, under its own participant name, into the resident's window. @@ -498,16 +498,16 @@ export class TuneOutCoordinator { case 'cancel_tuneout': { const channelId = String(input.channelId ?? ''); const located = this.findByChannel(channelId); - if (!located) return { success: false, error: `channel ${channelId} is not tuned out`, isError: false }; + if (!located) return { success: false, error: `channel ${channelId} is not tuned out`, isError: true }; const note = typeof input.text === 'string' && input.text.trim() ? input.text.trim() : undefined; const result = this.cancel(located.serverId, channelId, 'subconscious', 'subconscious judgment', note); return result.ok ? { success: true, data: { cancelled: true } } - : { success: false, error: result.error, isError: false }; + : { success: false, error: result.error, isError: true }; } case 'note_disposition': { const key = String(input.key ?? '').trim(); - if (!key) return { success: false, error: 'key required', isError: false }; + if (!key) return { success: false, error: 'key required', isError: true }; this.noteDisposition(key, String(input.text ?? '')); return { success: true, data: { noted: key } }; } @@ -515,13 +515,13 @@ export class TuneOutCoordinator { if (!this.hooks.allowChannelSpeech()) { return { success: false, - isError: false, + isError: true, error: 'channel speech is disabled for the subconscious (recipe: subconscious.allowChannelSpeech)', }; } const channelId = String(input.channelId ?? ''); const text = String(input.text ?? '').trim(); - if (!text) return { success: false, error: 'empty message', isError: false }; + if (!text) return { success: false, error: 'empty message', isError: true }; return this.channelRegistry.publishForAgent(channelId, text, this.hooks.subconsciousName() ?? 'Subconscious'); } default: diff --git a/test/refusals-are-error-results.test.ts b/test/refusals-are-error-results.test.ts new file mode 100644 index 000000000..c18383659 --- /dev/null +++ b/test/refusals-are-error-results.test.ts @@ -0,0 +1,190 @@ +/** + * A refusal is an error result. tune_out's refusals, channel_open on a + * tuned-out channel, the subconscious's refusals and the api module's + * "Unknown tool" reach the model flagged as errors, with the refusal's text, + * so it can tell a refusal from a success. Before, only deliver_summary's + * "subconscious not configured" was flagged: the model read each of the + * others as a tool that ran and answered (in XML tool mode, as the tool's + * stdout), and channel_open's refusal showed it only its data, never the + * sentence that says how to cancel the tune-out. A script's call gets each + * refusal as "Error: ", and a module calling in process gets the + * result flagged, with the tuned-out refusal's data kept. + */ +import { test, type TestContext } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { AgentFramework, ApiModule } from '../src/index.js'; +import type { Module, ModuleContext, ToolDefinition, ToolResult } from '../src/index.js'; +import { TUNE_OUT_DEFAULTS } from '../src/tune-out/coordinator.js'; +import { MockMembrane, createMockResponse } from './helpers/mock-membrane.js'; + +const FIXTURE = join(import.meta.dirname, 'fixtures/tune-out-mcpl-server.mjs'); +const CHANNEL = 'disc:guild:noisy'; +const TUNED_OUT = `Channel ${CHANNEL} is tuned out. Cancel the tune-out (tune_out with mode: 'cancel') ` + + 'to resume normal attention — cancelling delivers the diverted backlog.'; + +type SentResult = { toolUseId: string; content: unknown; isError: boolean }; + +async function waitFor(cond: () => boolean, what: string): Promise { + const deadline = Date.now() + 10_000; + while (!cond()) { + if (Date.now() > deadline) throw new Error(`timed out waiting for: ${what}`); + await new Promise((r) => setTimeout(r, 10)); + } +} + +/** A module that keeps its context, to call tools in process as a module does. */ +class ContextKeeper implements Module { + readonly name = 'context-keeper'; + ctx: ModuleContext | null = null; + async start(ctx: ModuleContext): Promise { + this.ctx = ctx; + } + async stop(): Promise { + this.ctx = null; + } + getTools(): ToolDefinition[] { + return []; + } + async handleToolCall(): Promise { + return { success: false, error: 'no tools', isError: true }; + } + async onProcess(): Promise> { + return {}; + } +} + +/** A resident and its subconscious, with the fixture's one channel registered. */ +async function setUp(t: TestContext, allowChannelSpeech = false, modules: Module[] = []) { + const dir = mkdtempSync(join(tmpdir(), 'refusals-are-errors-')); + const membrane = new MockMembrane(); + const framework = await AgentFramework.create({ + storePath: join(dir, 'store'), + membrane: membrane.asMembrane(), + agents: [{ name: 'scout', model: 'test-model', systemPrompt: 'You are scout.', proseRouting: 'disabled' }], + subconscious: { enabled: true, systemPrompt: 'Watch tuned-out channels and report.', allowChannelSpeech }, + mcplServers: [{ id: 'disc', command: process.execPath, args: [FIXTURE] }], + modules: [new ApiModule(), ...modules], + }); + t.after(async () => { + await framework.stop(); + rmSync(dir, { recursive: true, force: true }); + }); + const registry = (framework as unknown as { + channelRegistry: { listChannelsRaw(): unknown[] } | null; + }).channelRegistry; + await waitFor(() => (registry?.listChannelsRaw().length ?? 0) > 0, 'channel registration'); + return { framework, membrane }; +} + +/** + * Runs one turn of `agent` that makes each round of calls in turn, and + * returns, for each round, the results sent back to the model by call id. + */ +async function runRounds( + framework: AgentFramework, + membrane: MockMembrane, + agent: string, + rounds: Array }>>, +): Promise>> { + for (const calls of rounds) { + membrane.pushResponse(createMockResponse(calls.map((c) => ({ type: 'tool_use' as const, ...c })), 'tool_use')); + } + membrane.pushResponse(createMockResponse([])); + assert.equal(framework.nudgeAgent(agent).ok, true); + await framework.runUntilIdle(); + const sent = (membrane.lastStream!.receivedToolResults as SentResult[][]).map( + (round) => new Map(round.map((r) => [r.toolUseId, { content: r.content, isError: r.isError }])), + ); + assert.equal(sent.length, rounds.length, `${agent} answered every round`); + return sent; +} + +test("the resident's refusals reach the model as error results, and a tune-out it enters as a success", async (t) => { + const { framework, membrane } = await setUp(t); + const [first, second, third] = await runRounds(framework, membrane, 'scout', [ + [ + { id: 'unknown', name: 'tune_out', input: { channelId: 'disc:guild:none' } }, + { id: 'cancel', name: 'tune_out', input: { channelId: CHANNEL, mode: 'cancel' } }, + { id: 'api', name: 'api--lookup', input: {} }, + ], + [{ id: 'enter', name: 'tune_out', input: { channelId: CHANNEL } }], + [ + { id: 'again', name: 'tune_out', input: { channelId: CHANNEL } }, + { id: 'open', name: 'channel_open', input: { channelId: CHANNEL } }, + ], + ]); + assert.deepEqual(first!.get('unknown'), { content: 'unknown channel: disc:guild:none', isError: true }); + assert.deepEqual(first!.get('cancel'), { content: `channel ${CHANNEL} is not tuned out`, isError: true }); + assert.deepEqual(first!.get('api'), { content: 'Unknown tool', isError: true }); + // A control: the same tool's success still reads as one. + const { cadenceSeconds, backlogCap, maxWakes } = TUNE_OUT_DEFAULTS; + assert.deepEqual(second!.get('enter'), { + content: JSON.stringify({ tunedOut: CHANNEL, cadenceSeconds, backlogCap, maxWakes }), + isError: false, + }); + assert.deepEqual(third!.get('again'), { + content: `channel ${CHANNEL} is already tuned out (cancel first to change params)`, + isError: true, + }); + assert.deepEqual(third!.get('open'), { content: TUNED_OUT, isError: true }); +}); + +test("the subconscious's refusals reach it as error results", async (t) => { + const { framework, membrane } = await setUp(t); + const [round] = await runRounds(framework, membrane, 'Subconscious', [[ + { id: 'summary', name: 'deliver_summary', input: { text: ' ', channelId: CHANNEL } }, + { id: 'cancel', name: 'cancel_tuneout', input: { channelId: CHANNEL } }, + { id: 'note', name: 'note_disposition', input: { key: '', text: 'ignore release-bot' } }, + { id: 'speak', name: 'speak_in_channel', input: { channelId: CHANNEL, text: 'They are tuned out.' } }, + ]]); + assert.deepEqual(round!.get('summary'), { content: 'empty summary', isError: true }); + assert.deepEqual(round!.get('cancel'), { content: `channel ${CHANNEL} is not tuned out`, isError: true }); + assert.deepEqual(round!.get('note'), { content: 'key required', isError: true }); + assert.deepEqual(round!.get('speak'), { + content: 'channel speech is disabled for the subconscious (recipe: subconscious.allowChannelSpeech)', + isError: true, + }); +}); + +test("the subconscious's speak_in_channel with no text, where channel speech is allowed, reaches it as an error result", async (t) => { + const { framework, membrane } = await setUp(t, true); + const [round] = await runRounds(framework, membrane, 'Subconscious', [[ + { id: 'speak', name: 'speak_in_channel', input: { channelId: CHANNEL, text: ' ' } }, + ]]); + assert.deepEqual(round!.get('speak'), { content: 'empty message', isError: true }); +}); + +test('a script, and a module calling in process, get the refusals as errors too', async (t) => { + const keeper = new ContextKeeper(); + const { framework } = await setUp(t, false, [keeper]); + // A script's call is answered through the event loop, so it has to run. + framework.start(); + const script = (tool: string, input: Record) => (framework as unknown as { + handleScriptToolCall(agent: string, tool: string, args: Record): Promise; + }).handleScriptToolCall('scout', tool, input); + assert.equal(await script('tune_out', { channelId: 'disc:guild:none' }), 'Error: unknown channel: disc:guild:none'); + assert.equal(await script('api--lookup', {}), 'Error: Unknown tool'); + // A control: the same tool's success still reads as one. + const { cadenceSeconds, backlogCap, maxWakes } = TUNE_OUT_DEFAULTS; + assert.equal( + await script('tune_out', { channelId: CHANNEL }), + JSON.stringify({ tunedOut: CHANNEL, cadenceSeconds, backlogCap, maxWakes }), + ); + assert.equal(await script('channel_open', { channelId: CHANNEL }), `Error: ${TUNED_OUT}`); + // A module gets the result as the registry returns it: flagged, with the + // refusal's data kept, so it can still read why. + assert.deepEqual(await keeper.ctx!.callTool({ id: 'ctx-open', name: 'channel_open', input: { channelId: CHANNEL } }), { + success: false, + isError: true, + error: TUNED_OUT, + data: { refusal: 'tuned-out', channelId: CHANNEL }, + }); + assert.deepEqual( + await keeper.ctx!.callTool({ id: 'ctx-api', name: 'api--lookup', input: {} }), + { success: false, error: 'Unknown tool', isError: true }, + ); +});