From 341306d1868d6be6e384ea37bd8c33653bfa9f6e Mon Sep 17 00:00:00 2001 From: Crispin-2713 Date: Sun, 18 Oct 2026 16:54:17 -0400 Subject: [PATCH] feat(code-execution): a script's tool result carries .is_error and .structured beside its string A code_execution script gets each tool result as one string: the tool's text, an "Error: ..." string for a failure, or a structured result's JSON object. So a script tells a failure from a success by the text's prefix, and a successful result whose text happens to start with "Error:" can't be told from one. The string is now a str subclass, ToolResult, unchanged as a string, with two attributes beside it: - .is_error: True for an error result, False otherwise. The framework sets it at every return of handleScriptToolCall and scriptStructuredResult: True for each "Error: ..." string it makes (a failure, a refusal, code_execution called from a script, a result the script serializer can't convert) and for a structured result whose isError is true, including the two forms that couldn't be delivered (past the size cap and unsaved, and one JSON can't hold); False for text and data results, and for a result its tool returned with isError false, as the model reads it. scriptStructuredResult reads each flag from the object its string is made from. - .structured: the parsed structuredContent of a structured result's JSON object, or None. The host sends the string once and marks it as such an object (structured_json); the runner parses it on first read. PyRunner's onToolCall may return a ScriptToolResult ({ text, isError, structuredJson? }), exported, or a plain string as before, for which both attributes read None. Any other value is sent as before, and a handler that rejects reaches the script as an "Error: ..." result flagged True. ToolResult's constructor takes defaults so that copy, deepcopy and pickle, which rebuild a str subclass from its text, keep working in the script. The kwargs refusal shows a tool result key as its text, as it shows a str. The tool description gains a sentence saying what the attributes mean, so the first request after the upgrade that carries it misses the prompt cache from its tool block on. Tests pin the flag at each return, through both of the framework's runners, and the runner's value under a real python3. Co-Authored-By: Claude Opus 5.5 --- changelog.d/script-result-flags.added.md | 15 ++++ src/code-execution/index.ts | 1 + src/code-execution/py-runner.ts | 42 ++++++++++- src/code-execution/runtime-py.ts | 44 +++++++++-- src/code-execution/tool-definition.ts | 3 + src/framework.ts | 36 +++++---- src/index.ts | 1 + test/code-execution.test.ts | 89 ++++++++++++++++++++++- test/mcp-modern-framework.test.ts | 63 +++++++++++----- test/refusals-are-error-results.test.ts | 20 +++-- test/script-structured-references.test.ts | 8 +- test/tool-result-reference-uri.test.ts | 7 +- test/unconvertible-tool-result.test.ts | 44 +++++++++-- 13 files changed, 307 insertions(+), 66 deletions(-) create mode 100644 changelog.d/script-result-flags.added.md diff --git a/changelog.d/script-result-flags.added.md b/changelog.d/script-result-flags.added.md new file mode 100644 index 000000000..11015b6ee --- /dev/null +++ b/changelog.d/script-result-flags.added.md @@ -0,0 +1,15 @@ +- **A `code_execution` script can test a result for an error, and read its + structured content, without parsing.** Each tool function's string is + unchanged, and carries two attributes beside it: `.is_error`, `True` for an + error result (a failure the framework reports as `Error: …`, or a + structured result whose `isError` is true) and `False` otherwise, and + `.structured`, the parsed `structuredContent` of a structured result's + JSON object, or `None`. A result whose text merely starts with `Error:` + reads `False`. The string is now a `str` subclass, so code that uses it as + a string keeps working, `json.loads` and `isinstance(r, str)` included, + while an exact type check such as `type(r) is str` now reads `False`, and + a result a script pickles loads only in the script runtime. + `PyRunner`'s `onToolCall` may return a `ScriptToolResult` (exported) to + set both, or a plain string as before, for which both read `None`. + `code_execution`'s description changes, so the first request after the + upgrade that carries it misses the prompt cache from its tool block on. diff --git a/src/code-execution/index.ts b/src/code-execution/index.ts index 58f5f2a7c..062fd1f38 100644 --- a/src/code-execution/index.ts +++ b/src/code-execution/index.ts @@ -14,6 +14,7 @@ export type { ExecResult, PyRunnerOptions, ScriptToolCallHandler, + ScriptToolResult, BackgroundExecOptions, } from './py-runner.js'; export { PYTHON_RUNTIME_SOURCE } from './runtime-py.js'; diff --git a/src/code-execution/py-runner.ts b/src/code-execution/py-runner.ts index a534061b3..397be0054 100644 --- a/src/code-execution/py-runner.ts +++ b/src/code-execution/py-runner.ts @@ -55,11 +55,30 @@ export interface BackgroundExecOptions { lifetimeMs: number; } -/** Resolves inner tool calls. Must never reject — map errors to strings. */ +/** + * An inner tool call's result as the script gets it. `text` is the string + * its await returns, and the runner sets `.is_error` on that string from + * `isError`. With `structuredJson`, `text` is the JSON object a structured + * result reaches a script as, in any of its forms, and `.structured` reads + * its `structuredContent`, None for a form that holds none, such as one past + * the size cap; without it, `.structured` is None. + */ +export interface ScriptToolResult { + text: string; + isError: boolean; + structuredJson?: boolean; +} + +/** + * Resolves inner tool calls. Must never reject — map errors to results; a + * rejection reaches the script as an `Error: …` result. A plain string is + * the result's text with nothing said about it, so the script's + * `.is_error` and `.structured` are None. + */ export type ScriptToolCallHandler = ( toolName: string, args: Record, -) => Promise; +) => Promise; export interface PyRunnerOptions { pythonPath?: string; @@ -530,9 +549,24 @@ export class PyRunner { ? (msg.args as Record) : {}; this.onToolCall(toolName, args) - .catch((err) => `Error: ${err instanceof Error ? err.message : String(err)}`) + .catch((err): ScriptToolResult => ({ + text: `Error: ${err instanceof Error ? err.message : String(err)}`, + isError: true, + })) .then((result) => { - this.reply(child, { op: 'tool_result', id: callId, result }, `result of ${toolName} call ${callId}`); + // Only an object is a ScriptToolResult. Anything else a handler + // resolves with, a string or a value its type rules out, is sent + // as it is, so the call is still answered. + const reply = typeof result === 'object' && result !== null + ? { + op: 'tool_result', + id: callId, + result: result.text, + is_error: result.isError, + ...(result.structuredJson ? { structured_json: true } : {}), + } + : { op: 'tool_result', id: callId, result }; + this.reply(child, reply, `result of ${toolName} call ${callId}`); }); return; } diff --git a/src/code-execution/runtime-py.ts b/src/code-execution/runtime-py.ts index 840f61826..89147ed35 100644 --- a/src/code-execution/runtime-py.ts +++ b/src/code-execution/runtime-py.ts @@ -7,7 +7,8 @@ * * - scripts run with top-level `await` (PyCF_ALLOW_TOP_LEVEL_AWAIT) * - host tools are injected as async functions taking a single dict and - * returning a string (the tool_result text) + * returning the tool_result text as a str (ToolResult), which also + * carries .is_error and .structured * - a pending tool call that gets no response raises TimeoutError with the * same message shape the managed runtime produces * - interpreter globals persist across execs (container-reuse semantics) @@ -29,7 +30,7 @@ * host -> py : {op:"init", tools:[{py_name,tool_name}], call_timeout_s, * background?, log_path?, tail_chars?} * {op:"exec", id, code} - * {op:"tool_result", id, result} + * {op:"tool_result", id, result, is_error?, structured_json?} * {op:"wake_ack", id, error?} * {op:"cancel", id, reason?} * py -> host : {op:"ready"} @@ -49,6 +50,7 @@ export const PYTHON_RUNTIME_SOURCE: string = String.raw` import ast import asyncio +import functools import inspect import io import json @@ -258,6 +260,31 @@ def _deliver_cancel(st): st.task.cancel() +class ToolResult(str): + """A tool's result: the string the tool gave, with two attributes. + + is_error: True if the result is an error, False if not, None if the + host didn't say. + structured: the parsed structuredContent of a structured result's JSON + object, or None. + """ + + # Defaults, since pickle and copy rebuild a str subclass from its text + # alone and then restore the attributes. + def __new__(cls, text="", is_error=None, structured_json=False): + self = str.__new__(cls, text) + self.is_error = is_error + self._structured_json = structured_json + return self + + # Read from the string itself, the one copy sent, on first use only. + @functools.cached_property + def structured(self): + if not self._structured_json: + return None + return json.loads(self).get("structuredContent") + + def _resolve(fut, value): # Defense in depth: the script may have been cancelled since the lookup. if fut is not None and not fut.done(): @@ -307,8 +334,9 @@ def _make_tool_fn(tool_name, py_name): meant.update(kwargs) # A key is what the agent has to write, so a str key is shown # whole up to 60 characters, as names are elsewhere; a longer one, - # or any other key, is shown as a value is. - shown = ", ".join((repr(k) if type(k) is str and len(k) <= 60 else _brief(k)) + ": " + _brief(v) + # or any other key, is shown as a value is. A tool's result counts + # as a str: ToolResult keeps str's repr, so no script code runs. + shown = ", ".join((repr(k) if (type(k) is str or type(k) is ToolResult) and len(k) <= 60 else _brief(k)) + ": " + _brief(v) for k, v in meant.items()) raise TypeError( py_name + " wasn't called: a tool function takes one dict of " @@ -347,7 +375,8 @@ def _make_tool_fn(tool_name, py_name): tool_fn.__qualname__ = py_name tool_fn.__doc__ = ( "Host tool '" + tool_name + "'. Takes a single dict of arguments and " - "returns the tool result as a string." + "returns the tool result as a string, with .is_error and .structured " + "beside it." ) return tool_fn @@ -537,7 +566,10 @@ def _dispatch(msg): ) st.task = _current_exec_task elif op == "tool_result": - _resolve(_pending_tool_futures.get(msg.get("id")), str(msg.get("result", ""))) + _resolve( + _pending_tool_futures.get(msg.get("id")), + ToolResult(str(msg.get("result", "")), msg.get("is_error"), msg.get("structured_json") is True), + ) elif op == "wake_ack": _resolve(_pending_wake_futures.get(msg.get("id")), msg.get("error") or None) elif op == "cancel": diff --git a/src/code-execution/tool-definition.ts b/src/code-execution/tool-definition.ts index dbf37dae9..a14934e00 100644 --- a/src/code-execution/tool-definition.ts +++ b/src/code-execution/tool-definition.ts @@ -42,6 +42,9 @@ export function buildCodeExecutionToolDefinition(opts?: { `workspace path the payload was saved to (null if it wasn't) and "refId" is fetch_reference's ` + 'ref_id. A structured result over 5 MB is saved to the workspace instead, and the string is ' + '{"isError": ..., "oversized": {"chars": ..., "savedTo": ...}}. ' + + 'Each string also carries .is_error (True if the result is an error, else False; test it ' + + 'rather than an "Error:" prefix) and .structured (the parsed "structuredContent" of the ' + + 'JSON object above, or None). ' + "An exact-name lookup dict is also available: tools['mcpl--discord--fetch_history']({...}). " + 'Use top-level await; run independent calls in parallel with asyncio.gather. ' + 'Only what you print() (plus stderr and the exit code) comes back to you — intermediate ' + diff --git a/src/framework.ts b/src/framework.ts index b06394192..c0d092ae7 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -132,7 +132,7 @@ import { type ShownRanges, } from './tool-image-ledger.js'; import { randomUUID, createHash } from 'node:crypto'; -import { PyRunner, buildInjectedTools, formatLimit } from './code-execution/py-runner.js'; +import { PyRunner, buildInjectedTools, formatLimit, type ScriptToolResult } from './code-execution/py-runner.js'; import { buildCodeExecutionToolDefinition, CODE_EXECUTION_TOOL_NAME, @@ -18824,8 +18824,10 @@ export class AgentFramework { /** * Resolve one script-inner tool call to the string the script's tool - * function returns. Never rejects: errors become "Error: ..." strings, - * matching the managed PTC runtime ("Claude's code receives this error"). + * function returns, and whether that result is an error (the script's + * `.is_error`). Never rejects: a failure without structured content becomes + * an "Error: ..." string, matching the managed PTC runtime ("Claude's code + * receives this error"), and a structured one its JSON object. * * A result without structured content is serialized with the HISTORY * serializer: images become placeholders, since a script variable holding @@ -18844,9 +18846,9 @@ export class AgentFramework { origin?: ScriptCallOrigin, /** The foreground script that made the call; none for a background script. */ script?: { endTurn: boolean; turnId?: number }, - ): Promise { + ): Promise { if (toolName === CODE_EXECUTION_TOOL_NAME) { - return 'Error: code_execution cannot be called from within a script'; + return { text: 'Error: code_execution cannot be called from within a script', isError: true }; } // A foreground script's call was made in the turn of the code_execution // call whose script is running when it arrives. A background script runs @@ -18863,10 +18865,10 @@ export class AgentFramework { // it back out of text. const structured = Object.prototype.hasOwnProperty.call(result, 'structured'); if (result.isError && !structured) { - return `Error: ${result.error ?? 'tool call failed'}`; + return { text: `Error: ${result.error ?? 'tool call failed'}`, isError: true }; } if ((result.data === undefined || result.data === null) && !structured) { - return nonErrorResultText(result, AgentFramework.SCRIPT_RESULT_MAX_CHARS); + return { text: nonErrorResultText(result, AgentFramework.SCRIPT_RESULT_MAX_CHARS), isError: false }; } // RFC-005 "eagerly lazy": a script receiving a result is the strongest // available signal that the bytes are wanted (scripts process, they do @@ -18898,9 +18900,9 @@ export class AgentFramework { // context-cap truncation, no file spill. if (structured) return this.scriptStructuredResult(result, toolName); try { - return toolResultDataToHistoryString(result.data, AgentFramework.SCRIPT_RESULT_MAX_CHARS); + return { text: toolResultDataToHistoryString(result.data, AgentFramework.SCRIPT_RESULT_MAX_CHARS), isError: false }; } catch (err) { - return `Error: ${unconvertibleScriptResult(toolName, err)}`; + return { text: `Error: ${unconvertibleScriptResult(toolName, err)}`, isError: true }; } } @@ -18923,9 +18925,13 @@ export class AgentFramework { * that can't be converted to JSON (a cyclic object, a BigInt), which the * dispatch's own check makes unlikely here, gets the same object empty, * `{ content: [], structuredContent: null, isError: true, error }`, the - * error naming the tool. + * error naming the tool. In every form, the script's `.is_error` is the + * `isError` of the object its string holds, and its `.structured` is read + * from that string. */ - private async scriptStructuredResult(result: ToolResult, toolName: string): Promise { + private async scriptStructuredResult(result: ToolResult, toolName: string): Promise { + const asSent = (value: T, json: string = JSON.stringify(value)): ScriptToolResult => + ({ text: json, isError: value.isError, structuredJson: true }); const object = { content: Array.isArray(result.data) ? result.data : [], structuredContent: result.structured, @@ -18937,19 +18943,19 @@ export class AgentFramework { json = JSON.stringify(object); } catch (err) { const error = unconvertibleScriptResult(toolName, err, result.isError ? result.error : undefined); - return JSON.stringify({ content: [], structuredContent: null, isError: true, error }); + return asSent({ content: [], structuredContent: null, isError: true, error }); } - if (json.length <= AgentFramework.SCRIPT_RESULT_MAX_CHARS) return json; + if (json.length <= AgentFramework.SCRIPT_RESULT_MAX_CHARS) return asSent(object, json); const workspace = this.getWorkspaceModule(); const mount = workspace ? this.firstWritableMountName(workspace) : null; if (workspace && mount) { const path = `${mount}/tool-results/${new Date().toISOString().slice(0, 10)}-script-${randomUUID()}.json`; const written = await workspace.writeBinary(path, Buffer.from(json, 'utf8'), 'application/json').catch(() => null); if (written?.success) { - return JSON.stringify({ isError: object.isError, oversized: { chars: json.length, savedTo: path } }); + return asSent({ isError: object.isError, oversized: { chars: json.length, savedTo: path } }); } } - return JSON.stringify({ + return asSent({ isError: true, error: `the result (${json.length} chars) exceeds the ${AgentFramework.SCRIPT_RESULT_MAX_CHARS}-char script result cap ` + `and could not be saved to the workspace, so it could not be delivered; the tool may already have completed`, diff --git a/src/index.ts b/src/index.ts index fd34068f3..dc3748915 100644 --- a/src/index.ts +++ b/src/index.ts @@ -74,6 +74,7 @@ export type { ExecResult, PyRunnerOptions, ScriptToolCallHandler, + ScriptToolResult, } from './code-execution/index.js'; export type { SessionUsage, AgentUsage, SessionUsageSnapshot, UsageUpdatedEvent } from './usage/index.js'; diff --git a/test/code-execution.test.ts b/test/code-execution.test.ts index 36834a0c4..43645ed44 100644 --- a/test/code-execution.test.ts +++ b/test/code-execution.test.ts @@ -20,6 +20,7 @@ import type { ModuleContext, ProcessEvent, ProcessState, + ScriptToolResult, ToolCall, ToolDefinition, ToolResult, @@ -136,6 +137,77 @@ describe('PyRunner (real python3)', () => { } }); + it('gives a script each result as its string, with .is_error and .structured beside it', async () => { + const structured = JSON.stringify({ content: [], structuredContent: { rows: [1, 2] }, isError: false }); + const oversized = JSON.stringify({ isError: true, oversized: { chars: 9, savedTo: null } }); + const results: Record = { + failed: { text: 'Error: boom', isError: true }, + // Text that merely starts like an error, the case .is_error is for. + textlike: { text: 'Error: a line of the file, not a failure', isError: false }, + structured: { text: structured, isError: false, structuredJson: true }, + oversized: { text: oversized, isError: true, structuredJson: true }, + // A handler that returns a plain string says nothing about it. + plain: 'a string, nothing said', + // Unmarked text holding a structuredContent key is still text. + jsonlike: { text: '{"structuredContent": {"a": 1}}', isError: false }, + }; + const runner = new PyRunner({ + onToolCall: async (_name, args) => { + if (args.kind === 'rejects') throw new Error('handler broke'); + // A JS handler can resolve with what its type rules out; the call is + // still answered, as before this change. + if (args.kind === 'nothing') return undefined as unknown as string; + return results[String(args.kind)]!; + }, + }); + try { + const result = await runner.exec( + [ + 'import copy, json, pickle', + 'out = {}', + 'for kind in ["failed", "textlike", "structured", "oversized", "plain", "rejects", "jsonlike", "nothing"]:', + ' r = await test__echo({"kind": kind})', + ' out[kind] = {"text": str(r), "is_error": r.is_error, "structured": r.structured,', + ' "type": type(r).__name__, "is_str": isinstance(r, str)}', + 'r = await test__echo({"kind": "structured"})', + 'out["parsed"] = json.loads(r)["structuredContent"]', + 'out["copies"] = [[c.is_error, c.structured, c == r] for c in (copy.copy(r), pickle.loads(pickle.dumps(r)))]', + 'key = await test__echo({"kind": "plain"})', + 'try:', + ' await test__echo({key: 1}, extra=2)', + 'except TypeError as e:', + ' out["refusal"] = str(e)', + 'print(json.dumps(out))', + ].join('\n'), + ECHO_TOOLS, + ); + assert.strictEqual(result.returnCode, 0, result.stderr); + const out = JSON.parse(result.stdout) as Record; + const seen = (text: string, isError: boolean | null, value: unknown) => + ({ text, is_error: isError, structured: value, type: 'ToolResult', is_str: true }); + assert.deepStrictEqual(out.failed, seen('Error: boom', true, null)); + assert.deepStrictEqual(out.textlike, seen('Error: a line of the file, not a failure', false, null)); + assert.deepStrictEqual(out.structured, seen(structured, false, { rows: [1, 2] })); + // Marked as a structured result's object, but holding no + // structuredContent: none to read. + assert.deepStrictEqual(out.oversized, seen(oversized, true, null)); + assert.deepStrictEqual(out.plain, seen('a string, nothing said', null, null)); + // A handler that rejects reaches the script as an error, flagged. + assert.deepStrictEqual(out.rejects, seen('Error: handler broke', true, null)); + // The string is unchanged for code that parses it itself. + assert.deepStrictEqual(out.parsed, { rows: [1, 2] }); + // copy and pickle keep both attributes. + assert.deepStrictEqual(out.copies, [[false, { rows: [1, 2] }, true], [false, { rows: [1, 2] }, true]]); + assert.deepStrictEqual(out.jsonlike, seen('{"structuredContent": {"a": 1}}', false, null)); + assert.deepStrictEqual(out.nothing, seen('', null, null)); + // A refusal shows a key that is a tool's result as its text, the key the + // agent has to write, as it shows any str key. + assert.match(String(out.refusal), /\{'a string, nothing said': , 'extra': \}/); + } finally { + runner.dispose(); + } + }); + it('supports the exact-name tools[...] escape hatch', async () => { const runner = new PyRunner({ onToolCall: async (_name, args) => `got:${String(args.v)}`, @@ -986,10 +1058,14 @@ describe('framework code_execution integration (real python3)', () => { const code = [ 'import json', 'names = []', + 'flags = []', 'for m in ["alpha", "beta"]:', - ' r = json.loads(await test__echo({"message": m}))', + ' raw = await test__echo({"message": m})', + ' flags.append(raw.is_error)', + ' r = json.loads(raw)', ' names.append(r["echoed"]["message"].upper())', 'print("summary:", "+".join(names))', + 'print("is_error:", flags)', ].join('\n'); membrane.pushResponse(createMockResponse([ @@ -1029,6 +1105,9 @@ describe('framework code_execution integration (real python3)', () => { const wireResult = stream.receivedToolResults[0][0] as { toolUseId: string; content: string }; assert.strictEqual(wireResult.toolUseId, 'call_ce_1'); assert.match(wireResult.content, /summary: ALPHA\+BETA/); + // Through the framework, each result says it isn't an error (a + // handler that passed on only the string would make it None). + assert.match(wireResult.content, /is_error: \[False, False\]/); assert.match(wireResult.content, /"return_code":\s*0/); // ...and NOT the intermediate tool results. assert.ok( @@ -1280,7 +1359,8 @@ describe('framework background scripts + spill (real python3)', () => { const bgCode = [ 'import asyncio', 'await asyncio.sleep(0.2)', - 'await wake_agent({"found": "signal"})', + 'r = await test__echo({"message": "bg"})', + 'await wake_agent({"found": "signal", "is_error": r.is_error})', ].join('\\n'); // Turn 1: spawn the background script, then finish the turn. @@ -1315,8 +1395,11 @@ describe('framework background scripts + spill (real python3)', () => { assert.ok(wakeMessage, 'wake message should be injected'); const text = (wakeMessage.content[0] as { text?: string }).text ?? ''; assert.match(text, /\[background script bg-\d+\] Woke you/); - assert.match(text, /line 3 of your script/); + assert.match(text, /line 4 of your script/); assert.match(text, /"found": "signal"/); + // A background script's results carry the flag too (None if only the + // string were passed on). + assert.match(text, /"is_error": false/); assert.match(text, /workspace file files\/background-scripts\/bg-\d+\.log/); assert.match(text, /Script status: still running|Script status/); } finally { diff --git a/test/mcp-modern-framework.test.ts b/test/mcp-modern-framework.test.ts index acf85fcfe..82e5c88bf 100644 --- a/test/mcp-modern-framework.test.ts +++ b/test/mcp-modern-framework.test.ts @@ -16,7 +16,7 @@ import { join, dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; import { AgentFramework, WorkspaceModule } from '../src/index.js'; -import type { EventResponse, Module, ModuleContext, ProcessEvent, ProcessState, ToolDefinition, ToolResult } from '../src/index.js'; +import type { EventResponse, Module, ModuleContext, ProcessEvent, ProcessState, ScriptToolResult, ToolDefinition, ToolResult } from '../src/index.js'; import { MockMembrane, createMockResponse } from './helpers/mock-membrane.js'; const here = dirname(fileURLToPath(import.meta.url)); @@ -161,21 +161,35 @@ test('model path: the result reaches the model, structured-only as JSON', async }); }); +/** A script's result with its JSON string parsed, to compare whole. */ +const parsedText = (result: ScriptToolResult) => ({ ...result, text: JSON.parse(result.text) as unknown }); + test('scripts get structured results whole, as one JSON object', async () => { await withModern(async ({ framework }) => { framework.start(); const handle = (framework as unknown as { - handleScriptToolCall: (agent: string, tool: string, args: Record) => Promise; + handleScriptToolCall: (agent: string, tool: string, args: Record) => Promise; }).handleScriptToolCall.bind(framework); - const out = JSON.parse(await handle('agent', 'mcpl--modern--structured_and_text', {})); - assert.deepEqual(out, { - content: [{ type: 'text', text: 'Found 2 rows.' }], - structuredContent: { rows: [{ id: 1 }, { id: 2 }] }, + // Beside the string: isError, which the script reads as .is_error, and + // structuredJson, which has it read .structured from the string. + assert.deepEqual(parsedText(await handle('agent', 'mcpl--modern--structured_and_text', {})), { + text: { + content: [{ type: 'text', text: 'Found 2 rows.' }], + structuredContent: { rows: [{ id: 1 }, { id: 2 }] }, + isError: false, + }, isError: false, + structuredJson: true, + }); + // A structured tool error is flagged as one beside its string. + assert.deepEqual(parsedText(await handle('agent', 'mcpl--modern--sized', { pad: 1, isError: true })), { + text: sizedObject(1, true), + isError: true, + structuredJson: true, }); // Image plus scores: both views reach the script whole, the image's // payload included (no placeholder stands in for it). - const scored = JSON.parse(await handle('agent', 'mcpl--modern--scored_image', {})); + const scored = JSON.parse((await handle('agent', 'mcpl--modern--scored_image', {})).text); assert.deepEqual(scored, { content: [{ type: 'image', data: 'iVBORw0KGgo=', mimeType: 'image/png' }], structuredContent: { score: 0.9, label: 'cat' }, @@ -184,7 +198,7 @@ test('scripts get structured results whole, as one JSON object', async () => { // A resource link, embedded resources and audio are ordinary MCP blocks // from a modern server, not RFC-005 references: the script reads them as // the server sent them. - const media = JSON.parse(await handle('agent', 'mcpl--modern--structured_media', {})); + const media = JSON.parse((await handle('agent', 'mcpl--modern--structured_media', {})).text); assert.deepEqual(media, { content: [ { type: 'text', text: 'before' }, @@ -198,8 +212,9 @@ test('scripts get structured results whole, as one JSON object', async () => { isError: false, }); // Without structured content the script contract is unchanged (pinned): - // the history rendering, which JSON-encodes a text result. - assert.equal(await handle('agent', 'mcpl--modern--echo', { text: 'x' }), JSON.stringify('echo:x')); + // the history rendering, which JSON-encodes a text result. No + // structuredJson, so the script's .structured is None. + assert.deepEqual(await handle('agent', 'mcpl--modern--echo', { text: 'x' }), { text: JSON.stringify('echo:x'), isError: false }); }); }); @@ -220,7 +235,7 @@ const sizedObject = (pad: number, isError = false) => ({ /** The pad that makes the script's JSON exactly `chars` long. */ const padFor = (chars: number, isError = false) => chars - JSON.stringify(sizedObject(0, isError)).length; const scriptHandle = (framework: AgentFramework) => (framework as unknown as { - handleScriptToolCall: (agent: string, tool: string, args: Record) => Promise; + handleScriptToolCall: (agent: string, tool: string, args: Record) => Promise; }).handleScriptToolCall.bind(framework); async function withWorkspace(fn: (h: Harness & { workspace: WorkspaceModule }) => Promise): Promise { @@ -255,16 +270,22 @@ test('script results: exactly at the cap arrive inline; one past it are saved wh const handle = scriptHandle(framework); const at = await handle('agent', 'mcpl--modern--sized', { pad: padFor(SCRIPT_CAP) }); - assert.equal(at.length, SCRIPT_CAP); - assert.deepEqual(JSON.parse(at), sizedObject(padFor(SCRIPT_CAP))); - - const over = JSON.parse(await handle('agent', 'mcpl--modern--sized', { pad: padFor(SCRIPT_CAP + 1) })); + assert.equal(at.text.length, SCRIPT_CAP); + assert.deepEqual(parsedText(at), { text: sizedObject(padFor(SCRIPT_CAP)), isError: false, structuredJson: true }); + + // Past the cap the string holds where the object went, and is still + // marked as a structured result's object: .structured reads none from it. + const overResult = await handle('agent', 'mcpl--modern--sized', { pad: padFor(SCRIPT_CAP + 1) }); + assert.deepEqual({ isError: overResult.isError, structuredJson: overResult.structuredJson }, { isError: false, structuredJson: true }); + const over = JSON.parse(overResult.text); assert.deepEqual(Object.keys(over).sort(), ['isError', 'oversized']); assert.equal(over.isError, false); assert.equal(over.oversized.chars, SCRIPT_CAP + 1); assert.deepEqual(await savedObject(workspace, over.oversized.savedTo), sizedObject(padFor(SCRIPT_CAP + 1)), 'the whole object, recoverable'); - const failed = JSON.parse(await handle('agent', 'mcpl--modern--sized', { pad: padFor(SCRIPT_CAP + 1, true), isError: true })); + const failedResult = await handle('agent', 'mcpl--modern--sized', { pad: padFor(SCRIPT_CAP + 1, true), isError: true }); + assert.equal(failedResult.isError, true, 'flagged beside the string too'); + const failed = JSON.parse(failedResult.text); assert.equal(failed.isError, true, 'a tool error stays visible without reading the file'); assert.equal(failed.oversized.chars, SCRIPT_CAP + 1); assert.notEqual(failed.oversized.savedTo, over.oversized.savedTo); @@ -273,19 +294,23 @@ test('script results: exactly at the cap arrive inline; one past it are saved wh }); test('script results past the cap: no workspace, or a failed write, is a failure to deliver, never a success', async () => { - const assertUndelivered = (out: { isError: boolean; error: string; oversized: unknown }) => { + const assertUndelivered = (result: ScriptToolResult) => { + // Not delivered: flagged as an error beside a string that says the tool + // may already have completed. + assert.deepEqual({ isError: result.isError, structuredJson: result.structuredJson }, { isError: true, structuredJson: true }); + const out = JSON.parse(result.text) as { isError: boolean; error: string; oversized: unknown }; assert.equal(out.isError, true); assert.match(out.error, /exceeds the 5000000-char script result cap and could not be saved to the workspace, so it could not be delivered; the tool may already have completed/); assert.deepEqual(out.oversized, { chars: SCRIPT_CAP + 1, savedTo: null }); }; await withModern(async ({ framework }) => { framework.start(); - assertUndelivered(JSON.parse(await scriptHandle(framework)('agent', 'mcpl--modern--sized', { pad: padFor(SCRIPT_CAP + 1) }))); + assertUndelivered(await scriptHandle(framework)('agent', 'mcpl--modern--sized', { pad: padFor(SCRIPT_CAP + 1) })); }); await withWorkspace(async ({ framework, workspace }) => { framework.start(); workspace.writeBinary = async () => ({ success: false, error: 'disk full', isError: true }); - assertUndelivered(JSON.parse(await scriptHandle(framework)('agent', 'mcpl--modern--sized', { pad: padFor(SCRIPT_CAP + 1) }))); + assertUndelivered(await scriptHandle(framework)('agent', 'mcpl--modern--sized', { pad: padFor(SCRIPT_CAP + 1) })); }); }); diff --git a/test/refusals-are-error-results.test.ts b/test/refusals-are-error-results.test.ts index 39356fa10..02543a6ff 100644 --- a/test/refusals-are-error-results.test.ts +++ b/test/refusals-are-error-results.test.ts @@ -17,7 +17,7 @@ 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 type { Module, ModuleContext, ScriptToolResult, ToolDefinition, ToolResult } from '../src/index.js'; import { TUNE_OUT_DEFAULTS } from '../src/tune-out/coordinator.js'; import { MockMembrane, createMockResponse } from './helpers/mock-membrane.js'; @@ -189,17 +189,23 @@ test('a script, and a module calling in process, get the refusals as errors too' // 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(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: ${API_UNKNOWN}`); + // Each refusal reaches the script as an error, flagged as one (.is_error). + assert.deepEqual(await script('tune_out', { channelId: 'disc:guild:none' }), { text: 'Error: unknown channel: disc:guild:none', isError: true }); + assert.deepEqual(await script('api--lookup', {}), { text: `Error: ${API_UNKNOWN}`, isError: true }); // A control: the same tool's success still reads as one. const { cadenceSeconds, backlogCap, maxWakes } = TUNE_OUT_DEFAULTS; - assert.equal( + assert.deepEqual( await script('tune_out', { channelId: CHANNEL }), - JSON.stringify({ tunedOut: CHANNEL, cadenceSeconds, backlogCap, maxWakes }), + { text: JSON.stringify({ tunedOut: CHANNEL, cadenceSeconds, backlogCap, maxWakes }), isError: false }, + ); + assert.deepEqual(await script('channel_open', { channelId: CHANNEL }), { text: `Error: ${TUNED_OUT}`, isError: true }); + // A script can't start a script: refused before any dispatch, as an error. + assert.deepEqual( + await script('code_execution', { code: 'print(1)' }), + { text: 'Error: code_execution cannot be called from within a script', isError: true }, ); - 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 } }), { diff --git a/test/script-structured-references.test.ts b/test/script-structured-references.test.ts index 465ca069f..b610135c1 100644 --- a/test/script-structured-references.test.ts +++ b/test/script-structured-references.test.ts @@ -17,7 +17,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { AgentFramework, WorkspaceModule } from '../src/index.js'; -import type { ToolResult } from '../src/index.js'; +import type { ScriptToolResult, ToolResult } from '../src/index.js'; import { EXPIRY_REASON, INLINE_WITHHELD_TEXT } from '../src/mcpl/references.js'; import type { ReferenceRecord } from '../src/mcpl/references.js'; import { MockMembrane } from './helpers/mock-membrane.js'; @@ -96,7 +96,7 @@ async function withPeers(fn: (h: Harness) => Promise, opts: { eager?: bool mcplServerRegistry: unknown; mcplPrefixMap: Map; getReferenceFetcher: () => unknown; - handleScriptToolCall: (agent: string, tool: string, args: Record) => Promise; + handleScriptToolCall: (agent: string, tool: string, args: Record) => Promise; dispatchMcplToolCall: (agent: string, call: { id: string; name: string; input: unknown }, serverId: string, prefix: string) => void; pushEvent: (event: { type: string; callId?: string; result?: ToolResult }) => void; }; @@ -130,7 +130,9 @@ async function withPeers(fn: (h: Harness) => Promise, opts: { eager?: bool try { await fn({ answer: (result) => { answer = result; }, - script: (tool) => internals.handleScriptToolCall.call(framework, 'agent', tool, {}), + // The string a script's call returns; its flags are pinned in + // mcp-modern-framework and code-execution. + script: async (tool) => (await internals.handleScriptToolCall.call(framework, 'agent', tool, {})).text, modelCall, fetched, workspace, diff --git a/test/tool-result-reference-uri.test.ts b/test/tool-result-reference-uri.test.ts index 6f9f82dbb..64dc82da2 100644 --- a/test/tool-result-reference-uri.test.ts +++ b/test/tool-result-reference-uri.test.ts @@ -11,6 +11,7 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { AgentFramework } from '../src/index.js'; +import type { ScriptToolResult } from '../src/index.js'; import type { Module, ModuleContext, ToolCall, ToolResult, ToolDefinition } from '../src/types/index.js'; import { nonErrorResultText, toolResultDataToHistoryString } from '../src/tool-result-history.js'; import { parseImagePlaceholders } from '../src/tool-image-ledger.js'; @@ -308,14 +309,14 @@ describe('a model\'s call answered with such data', () => { describe('a script given such data', () => { type ScriptFramework = { dispatchScriptToolCall: (...args: unknown[]) => Promise; - handleScriptToolCall(agentName: string, toolName: string, args: Record): Promise; + handleScriptToolCall(agentName: string, toolName: string, args: Record): Promise; }; it('gets the text history stores, with each reference as its stub and the rest as JSON writes it', async () => { const fw = Object.create(AgentFramework.prototype) as ScriptFramework; const shot = { type: 'image', data: BASE64, mimeType: 'image/png' }; fw.dispatchScriptToolCall = async () => ({ success: true, data: { file: RESOURCE, shot, page: 2 } }); - const text = await fw.handleScriptToolCall('zz-agent', 'zz_tool', {}); + const { text } = await fw.handleScriptToolCall('zz-agent', 'zz_tool', {}); assert.ok(!text.includes(SIGNATURE), text); const result = parsed(text) as { file: string; shot: unknown; page: number }; assert.match(result.file, STUB); @@ -331,7 +332,7 @@ describe('a script given such data', () => { console.error = (...args: unknown[]) => { logged.push(args.map(String).join(' ')); }; let text: string; try { - text = await fw.handleScriptToolCall('zz-agent', 'zz_tool', {}); + ({ text } = await fw.handleScriptToolCall('zz-agent', 'zz_tool', {})); } finally { console.error = error; } diff --git a/test/unconvertible-tool-result.test.ts b/test/unconvertible-tool-result.test.ts index 5b0497fa5..af4e06216 100644 --- a/test/unconvertible-tool-result.test.ts +++ b/test/unconvertible-tool-result.test.ts @@ -13,7 +13,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { AgentFramework } from '../src/index.js'; -import type { EventResponse, Module, ModuleContext, ProcessEvent, ProcessState, ToolCall, ToolDefinition, ToolResult } from '../src/index.js'; +import type { EventResponse, Module, ModuleContext, ProcessEvent, ProcessState, ScriptToolResult, ToolCall, ToolDefinition, ToolResult } from '../src/index.js'; import { MockMembrane, createMockResponse } from './helpers/mock-membrane.js'; function cyclic(): Record { @@ -42,6 +42,8 @@ const RESULTS: Record unknown> = { errerror: () => ({ success: false, isError: true, error: new Error('db timeout') }), nullerror: () => ({ success: false, isError: true, error: null }), quietfail: () => ({ success: false, error: 'unknown channel: c1', isError: false }), + // An unflagged failure whose own text starts like an error. + prefixquiet: () => ({ success: false, error: 'Error: quota is fine', isError: false }), noresult: () => undefined, nullresult: () => null, arrayresult: () => [], @@ -275,15 +277,45 @@ test('a script reads a quiet failure as its error text, and an unusable result a // A script's call is answered through the event loop, so it has to run. framework.start(); const script = (tool: string) => (framework as unknown as { - handleScriptToolCall(agent: string, tool: string, args: Record): Promise; + handleScriptToolCall(agent: string, tool: string, args: Record): Promise; }).handleScriptToolCall('assistant', `odd--${tool}`, {}); - assert.equal(await within(script('quietfail'), 10_000, 'the call'), 'unknown channel: c1'); - assert.equal(await within(script('nulldataquiet'), 10_000, 'the call'), 'gone'); - assert.equal(await within(script('cyclic'), 10_000, 'the call'), `Error: ${unconvertible('cyclic', 'Converting circular structure to JSON')}`); - assert.equal(await within(script('getterthrows'), 10_000, 'the call'), `Error: ${unreadable('getterthrows', 'a value with no text form')}`); + // A failure the tool doesn't flag reads as the model reads it, isError + // false: the script's .is_error follows the result's flag, not its text. + assert.deepEqual(await within(script('quietfail'), 10_000, 'the call'), { text: 'unknown channel: c1', isError: false }); + assert.deepEqual(await within(script('nulldataquiet'), 10_000, 'the call'), { text: 'gone', isError: false }); + // Text that only starts like an error isn't flagged as one: the flag is + // the result's, not the text's prefix. + assert.deepEqual(await within(script('prefixquiet'), 10_000, 'the call'), { text: 'Error: quota is fine', isError: false }); + assert.deepEqual(await within(script('cyclic'), 10_000, 'the call'), { text: `Error: ${unconvertible('cyclic', 'Converting circular structure to JSON')}`, isError: true }); + assert.deepEqual(await within(script('getterthrows'), 10_000, 'the call'), { text: `Error: ${unreadable('getterthrows', 'a value with no text form')}`, isError: true }); await within(framework.stop(), 10_000, 'stop()'); }); +test("a result the script's serializer can't convert reaches the script as an error, flagged as one", async (t) => { + // The dispatch's own check (storableToolResult) turns such a result into an + // error first, so these returns are reached only past it: here, with the + // dispatch answering a cyclic result directly. + const { framework } = await setUp(t, []); + const internals = framework as unknown as { + dispatchScriptToolCall: () => Promise; + handleScriptToolCall(agent: string, tool: string, args: Record): Promise; + }; + const reason = 'Converting circular structure to JSON'; + internals.dispatchScriptToolCall = async () => ({ success: true, data: cyclic() }); + const text = await internals.handleScriptToolCall('assistant', 'odd--cyclic', {}); + assert.equal(text.isError, true); + assert.equal(text.structuredJson, undefined); + assert.ok(text.text.startsWith("Error: tool 'odd--cyclic' returned a result that can't be converted to JSON, so the script can't receive it"), text.text); + assert.ok(text.text.includes(reason), text.text); + internals.dispatchScriptToolCall = async () => ({ success: true, structured: cyclic() }); + const structured = await internals.handleScriptToolCall('assistant', 'odd--cyclic', {}); + assert.equal(structured.isError, true); + assert.equal(structured.structuredJson, true); + const object = JSON.parse(structured.text) as { content: unknown[]; structuredContent: unknown; isError: boolean; error: string }; + assert.deepEqual({ ...object, error: undefined }, { content: [], structuredContent: null, isError: true, error: undefined }); + assert.ok(object.error.includes(reason), object.error); +}); + test("the read_image tool's own handler gets the same check", async (t) => { const { framework, membrane, logged } = await setUp(t, ['read_image'], [new OddWorkspace()]); framework.pushEvent({ type: 'external-message', source: 'test', content: 'Look.', metadata: {} });