diff --git a/changelog.d/mcpl-agent-words.changed.md b/changelog.d/mcpl-agent-words.changed.md new file mode 100644 index 000000000..98b2a6228 --- /dev/null +++ b/changelog.d/mcpl-agent-words.changed.md @@ -0,0 +1,11 @@ +- The MCPL texts an agent reads for a failed request, a refused tool call and a block the host can't show now say what happened, whether anything took effect, and what works from there, or that nothing does. Method names, request ids, JSON-RPC codes, close details and field names leave them. The ones an operator needs go to the host's log. + - **A failed MCPL request:** + - A connection that closes with a call pending reads `The connection to the "" server closed before it answered, so whether the request took effect is unknown.` It then says whether this host is reconnecting, where it read `MCPL server "" disconnected unexpectedly (code=…, signal=…, reason=…) while awaiting (id=N)`. + - The other texts change the same way: a close by this host, a request on a closed connection or one the transport couldn't write, a server's error answer (in the server's own words, or, with none, saying the outcome is unknown) and a timeout. + - The log gains a line for a close with requests pending, with the close's code, signal and reason, whether this host closed it, and the requests left unanswered; one for a server's error answer, with its method, id and code; one for a timeout, with its method and id; and one for a request the transport couldn't write, with its method, id and the transport's words. + - **The framework's MCPL tool calls:** + - A call to a server missing from the registry, or to a tool its policy denies, says it didn't run and whether anything can change that, in the same words for the model's call and a script's. One made while this host is connecting to the server, in a runtime connect or a restart's, says so and to try again in a moment. + - An error result with no text, or only whitespace, reads as one whose effect is unknown, where it read `Tool call failed`, or the blank text. + - A conversation's `channel_open` is told where to reply. + - **Blocks the host can't show** read as what they were and why: `[Image not shown: …]`, `[Audio not shown: …]`, `[Reference not shown: …]`, `[Content not shown: …]`, and the text marker `[Text not shown: a text part arrived without readable text.]`. They had read `[Image: no data]`, `[unrecognized content block: …]`, `[inline content withheld: …]` and `[Text not shown: the server's text block had no string text.]`. An image or audio block with an empty link and no data reads as the no-data marker on the tool-result lane too. + - **A reference's expiry and missing server** read `expired`, `treated as expired, since its expiry time can't be read`, `treated as expired, since its expiry time isn't in a form this host accepts` and `not tied to any server, so there's nowhere to fetch it from`. They had read `expired: its expiresAt has passed`, `… (fail closed)` and `unbound: no server is recorded to fetch it from`. diff --git a/changelog.d/mcpl-connect-in-flight.fixed.md b/changelog.d/mcpl-connect-in-flight.fixed.md new file mode 100644 index 000000000..f972c8ff0 --- /dev/null +++ b/changelog.d/mcpl-connect-in-flight.fixed.md @@ -0,0 +1 @@ +- A runtime connect, a restart or a disconnect of an MCPL server whose connect is still in flight is now refused before it changes anything, with `MCPL server "" has a connect in flight; try again once it settles`. Nothing stops a connect in flight. A second connect went ahead beside it, and whichever settled last replaced the other in the registry without closing it. A disconnect removed the server's config and tool prefix, and the connect then registered the server without them, where no call reached it and only another disconnect cleared it. A restart begins with that disconnect. diff --git a/src/framework.ts b/src/framework.ts index ebb8edddb..a430f6b40 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -142,6 +142,43 @@ import { splitProseRuns, withoutRoundsProse } from './prose-segments.js'; import { cumulativeDelta } from './usage-accounting.js'; import { stampThinkingTokenEstimates } from './thinking-token-stamp.js'; +/** An MCPL tool call refused because its server isn't in the registry: one + * closed for good, one removed, or one whose connect failed without + * reconnect, which this host doesn't bring back on its own. A server that's + * reconnecting stays registered, as does the stub a failed connect with + * reconnect leaves, and its closed connection answers the call instead. One + * this host is connecting to isn't registered yet, and + * mcplServerConnectingText answers for it. */ +function mcplServerGoneText(serverId: string, toolName: string): string { + return `The "${serverId}" server that provides ${toolName} isn't connected, and this host won't reconnect to it on its own, ` + + "so the call didn't run. If you need it, ask whoever runs this host."; +} + +/** An MCPL tool call refused while this host connects to its server, in a + * runtime connect or a restart's: the server's prefix routes calls to it + * before the registry holds it, which it does once the handshake is done. */ +function mcplServerConnectingText(serverId: string, toolName: string): string { + return `This host is connecting to the "${serverId}" server that provides ${toolName}, so the call didn't run. ` + + 'Try again in a moment.'; +} + +/** An MCPL tool call its server's tool policy denies. The policy hides the + * tool from the list it's offered in, so this is a call to a tool the agent + * wasn't offered. */ +function mcplToolDeniedText(serverId: string, toolName: string): string { + return `${toolName} isn't allowed by this host's policy for the "${serverId}" server, so the call didn't run, ` + + "and calling it again won't change that. If you need it, ask whoever runs this host."; +} + +/** A runtime connect, restart or disconnect refused because a connect of the + * server is in flight (mcplConnecting). The registry takes a server only once + * its handshake is done, and nothing can stop that connect meanwhile, so it + * would register the server after whatever ran beside it. Whoever asked reads + * it: an operator, or an agent through a host's tool that passes it on. */ +function mcplConnectInFlightError(serverId: string): Error { + return new Error(`MCPL server "${serverId}" has a connect in flight; try again once it settles`); +} + /** Detect a supported image media type from magic bytes (the model API * rejects mislabeled media types, so trust bytes over extensions). * Returns undefined for non-image content. */ @@ -340,13 +377,23 @@ function mcplResultText(content: McpToolResultContent[] | undefined): string { /** * An MCPL tools/call result the server marked `isError`, as a failure: its - * text is the error, or `Tool call failed` when it has none, and its other - * blocks aren't kept. The model's dispatch and an in-process caller - * (`executeToolCall`, `ModuleContext.callTool`) both take a server's error - * from here, so it reads the same on each path. + * text is the error, and with none, or only whitespace, as a server's error + * answer reads it, the error says the server gave no words and that whether + * the call did anything is unknown. Its other blocks aren't kept. The model's + * dispatch and an in-process caller (`executeToolCall`, + * `ModuleContext.callTool`) both take a server's error from here, so it reads + * the same on each path. `callName` is the tool's name as it was called. */ -function mcplToolFailure(content: McpToolResultContent[] | undefined): ToolResult { - return { success: false, error: mcplResultText(content) || 'Tool call failed', isError: true }; +function mcplToolFailure(content: McpToolResultContent[] | undefined, serverId: string, callName: string): ToolResult { + const said = mcplResultText(content); + return { + success: false, + error: said.trim() + ? said + : `The "${serverId}" server reported an error for ${callName} without saying what went wrong. ` + + 'Whether it did anything is unknown: check before calling it again.', + isError: true, + }; } /** @@ -2358,6 +2405,13 @@ export class AgentFramework { private toolPatternKnownPrefixes: Map = new Map(); /** Maps serverId → McplServerConfig for prefix lookup. */ private mcplServerConfigs: Map = new Map(); + /** Servers whose connect is in flight (connectMcplServerInternal), from the + * registry's handshake until it settles: a call their prefix routes then + * finds no server registered, and is told this host is connecting to it. + * One mark per id: the registry holds one connection per id, and a + * second connect, a restart or a disconnect of an id whose connect is in + * flight is refused (refuseMcplConnect, runDisconnectMcplServer). */ + private mcplConnecting: Set = new Set(); /** Host capabilities advertised during the MCP handshake — stored so servers * can be connected at runtime (connectMcplServer) after initialization. */ private mcplHostCapabilities: McplHostCapabilities | null = null; @@ -17677,22 +17731,18 @@ export class AgentFramework { } const server = this.mcplServerRegistry.getServer(serverId); if (!server) { - return { success: false, error: `MCPL server ${serverId} not found`, isError: true }; + return { success: false, error: this.mcplServerAbsentText(serverId, call.name), isError: true }; } const toolName = call.name.slice(prefix.length + 2); // Strip prefix + '--' if (!isToolAllowed(toolName, this.mcplServerConfigs.get(serverId))) { - return { - success: false, - error: `Tool '${call.name}' is not permitted by this server's tool policy.`, - isError: true, - }; + return { success: false, error: mcplToolDeniedText(serverId, call.name), isError: true }; } try { const result = await server.sendToolsCall(toolName, call.input as Record); // A server's error is a failure here too, as the model's dispatch // reads it. A success is the server's own blocks, for the caller to // use in process. - if (result.isError) return mcplToolFailure(result.content); + if (result.isError) return mcplToolFailure(result.content, serverId, call.name); // RFC-005 §6.2: register the result's references with this server, so // a stub later made of them is bound and fetch_reference can fetch // them through this server's address, where it has one. Nothing is @@ -21291,7 +21341,18 @@ export class AgentFramework { }), }); } - const connection = await this.mcplServerRegistry.addServer(connectionConfig, this.mcplHostCapabilities); + // The prefix above routes this server's tool calls already, and addServer + // registers the server only once its handshake is done, so until it + // settles a call is told this host is connecting to it. A connect that + // fails without reconnect leaves it unregistered, and a call is told it's + // gone; with reconnect, the stub it leaves is registered and answers. + this.mcplConnecting.add(config.id); + let connection: McplServerConnection; + try { + connection = await this.mcplServerRegistry.addServer(connectionConfig, this.mcplHostCapabilities); + } finally { + this.mcplConnecting.delete(config.id); + } // Wire listeners before either startup staging or the runtime global gate // releases control traffic needed for registration and marker service. @@ -21835,16 +21896,22 @@ export class AgentFramework { /** * Refuse a runtime connect, or a restart, before it writes anything: an id - * already registered, or a tool prefix that equals a module name or another - * server's prefix, or meets another server's (prefixesMeet, #265). create() - * applies the same prefix rules at startup. The connecting id's own entries - * don't count, nor, for a restart, those of the server it replaces - * (`replacing`), which it disconnects first. + * whose connect is in flight or that's already registered, or a tool prefix + * that equals a module name or another server's prefix, or meets another + * server's (prefixesMeet, #265). create() applies the same prefix rules at + * startup. The connecting id's own entries don't count, nor, for a restart, + * those of the server it replaces (`replacing`), which it disconnects first. */ private refuseMcplConnect( config: import('./mcpl/types.js').McplServerConfig, replacing?: string, ): void { + // A connect in flight isn't registered until its handshake is done, so + // the registry's check below misses it. Its id is taken whether this + // connects it or a restart replaces it: the restart's disconnect can't + // stop a connect in flight, which would register the server after it. + const inFlight = [config.id, replacing].find((id) => id !== undefined && this.mcplConnecting.has(id)); + if (inFlight !== undefined) throw mcplConnectInFlightError(inFlight); // The registry refuses a second server under one id too, but only once // the connect has written its config and prefix over the live server's. if (config.id !== replacing && this.mcplServerRegistry?.getServer(config.id)) { @@ -21884,6 +21951,10 @@ export class AgentFramework { if (!this.mcplServerRegistry) { throw new Error('MCPL subsystem is not initialized'); } + // A connect in flight would register the server after this disconnect, + // with its config and prefix gone, where only another disconnect reaches + // it; refused as a second connect of it is (refuseMcplConnect). + if (this.mcplConnecting.has(id)) throw mcplConnectInFlightError(id); const config = this.mcplServerConfigs.get(id); const oldToolNames = new Set(this.mcplTools.map(t => t.name)); @@ -22850,6 +22921,14 @@ export class AgentFramework { } } + /** The refusal of an MCPL tool call whose server isn't in the registry, for + * the model's call and a script's alike. */ + private mcplServerAbsentText(serverId: string, toolName: string): string { + return this.mcplConnecting.has(serverId) + ? mcplServerConnectingText(serverId, toolName) + : mcplServerGoneText(serverId, toolName); + } + /** * Dispatch a tool call to an MCPL server. * Strips the configured toolPrefix and routes to the server. @@ -22867,7 +22946,7 @@ export class AgentFramework { toolName: call.name, agentName, moduleName: `mcpl:${serverId}`, - result: { success: false, error: `MCPL server not found: ${serverId}`, isError: true }, + result: { success: false, error: this.mcplServerAbsentText(serverId, call.name), isError: true }, }); return; } @@ -22882,11 +22961,7 @@ export class AgentFramework { toolName: call.name, agentName, moduleName: `mcpl:${serverId}`, - result: { - success: false, - error: `Tool '${call.name}' is not permitted by this server's tool policy.`, - isError: true, - }, + result: { success: false, error: mcplToolDeniedText(serverId, call.name), isError: true }, }); return; } @@ -22964,7 +23039,7 @@ export class AgentFramework { // string for backward compatibility with callers that expect data to // be string-ish. const converted: ToolResult = result.isError - ? mcplToolFailure(result.content) + ? mcplToolFailure(result.content, serverId, call.name) : { success: true, data: result.content?.some((c) => c.type !== 'text') @@ -23097,7 +23172,7 @@ export class AgentFramework { }; if (call.name === 'channel_open') { - reject(`This conversation is bound to channel ${home}; conversation agents cannot open channels.`); + reject(`This conversation is bound to channel "${home}", so it can't open other channels, and nothing was opened. Reply in "${home}".`); return; } if (call.name === 'channel_publish' || call.name === 'channel_close') { diff --git a/src/mcpl/channel-registry.ts b/src/mcpl/channel-registry.ts index d46c150c4..067fce3bf 100644 --- a/src/mcpl/channel-registry.ts +++ b/src/mcpl/channel-registry.ts @@ -469,7 +469,12 @@ export interface PublishOutcome { * a failed request this way. */ function publishErrorOutcome(err: unknown, destination: PublishDestination, at: number): PublishOutcome { - const message = connectorText(err instanceof Error ? err.message : String(err)); + // This host's own words for a request that wasn't sent or wasn't answered + // are sentences, and every composition of the reason adds its own ending, + // so one final period comes off them. A server's error answer keeps its + // words as they came. + const text = connectorText(err instanceof Error ? err.message : String(err)); + const message = err instanceof McplRequestError && err.outcome !== 'error-response' ? text.replace(/(? { return typeof x === 'object' && x !== null && !Array.isArray(x); @@ -80,13 +83,13 @@ function takeString( export function classifyBlock(block: unknown): BlockClassification { if (!isRecord(block) || typeof block.type !== 'string') { - return { kind: 'invalid', reason: 'not a content block' }; + return { kind: 'invalid', shown: unknownBlockText(block) }; } switch (block.type) { case 'text': return typeof block.text === 'string' ? { kind: 'text' } - : { kind: 'invalid', reason: 'text block without string text' }; + : { kind: 'invalid', shown: NON_STRING_TEXT_PLACEHOLDER }; case 'image': case 'audio': { const hasData = typeof block.data === 'string'; @@ -94,32 +97,30 @@ export function classifyBlock(block: unknown): BlockClassification { if (hasData) { // Inline form claiming bulk disposition (or smuggling a uri beside // the data) is the vector-2 contradiction: fail closed, withhold. + // A uri of '' counts here too, though it's no link: until the + // tool-result lane gives every inline shape a marker of its own, + // showing that data would put some of it into history as raw JSON. + // Once that lane does, an empty link beside data reads as no link. return { kind: 'inline', contradiction: block.disposition !== undefined || hasUri }; } - if (hasUri) return parseReferenceFields(block); - return { kind: 'invalid', reason: `${block.type} block ${mediaWithoutStringReason(block)}` }; + // With no data, an empty uri is no link, as the push lane's converter + // reads it. + if (hasUri && block.uri !== '') return parseReferenceFields(block); + return { kind: 'invalid', shown: block.type === 'image' ? IMAGE_NO_DATA_TEXT : AUDIO_NO_DATA_TEXT }; } case 'resource': return parseReferenceFields(block); default: - return { kind: 'invalid', reason: `unknown block type: ${block.type}` }; + return { kind: 'invalid', shown: unknownBlockText(block) }; } } -/** What a media block with neither string data nor a string uri has instead: - * nothing, or a field that isn't a string. */ -function mediaWithoutStringReason(block: Record): string { - const notString = (['data', 'uri'] as const).filter((f) => block[f] !== undefined && block[f] !== null); - if (notString.length === 0) return 'with neither data nor uri'; - return notString.length === 2 ? "whose data and uri aren't strings" : `whose ${notString[0]} isn't a string`; -} - function parseReferenceFields(block: Record): BlockClassification { if (typeof block.uri !== 'string' || block.uri.length === 0) { - return { kind: 'invalid', reason: 'reference without string uri' }; + return { kind: 'invalid', shown: REFERENCE_NO_LINK_TEXT }; } if (block.uri.length > REFERENCE_LIMITS.uri) { - return { kind: 'invalid', reason: 'uri exceeds schema maximum' }; + return { kind: 'invalid', shown: REFERENCE_LINK_TOO_LONG_TEXT }; } const out = { rejected: [] as string[], truncated: [] as string[] }; const t: ReferenceTestimony = { uri: block.uri, rejectedFields: out.rejected, truncatedFields: out.truncated }; @@ -152,7 +153,38 @@ export function isInlineContradiction(block: unknown): boolean { } export const INLINE_WITHHELD_TEXT = - '[inline content withheld: inline data that also claims a uri or a bulk disposition]'; + '[Content not shown: its inline data arrived marked as a reference too, so this host withheld it.]'; + +/* + * What the model-visible lanes (push, channel, context injection and tool + * results) put in place of a block they can't show as itself. Each says what + * kind of thing was there and why it can't be seen, in the host's words, so + * the agent doesn't read the content as complete; there's nothing for the + * agent to do about any of them. The text marker is NON_STRING_TEXT_PLACEHOLDER + * (visible-content.ts). + */ +export const IMAGE_NO_MEDIA_TYPE_TEXT = '[Image not shown: its data arrived without a media type.]'; +export const IMAGE_NO_DATA_TEXT = '[Image not shown: it arrived with no usable data or link.]'; +export const AUDIO_NO_MEDIA_TYPE_TEXT = '[Audio not shown: its data arrived without a media type.]'; +/** On the push, channel and context-injection lanes, audio shows only as + * inline data, so a plain link isn't enough either. (A tool result's audio + * with a link is a reference, as any block with one is there.) */ +export const AUDIO_NO_DATA_TEXT = '[Audio not shown: it arrived with no usable inline data.]'; +export const REFERENCE_NO_LINK_TEXT = '[Reference not shown: it arrived without a link.]'; +export const REFERENCE_LINK_TOO_LONG_TEXT = '[Reference not shown: its link is longer than this host accepts.]'; +const UNREADABLE_BLOCK_TEXT = "[Content not shown: a part arrived in a form this host can't read.]"; + +/** A block of a type the host doesn't show, of no type at all, or no block. + * The type is the sender's word, so it's shown as a label is: bounded, with + * its controls removed. One that isn't a string is no type, and is never + * converted to one. */ +export function unknownBlockText(block: unknown): string { + if (!isRecord(block)) return UNREADABLE_BLOCK_TEXT; + const label = typeof block.type === 'string' ? sanitizeLabel(block.type, STUB_FIELD_CHARS) : ''; + return label + ? `[Content not shown: this host can't display "${label}" content.]` + : '[Content not shown: a part arrived with no type.]'; +} /** * Why a reference counts as expired, or null when it doesn't: its expiresAt @@ -177,17 +209,18 @@ export function isReferenceExpired(t: Pick `\n stderr: ${logDetail(line)}`).join(''); } -/** A transport close's reason, as an error message gives it: the transport's - * own words. A WebSocket peer's close reason is the server's text - * (`reasonFromPeer`), so an error gives the words the transport uses for a - * close without one, as `mcpl:server-closed` leaves that text out. A +/** A transport close's reason, as the host's log and the handshake's error + * give it: the transport's own words. A WebSocket peer's close reason is the + * server's text (`reasonFromPeer`), so they give the words the transport uses + * for a close without one, as `mcpl:server-closed` leaves that text out. A * WebSocket close's code, a number, says what the server meant (4001: a - * refused credential), with or without a reason, so each error gives it too: - * a pending request's error has `code=`, and the handshake's adds it for any - * WebSocket close, since a close before the handshake completes reaches no - * 'close' event. A stdio child's exit reads as before. */ + * refused credential), with or without a reason, so each gives it too: the + * log line for a close with requests pending has `code=`, and the + * handshake's error adds it for any WebSocket close, since a close before the + * handshake completes reaches no 'close' event. A stdio child's exit reads + * as before. */ function closeReasonText(info: TransportCloseInfo): string { if (info.reasonFromPeer) return 'websocket closed'; return info.reason ?? 'unknown'; @@ -311,6 +312,10 @@ export class McplServerConnection extends EventEmitter { /** Set by `reconnectAfterFailure()` before it closes a live transport, so * the transport close handler reports that teardown as `byHost: true`. */ private hostCloseRequested = false; + /** Set by `close()`: this host has stopped the connection, as a removal and + * a restart both do, so a request after it can't say whether the server + * comes back. */ + private closedByThisHost = false; private closePromise: Promise | null = null; private closeIncomplete = false; private reconnectPromise: Promise | null = null; @@ -1063,10 +1068,18 @@ export class McplServerConnection extends EventEmitter { // Lifecycle // ========================================================================== + /** The requests still waiting for an answer, for the operator's log line + * when the connection goes: their methods and ids. */ + private unansweredRequests(): string { + const list = [...this.pendingRequests].map(([id, pending]) => `${pending.method} (id=${id})`).join(', '); + return `${this.pendingRequests.size} request(s) unanswered: ${list}`; + } + /** Close the connection: disable reconnect, kill the child process, and clean up resources. */ async close(): Promise { // Disable reconnect before closing — explicit close means stop retrying this.reconnectEnabled = false; + this.closedByThisHost = true; if (this.reconnectTimer) { clearTimeout(this.reconnectTimer); this.reconnectTimer = null; @@ -1078,10 +1091,22 @@ export class McplServerConnection extends EventEmitter { this.closeIncomplete = true; this.resetPolicyForTransportBoundary(); - // Reject all pending requests - for (const [id, pending] of this.pendingRequests) { + // Reject all pending requests. Each message is the whole of what an + // agent reads for a tools/call, and what follows a channel tool's own + // words ("Failed to publish to channel: …") for the requests a channel + // tool makes, so it names no method; the log line does. It doesn't say + // whether this host reconnects: a restart closes the connection and then + // connects again. + if (this.pendingRequests.size > 0) { + console.error(`MCPL server "${this.id}": this host closed its connection with ${this.unansweredRequests()}`); + } + for (const pending of this.pendingRequests.values()) { if (pending.timer) clearTimeout(pending.timer); - pending.reject(new McplRequestError(`Connection to MCPL server "${this.id}" closed while awaiting response for ${pending.method} (id=${id})`, 'no-response')); + pending.reject(new McplRequestError( + `The "${this.id}" server didn't answer before this host closed its connection, so whether the request took effect is unknown. ` + + 'Check its effect before trying again.', + 'no-response', + )); } this.pendingRequests.clear(); this.orphanedRequests.clear(); @@ -1257,7 +1282,19 @@ export class McplServerConnection extends EventEmitter { options: RequestOptions = {}, ): Promise { if (this.closed) { - return Promise.reject(new McplRequestError(`Cannot send request: connection to "${this.id}" is closed`, 'not-sent')); + // After this host's own close, which a restart makes before connecting + // again and a removal makes for good, only that this host closed it + // holds, and one next step holds for both: calling again reaches the + // restarted server, or fails where it's gone. + return Promise.reject(new McplRequestError(this.closedByThisHost + ? `The request wasn't sent: this host closed its connection to the "${this.id}" server, so nothing ran. ` + + 'If calling it again fails, ask whoever runs this host.' + : `The request wasn't sent: the connection to the "${this.id}" server is closed, so nothing ran. ` + + (this.willReconnect + ? 'This host is reconnecting to it; try again later.' + : "This host won't reconnect to it on its own. If you need it, ask whoever runs this host."), + 'not-sent', + )); } const id = this.nextRequestId++; @@ -1290,13 +1327,17 @@ export class McplServerConnection extends EventEmitter { }); if (this.orphanedRequests.size > 4096) this.orphanedRequests.delete(this.orphanedRequests.keys().next().value!); } + // The method and id are the operator's, so they go to the log. + console.error( + `MCPL server "${this.id}" did not respond to ${method} (id=${id}) within ${timeoutMs}ms; ` + + 'the request was abandoned here, not cancelled', + ); + const seconds = timeoutMs / 1000; reject(new McplRequestError( - `MCPL server "${this.id}" did not respond to ${method} (id=${id}) ` + - `within ${timeoutMs}ms — the server may be hung. The response ` + - `outcome is unknown: the request was abandoned locally but was not ` + - `cancelled, so the tool may still have completed server-side; verify ` + - `state before retrying; a blind retry of a stateful/side-effecting ` + - `tool may duplicate it.`, + `The "${this.id}" server didn't answer within ${seconds} second${seconds === 1 ? '' : 's'}, ` + + 'so whether the request took effect is unknown: ' + + 'it was abandoned here, not cancelled, and may still complete on the server. ' + + 'Check its effect before trying again, since repeating something that changes state may do it twice.', 'no-response', )); }, timeoutMs); @@ -1308,11 +1349,15 @@ export class McplServerConnection extends EventEmitter { this.transport!.writeLine(JSON.stringify(request)); } catch (err) { // A synchronous write rejection sent no request. Remove its pending - // state and watchdog before reporting that it is safe to retry. + // state and watchdog before reporting that it is safe to retry. The + // method, id and the transport's words are the operator's. this.pendingRequests.delete(id); if (pending.timer) clearTimeout(pending.timer); + console.error( + `MCPL server "${this.id}": writing ${method} (id=${id}) failed: ${logDetail(err instanceof Error ? err.message : err)}`, + ); reject(new McplRequestError( - `Cannot send ${method} to MCPL server "${this.id}": ${err instanceof Error ? err.message : String(err)}`, + `The request wasn't sent: this host couldn't write it to the "${this.id}" server, so nothing ran. Try again later.`, 'not-sent', )); } @@ -1622,20 +1667,24 @@ export class McplServerConnection extends EventEmitter { if (pending.timer) clearTimeout(pending.timer); if (response.error) { - // describeCause rather than the template's own conversion, as for an - // initialize error: the server's code and message can be any JSON - // value, and one with no primitive form, such as {"toString":1}, would - // throw here, after the request has left pendingRequests and its timer - // is cleared, so it would never settle. Both are the server's text, a - // code as much as a message, since a server can send a string, so both - // are bounded as errors.ts says. - pending.reject( - new McplRequestError( - `MCPL server "${this.id}" returned error for ${pending.method}: [${boundedForLog(describeCause(response.error.code))}] ${boundedForLog(describeCause(response.error.message))}`, - 'error-response', - (response.error as { data?: unknown }).data, - ), + // The server's own words, after a word on whose they are, bounded as + // errors.ts says, since they're the server's text. Only a string is + // words: a message of any other JSON value reads as none. The method, + // id and JSON-RPC code are the operator's, so they go to the log, which + // prints any value a server can send without throwing (logValue). + const said = typeof response.error.message === 'string' ? boundedForLog(response.error.message.trim()) : ''; + console.error( + `MCPL server "${this.id}" answered ${pending.method} (id=${response.id}) with error ${logValue(response.error.code)}: ` + + logValue(response.error.message), ); + pending.reject(new McplRequestError( + said + ? `The "${this.id}" server answered with an error: ${said}` + : `The "${this.id}" server answered with an error without saying what went wrong, so whether the request took effect is unknown. ` + + 'Check its effect before trying again.', + 'error-response', + (response.error as { data?: unknown }).data, + )); } else { pending.resolve(response.result); } @@ -1683,13 +1732,29 @@ export class McplServerConnection extends EventEmitter { if (!this.closed) { this.closed = true; this.resetPolicyForTransportBoundary(); + // A teardown the host asked for via reconnectAfterFailure() arrives + // here too; it's reported as such below. + const byHost = this.hostCloseRequested; + this.hostCloseRequested = false; - // Reject all pending requests - for (const [id, pending] of this.pendingRequests) { + // Reject all pending requests. The words hold whether the server + // dropped or this host closed the transport to recover; how it closed + // is the operator's, so it goes to the log, not into the messages. + if (this.pendingRequests.size > 0) { + // Whatever the host asked for, the close's own details are kept, so + // a drop that races a recycle loses nothing. + const how = `${byHost ? 'closed by this host, ' : ''}` + + `code=${info.code ?? 'n/a'}, signal=${info.signal ?? 'n/a'}, reason=${closeReasonText(info)}`; + console.error(`MCPL server "${this.id}" disconnected (${how}) with ${this.unansweredRequests()}`); + } + for (const pending of this.pendingRequests.values()) { if (pending.timer) clearTimeout(pending.timer); pending.reject( new McplRequestError( - `MCPL server "${this.id}" disconnected unexpectedly (code=${info.code ?? 'n/a'}, signal=${info.signal ?? 'n/a'}, reason=${closeReasonText(info)}) while awaiting ${pending.method} (id=${id})`, + `The connection to the "${this.id}" server closed before it answered, so whether the request took effect is unknown. ` + + (this.reconnectEnabled + ? 'Check its effect before trying again; this host is reconnecting to it.' + : "This host won't reconnect to it on its own. If you need it, ask whoever runs this host."), 'no-response', ), ); @@ -1699,10 +1764,7 @@ export class McplServerConnection extends EventEmitter { // Forward the transport's close info so listeners can tell a server // that refused the credential (WebSocket 4001) from a crash, and act - // before the reconnect below redials. A teardown the host asked for - // via reconnectAfterFailure() arrives here too; report it as such. - const byHost = this.hostCloseRequested; - this.hostCloseRequested = false; + // before the reconnect below redials. const closeInfo: McplConnectionCloseInfo = { transport: transport.kind, code: info.code ?? null, diff --git a/src/mcpl/visible-content.ts b/src/mcpl/visible-content.ts index 0f6ba545c..ed63e349b 100644 --- a/src/mcpl/visible-content.ts +++ b/src/mcpl/visible-content.ts @@ -36,8 +36,9 @@ export function isVisiblyEmptyContent(content: readonly unknown[] | null | undef * block whose text isn't a string, which only a nonconformant sender writes, * so the agent learns that a block was there. It is the host's word, not the * sender's, so it never makes content visible (isVisiblyEmptyConversion). + * The other blocks the converters can't show have theirs in references.ts. */ -export const NON_STRING_TEXT_PLACEHOLDER = "[Text not shown: the server's text block had no string text.]"; +export const NON_STRING_TEXT_PLACEHOLDER = '[Text not shown: a text part arrived without readable text.]'; /** A wire text block whose text isn't a string, which the converter shows as * NON_STRING_TEXT_PLACEHOLDER. */ diff --git a/test/conversation-routing.test.ts b/test/conversation-routing.test.ts index 7fa50a936..95f23412e 100644 --- a/test/conversation-routing.test.ts +++ b/test/conversation-routing.test.ts @@ -975,7 +975,7 @@ describe('Conversation routing', () => { fw.dispatchChannelToolCall(fork, { id: 't2', name: 'channel_close', input: { channelId: 'slack:C8' } }); assert.equal(failures.length, 2, 'both foreign channel operations should be rejected'); - assert.ok(failures[0]!.error.includes('cannot open channels'), 'channel_open is rejected outright'); + assert.ok(failures[0]!.error.includes("so it can't open other channels, and nothing was opened"), 'channel_open is rejected outright'); assert.ok(failures[1]!.error.includes('closing slack:C8 is not allowed'), 'foreign channel_close is rejected'); await framework.stop(); diff --git a/test/mcpl-agent-words.test.ts b/test/mcpl-agent-words.test.ts new file mode 100644 index 000000000..e2aa88d72 --- /dev/null +++ b/test/mcpl-agent-words.test.ts @@ -0,0 +1,403 @@ +/** + * What the MCPL lanes tell an agent, from the error-wording sweep: an MCPL + * request that fails says whether it took effect, the framework's refusals of + * an MCPL tool call say whether the call ran and what can change that, and a + * block the host can't show says what kind of thing it was and why. + * + * Run: node --import tsx --test test/mcpl-agent-words.test.ts + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; + +import { AgentFramework } from '../src/framework.js'; +import { + AUDIO_NO_DATA_TEXT, + AUDIO_NO_MEDIA_TYPE_TEXT, + IMAGE_NO_DATA_TEXT, + IMAGE_NO_MEDIA_TYPE_TEXT, + INLINE_WITHHELD_TEXT, + REFERENCE_LIMITS, + REFERENCE_LINK_TOO_LONG_TEXT, + REFERENCE_NO_LINK_TEXT, + classifyBlock, + referenceStubOrNull, +} from '../src/mcpl/references.js'; +import { convertBlock } from '../src/mcpl/convert-block.js'; +import { toolResultDataToHistoryString } from '../src/tool-result-history.js'; +import { McplServerConnection } from '../src/mcpl/server-connection.js'; +import type { McplHostCapabilities, McplServerConfig } from '../src/mcpl/types.js'; + +// --------------------------------------------------------------------------- +// Blocks the host can't show +// --------------------------------------------------------------------------- + +test("the words that stand in for a block the host can't show, which other tests take from the constants", () => { + assert.deepEqual( + [IMAGE_NO_MEDIA_TYPE_TEXT, IMAGE_NO_DATA_TEXT, AUDIO_NO_MEDIA_TYPE_TEXT, AUDIO_NO_DATA_TEXT, REFERENCE_NO_LINK_TEXT, REFERENCE_LINK_TOO_LONG_TEXT, INLINE_WITHHELD_TEXT], + [ + '[Image not shown: its data arrived without a media type.]', + '[Image not shown: it arrived with no usable data or link.]', + '[Audio not shown: its data arrived without a media type.]', + '[Audio not shown: it arrived with no usable inline data.]', + '[Reference not shown: it arrived without a link.]', + '[Reference not shown: its link is longer than this host accepts.]', + '[Content not shown: its inline data arrived marked as a reference too, so this host withheld it.]', + ], + ); +}); + +test("a reference block that can't be read says which part is missing, and what isn't a block says it can't be read", () => { + assert.equal(referenceStubOrNull({ type: 'resource', uri: 'u'.repeat(REFERENCE_LIMITS.uri + 1) }), REFERENCE_LINK_TOO_LONG_TEXT); + assert.equal(referenceStubOrNull({ type: 'resource' }), REFERENCE_NO_LINK_TEXT); + assert.equal(referenceStubOrNull(424242), "[Content not shown: a part arrived in a form this host can't read.]"); + assert.equal(referenceStubOrNull({ type: 'hologram\u0007' }), `[Content not shown: this host can't display "hologram" content.]`); + assert.equal(referenceStubOrNull({ type: '\u0007' }), '[Content not shown: a part arrived with no type.]'); + assert.equal(referenceStubOrNull({ type: 42 }), '[Content not shown: a part arrived with no type.]'); + // An empty link is no link: the tool-result lane says what the push lane's + // converter says for the same block. + for (const [block, text] of [[{ type: 'image', uri: '' }, IMAGE_NO_DATA_TEXT], [{ type: 'audio', uri: '' }, AUDIO_NO_DATA_TEXT]] as const) { + assert.equal(referenceStubOrNull(block), text); + assert.deepEqual(convertBlock(block as never, 'test', 'words'), { type: 'text', text }); + } + // Beside inline data, any string uri, an empty one too, stays withheld as + // on main: the tool-result lane can't show every inline shape yet, and + // showing this audio would put its data into history as raw JSON. + const audio = { type: 'audio', data: 'aGVsbG8=', mimeType: 'audio/wav', uri: '' }; + assert.deepEqual(classifyBlock(audio), { kind: 'inline', contradiction: true }); + const stored = toolResultDataToHistoryString([{ type: 'text', text: 't' }, audio]); + assert.ok(stored.includes(INLINE_WITHHELD_TEXT) && !stored.includes('aGVsbG8='), stored); +}); + +// --------------------------------------------------------------------------- +// An MCPL request that fails +// --------------------------------------------------------------------------- + +// Stdio server: answers initialize and tools/list; tools/call "refuse" gets a +// JSON-RPC error, "mute" one with a blank message, "odd" one whose message +// isn't a string, "exit" ends the process with the call pending, +// "answer-then-exit" answers and then ends it, and "hang" is never answered. +const SERVER = ` +let buf = ''; +process.stdin.on('data', (c) => { + buf += c.toString('utf8'); + let i; + while ((i = buf.indexOf('\\n')) >= 0) { + const line = buf.slice(0, i); buf = buf.slice(i + 1); + if (!line.trim()) continue; + let m; try { m = JSON.parse(line); } catch { continue; } + const send = (o) => process.stdout.write(JSON.stringify({ jsonrpc: '2.0', id: m.id, ...o }) + '\\n'); + if (m.method === 'initialize') send({ result: { capabilities: {} } }); + else if (m.method === 'tools/list') send({ result: { tools: [] } }); + else if (m.method === 'tools/call' && m.params.name === 'refuse') send({ error: { code: -32000, message: 'Missing Access' } }); + else if (m.method === 'tools/call' && m.params.name === 'mute') send({ error: { code: -32000, message: ' ' } }); + else if (m.method === 'tools/call' && m.params.name === 'odd') send({ error: { code: -32000, message: 42 } }); + else if (m.method === 'tools/call' && m.params.name === 'answer-then-exit') { send({ result: { content: [] } }); setTimeout(() => process.exit(0), 50); } + else if (m.method === 'tools/call' && m.params.name === 'exit') process.exit(3); + } +}); +setInterval(() => {}, 1 << 30); +`; + +const HOST_CAPS: McplHostCapabilities = { version: '0.4', pushEvents: true, contextHooks: { beforeInference: true }, featureSets: true }; +const config = (overrides: Partial = {}): McplServerConfig => + ({ id: 'words', command: process.execPath, args: ['-e', SERVER], ...overrides }); + +/** `promise`, or a failure naming `what` after five seconds, so a regression fails rather than hangs. */ +async function within(promise: Promise, what: string): Promise { + let timer: ReturnType | undefined; + const late = new Promise((_, reject) => { timer = setTimeout(() => reject(new Error(`no ${what} within 5 s`)), 5000); }); + try { return await Promise.race([promise, late]); } finally { clearTimeout(timer); } +} + +/** What a server's error answer with no words reads as. */ +const NO_WORDS = 'The "words" server answered with an error without saying what went wrong, so whether the request took effect is unknown. ' + + 'Check its effect before trying again.'; + +/** Whether some console.error line matches. */ +const logged = (calls: ReadonlyArray<{ arguments: unknown[] }>, line: RegExp): boolean => + calls.some((call) => line.test(String(call.arguments[0]))); + +test("a server's error answer is shown as the server's, and its method, id and code go to the log", async (t) => { + const errors = t.mock.method(console, 'error', () => {}); + const connection = await McplServerConnection.connect(config(), HOST_CAPS); + t.after(() => connection.close()); + connection.ready(); + await assert.rejects(connection.sendToolsCall('refuse', {}), + { message: 'The "words" server answered with an error: Missing Access' }); + assert.ok(logged(errors.mock.calls, /^MCPL server "words" answered tools\/call \(id=\d+\) with error -32000: /), 'the operator log has the answer'); + // An answer with no words says so, rather than ending on a colon. + await assert.rejects(connection.sendToolsCall('mute', {}), + { message: NO_WORDS }); + // A message that isn't a string reads the same, and the log keeps what was sent. + await assert.rejects(connection.sendToolsCall('odd', {}), + { message: NO_WORDS }); + assert.ok(logged(errors.mock.calls, /^MCPL server "words" answered tools\/call \(id=\d+\) with error -32000: 42$/), 'the log has the number'); +}); + +test("a server that drops with a call pending says the outcome is unknown, and whether it's coming back", async (t) => { + const errors = t.mock.method(console, 'error', () => {}); + const once = await McplServerConnection.connect(config(), HOST_CAPS); + t.after(() => once.close()); + once.ready(); + await assert.rejects(once.sendToolsCall('exit', {}), { + message: 'The connection to the "words" server closed before it answered, so whether the request took effect is unknown. ' + + "This host won't reconnect to it on its own. If you need it, ask whoever runs this host.", + }); + // The close's details are the operator's: they go to the log. + assert.ok(logged(errors.mock.calls, /^MCPL server "words" disconnected \(code=.*\) with 1 request\(s\) unanswered: tools\/call \(id=\d+\)$/), + 'the operator log has the close'); + await assert.rejects(once.sendToolsCall('refuse', {}), { + message: `The request wasn't sent: the connection to the "words" server is closed, so nothing ran. This host won't reconnect to it on its own. If you need it, ask whoever runs this host.`, + }); + + const again = await McplServerConnection.connectWithReconnect(config({ reconnect: true, reconnectIntervalMs: 600_000 }), HOST_CAPS); + t.after(() => again.close()); + again.ready(); + await assert.rejects(again.sendToolsCall('exit', {}), { + message: 'The connection to the "words" server closed before it answered, so whether the request took effect is unknown. ' + + 'Check its effect before trying again; this host is reconnecting to it.', + }); + await assert.rejects(again.sendToolsCall('refuse', {}), { + message: `The request wasn't sent: the connection to the "words" server is closed, so nothing ran. This host is reconnecting to it; try again later.`, + }); +}); + +test('a call pending when this host recycles the connection reads as a closed connection, and the log says the host closed it', async (t) => { + const errors = t.mock.method(console, 'error', () => {}); + const connection = await McplServerConnection.connectWithReconnect(config({ reconnect: true, reconnectIntervalMs: 600_000 }), HOST_CAPS); + t.after(() => connection.close()); + connection.ready(); + const pending = assert.rejects(connection.sendToolsCall('hang', {}), { + message: 'The connection to the "words" server closed before it answered, so whether the request took effect is unknown. ' + + 'Check its effect before trying again; this host is reconnecting to it.', + }); + await connection.reconnectAfterFailure(); + await pending; + assert.ok(logged(errors.mock.calls, /^MCPL server "words" disconnected \(closed by this host, code=.*, signal=.*, reason=.*\) with 1 request\(s\) unanswered: tools\/call \(id=\d+\)$/), + 'the operator log says the host closed it, and keeps the close\'s details'); +}); + +test('a timeout of one second says so in the singular', async (t) => { + t.mock.method(console, 'error', () => {}); + const connection = await McplServerConnection.connect(config({ requestTimeoutMs: 1000 }), HOST_CAPS); + t.after(() => connection.close()); + connection.ready(); + await assert.rejects(connection.sendToolsCall('hang', {}), (error: Error) => { + assert.match(error.message, /^The "words" server didn't answer within 1 second, so whether the request took effect is unknown: /); + return true; + }); +}); + +test('a connection that closes with nothing pending logs nothing new, by this host or by the server', async (t) => { + const errors = t.mock.method(console, 'error', () => {}); + const byHost = await McplServerConnection.connect(config(), HOST_CAPS); + byHost.ready(); + await byHost.close(); + const dropped = await McplServerConnection.connect(config(), HOST_CAPS); + t.after(() => dropped.close()); + dropped.ready(); + const closed = new Promise((resolve) => { dropped.once('close', () => resolve()); }); + await dropped.sendToolsCall('answer-then-exit', {}); + await within(closed, 'the server to exit'); + assert.ok(!logged(errors.mock.calls, /unanswered/), errors.mock.calls.map((call) => String(call.arguments[0])).join('\n')); +}); + +test('a call pending when this host closes the connection says the outcome is unknown, and the log names it', async (t) => { + const errors = t.mock.method(console, 'error', () => {}); + const connection = await McplServerConnection.connect(config(), HOST_CAPS); + connection.ready(); + // No word on reconnecting: a restart closes the connection, then connects again. + const unknown = { + message: `The "words" server didn't answer before this host closed its connection, so whether the request took effect is unknown. ` + + 'Check its effect before trying again.', + }; + const pending = [assert.rejects(connection.sendToolsCall('hang', {}), unknown), assert.rejects(connection.sendToolsCall('hang', {}), unknown)]; + await connection.close(); + await Promise.all(pending); + // A request after the close isn't told whether the server comes back either. + await assert.rejects(connection.sendToolsCall('refuse', {}), { + message: `The request wasn't sent: this host closed its connection to the "words" server, so nothing ran. ` + + 'If calling it again fails, ask whoever runs this host.', + }); + assert.ok(logged(errors.mock.calls, /^MCPL server "words": this host closed its connection with 2 request\(s\) unanswered: tools\/call \(id=(\d+)\), tools\/call \(id=(?!\1\))\d+\)$/), + 'the operator log has both requests'); +}); + +// --------------------------------------------------------------------------- +// The framework's MCPL tool calls, and a conversation's channel guard +// --------------------------------------------------------------------------- + +type ToolResultEvent = { type: string; result?: { success: boolean; error?: string; isError?: boolean } }; + +/** A framework with only what dispatching an MCPL tool call reads. */ +function frameworkWith(server: unknown, config: Record = {}) { + const events: ToolResultEvent[] = []; + let resolve!: () => void; + const delivered = new Promise((r) => { resolve = r; }); + const framework = Object.create(AgentFramework.prototype) as Record & { + dispatchMcplToolCall(agentName: string, call: { id: string; name: string; input: unknown }, serverId: string, prefix: string): void; + executeMcplToolCall(call: { id: string; name: string; input: unknown }, serverId: string, prefix: string): Promise<{ error?: string }>; + dispatchChannelToolCall(agentName: string, call: { id: string; name: string; input: unknown }): void; + }; + framework.mcplServerRegistry = { getServer: () => server }; + framework.mcplServerConfigs = new Map([['words', { id: 'words', ...config }]]); + framework.mcplConnecting = new Set(); + framework.emitTrace = () => {}; + framework.pushEvent = (event: ToolResultEvent) => { events.push(event); if (event.type === 'tool-result') resolve(); }; + return { framework, events, delivered }; +} + +test("an MCPL tool call refused before it runs says it didn't run, and whether anything can change that", async () => { + const gone = frameworkWith(null); + gone.framework.dispatchMcplToolCall('agent', { id: 'c1', name: 'words--lookup', input: {} }, 'words', 'words'); + // A server missing from the registry was closed for good, removed, or its connect failed without reconnect. + assert.equal(gone.events[0]?.result?.error, `The "words" server that provides words--lookup isn't connected, ` + + "and this host won't reconnect to it on its own, so the call didn't run. If you need it, ask whoever runs this host."); + assert.equal((await gone.framework.executeMcplToolCall({ id: 'c2', name: 'words--lookup', input: {} }, 'words', 'words')).error, + gone.events[0]?.result?.error, "a script's call reads the same"); + + const denied = frameworkWith({}, { disabledTools: ['lookup'] }); + denied.framework.dispatchMcplToolCall('agent', { id: 'c3', name: 'words--lookup', input: {} }, 'words', 'words'); + const deniedText = `words--lookup isn't allowed by this host's policy for the "words" server, so the call didn't run, ` + + "and calling it again won't change that. If you need it, ask whoever runs this host."; + assert.equal(denied.events[0]?.result?.error, deniedText); + assert.equal((await denied.framework.executeMcplToolCall({ id: 'c4', name: 'words--lookup', input: {} }, 'words', 'words')).error, deniedText); +}); + +test("a call while this host connects to the tool's server says so, and after a connect that failed, that the server's gone", async () => { + // A runtime connect or a restart's maps the server's prefix, so its tool + // calls route to it, then awaits addServer, which registers the server once + // the handshake is done. Each handshake here waits for the test. + const servers = new Map(); + const handshakes: Array<(ok: boolean) => void> = []; + const { framework, events } = frameworkWith(null); + Object.assign(framework, { + mcplServerRegistry: { + getServer: (id: string) => servers.get(id) ?? null, + addServer: (cfg: { id: string }) => new Promise((resolve, reject) => { + handshakes.push((ok) => { + if (!ok) return reject(new Error('handshake refused')); + const connection = { on: () => {} }; + servers.set(cfg.id, connection); + resolve(connection); + }); + }), + }, + mcplHostCapabilities: {}, + mcplPrefixMap: new Map(), + mcplServerConfigs: new Map(), + channelRegistry: null, + discordAwarenessOutbox: null, + discordAwarenessEmoji: '🔕', + wireMcplEvents: () => {}, + registerMcplServerFeatures: async () => {}, + }); + const connect = (framework as unknown as { + connectMcplServerInternal(config: McplServerConfig, deferAwareness?: boolean): Promise; + }).connectMcplServerInternal.bind(framework); + const call = (id: string) => ({ id, name: 'words--lookup', input: {} }); + const settle = () => new Promise((r) => setImmediate(r)); + + const connecting = connect({ id: 'words', url: 'ws://127.0.0.1:9', toolPrefix: 'words' }, true); + await settle(); + assert.equal(handshakes.length, 1, 'the handshake has begun'); + framework.dispatchMcplToolCall('agent', call('c1'), 'words', 'words'); + const connectingText = 'This host is connecting to the "words" server that provides words--lookup, so the call didn\'t run. Try again in a moment.'; + assert.equal(events[0]?.result?.error, connectingText); + assert.equal((await framework.executeMcplToolCall(call('c2'), 'words', 'words')).error, connectingText, "a script's call reads the same"); + // Only the server being connected to reads so: another one missing is gone. + framework.dispatchMcplToolCall('agent', { id: 'c3', name: 'other--lookup', input: {} }, 'other', 'other'); + assert.equal(events[1]?.result?.error, 'The "other" server that provides other--lookup isn\'t connected, ' + + "and this host won't reconnect to it on its own, so the call didn't run. If you need it, ask whoever runs this host."); + handshakes[0]!(true); + await connecting; + + // The mark goes when the connect succeeds: once the server is gone, a call reads so. + servers.clear(); + const goneText = 'The "words" server that provides words--lookup isn\'t connected, ' + + "and this host won't reconnect to it on its own, so the call didn't run. If you need it, ask whoever runs this host."; + framework.dispatchMcplToolCall('agent', call('c4'), 'words', 'words'); + assert.equal(events[2]?.result?.error, goneText, 'after a connect that succeeded'); + + // A connect that fails without reconnect leaves the server unregistered, which nothing brings back. + const failing = connect({ id: 'words', url: 'ws://127.0.0.1:9', toolPrefix: 'words' }, true); + await settle(); + handshakes[1]!(false); + await assert.rejects(failing, /handshake refused/); + framework.dispatchMcplToolCall('agent', call('c5'), 'words', 'words'); + assert.equal(events[3]?.result?.error, goneText, 'after a connect that failed'); + assert.equal((await framework.executeMcplToolCall(call('c6'), 'words', 'words')).error, goneText, "a script's call reads the same"); +}); + +test('a second connect, a restart or a disconnect of a server whose connect is in flight is refused before it writes anything', async () => { + // The registry takes the server once its handshake is done, so until then + // only the connecting mark says the id is taken. Each handshake here waits for the test. + const servers = new Map(); + const handshakes: Array<() => void> = []; + const { framework } = frameworkWith(null); + Object.assign(framework, { + mcplServerRegistry: { + getServer: (id: string) => servers.get(id) ?? null, + addServer: (cfg: { id: string }) => new Promise((resolve) => { + handshakes.push(() => { + const connection = { on: () => {} }; + servers.set(cfg.id, connection); + resolve(connection); + }); + }), + }, + moduleRegistry: { getAllModules: () => [] }, + mcplHostCapabilities: {}, + mcplPrefixMap: new Map(), + mcplServerConfigs: new Map(), + channelRegistry: null, + discordAwarenessOutbox: null, + discordAwarenessEmoji: '🔕', + wireMcplEvents: () => {}, + registerMcplServerFeatures: async () => {}, + }); + const fw = framework as unknown as { + connectMcplServerInternal(config: McplServerConfig, deferAwareness?: boolean): Promise; + refuseMcplConnect(config: McplServerConfig, replacing?: string): void; + runDisconnectMcplServer(id: string): Promise; + }; + const words = { id: 'words', url: 'ws://127.0.0.1:9', toolPrefix: 'words' } as McplServerConfig; + const other = { id: 'other', url: 'ws://127.0.0.1:9', toolPrefix: 'other' } as McplServerConfig; + const connecting = fw.connectMcplServerInternal(words, true); + await new Promise((r) => setImmediate(r)); + assert.equal(handshakes.length, 1, 'the handshake has begun'); + const inFlight = { message: 'MCPL server "words" has a connect in flight; try again once it settles' }; + assert.throws(() => fw.refuseMcplConnect(words), inFlight, 'a second connect of it'); + // A restart's disconnect can't stop the connect in flight, which would register the server after it. + assert.throws(() => fw.refuseMcplConnect(words, 'words'), inFlight, 'a restart of it'); + assert.throws(() => fw.refuseMcplConnect(other, 'words'), inFlight, 'a restart of it into another id'); + assert.throws(() => fw.refuseMcplConnect(words, 'other'), inFlight, 'a restart of another server into its id'); + // A disconnect can't stop it either: the server would register after it, its config and prefix gone. + await assert.rejects(fw.runDisconnectMcplServer('words'), inFlight, 'a disconnect of it'); + assert.equal((framework.mcplPrefixMap as Map).get('words'), 'words', 'the refused disconnect left its prefix'); + handshakes[0]!(); + await connecting; + // Once the connect settles, the mark is gone: the registry's check refuses a + // second connect, and a restart, which disconnects the server first, may go ahead. + assert.throws(() => fw.refuseMcplConnect(words), { message: 'MCPL server "words" is already registered' }); + assert.doesNotThrow(() => fw.refuseMcplConnect(words, 'words')); +}); + +test("an MCPL tool's error result with no words says so, and that its effect is unknown", async () => { + const silent = frameworkWith({ sendToolsCall: async () => ({ isError: true, content: [] }) }); + silent.framework.dispatchMcplToolCall('agent', { id: 'c1', name: 'words--lookup', input: {} }, 'words', 'words'); + await within(silent.delivered, 'the tool result'); + const result = silent.events.find((event) => event.type === 'tool-result')!.result!; + assert.equal(result.isError, true); + assert.equal(result.error, `The "words" server reported an error for words--lookup without saying what went wrong. ` + + 'Whether it did anything is unknown: check before calling it again.'); +}); + +test("a conversation bound to its channel is told it can't open another, and where to reply", () => { + const { framework, events } = frameworkWith(null); + framework.conversationAgentHomes = new Map([['fork', 'home-1']]); + framework.toolLifecycleEmitter = undefined; + framework.dispatchChannelToolCall('fork', { id: 'c1', name: 'channel_open', input: { channelId: 'elsewhere' } }); + assert.equal(events[0]?.result?.error, + `This conversation is bound to channel "home-1", so it can't open other channels, and nothing was opened. Reply in "home-1".`); +}); diff --git a/test/mcpl-awareness-barrier.test.ts b/test/mcpl-awareness-barrier.test.ts index 23b6350d5..52c657152 100644 --- a/test/mcpl-awareness-barrier.test.ts +++ b/test/mcpl-awareness-barrier.test.ts @@ -264,8 +264,8 @@ test('mandatory awareness deadline survives requestTimeoutMs: 0 and durably rele const entry = outbox.batches()[0]!.refs[0]!; assert.equal(entry.deliveryStatus, 'pending'); assert.equal(entry.attempts, 1); - assert.match(entry.lastError ?? '', /did not respond to tools\/call/); - assert.match(entry.lastError ?? '', /outcome is unknown/i); + assert.match(entry.lastError ?? '', /didn't answer within/); + assert.match(entry.lastError ?? '', /took effect is unknown/i); assert.equal(records(statusPath).filter((record) => record.event === 'reaction-call').length, 1); const push = records(statusPath).find((record) => record.event === 'push-response'); assert.equal(push?.ledgerStatus, 'pending', 'failure was durable before inference release'); diff --git a/test/mcpl-coalesced-content-parity.test.ts b/test/mcpl-coalesced-content-parity.test.ts index 175de3ced..3de87d882 100644 --- a/test/mcpl-coalesced-content-parity.test.ts +++ b/test/mcpl-coalesced-content-parity.test.ts @@ -19,10 +19,20 @@ import assert from 'node:assert/strict'; import { fixture, TS } from './helpers/coalescing-fixture.js'; import { RefusalLines } from '../src/mcpl/refusal-lines.js'; import { NON_STRING_TEXT_PLACEHOLDER } from '../src/mcpl/visible-content.js'; +import { + AUDIO_NO_DATA_TEXT, + AUDIO_NO_MEDIA_TYPE_TEXT, + IMAGE_NO_DATA_TEXT, + IMAGE_NO_MEDIA_TYPE_TEXT, + REFERENCE_NO_LINK_TEXT, +} from '../src/mcpl/references.js'; import { HookOrchestrator } from '../src/mcpl/hook-orchestrator.js'; import { CapabilityGrant } from '../src/mcpl/capability-grant.js'; import { logDetail, logValue } from '../src/mcpl/log-value.js'; +/** What every lane shows for a block whose type isn't a string. */ +const NO_TYPE = '[Content not shown: a part arrived with no type.]'; + /** Base64 for an image well over 1 MiB once serialized (a Retina screenshot). */ const BIG_IMAGE = { type: 'image', data: 'A'.repeat(1_600_000), mimeType: 'image/png' }; @@ -243,13 +253,13 @@ test('a block whose type, text or source isn\'t a string converts on both lanes, // mimeType and uri also takes an array, a number and null. const odd = (marker: string) => [{ toString: 1 }, [marker], 424242, null]; const malformed: Array<{ content: unknown[]; shows: string }> = [ - { content: [{ type: { toString: 1 } }], shows: '[unrecognized content block: untyped]' }, - { content: [{ type: 7 }], shows: '[unrecognized content block: untyped]' }, - ...odd('array_data').map((data) => ({ content: [{ type: 'image', data, mimeType: 'image/png' }], shows: '[Image: no data]' })), - ...odd('array_uri').map((uri) => ({ content: [{ type: 'image', uri }], shows: '[Image: no data]' })), + { content: [{ type: { toString: 1 } }], shows: NO_TYPE }, + { content: [{ type: 7 }], shows: NO_TYPE }, + ...odd('array_data').map((data) => ({ content: [{ type: 'image', data, mimeType: 'image/png' }], shows: IMAGE_NO_DATA_TEXT })), + ...odd('array_uri').map((uri) => ({ content: [{ type: 'image', uri }], shows: IMAGE_NO_DATA_TEXT })), // Data whose media type is unusable is named as such, not as no data. - ...odd('array_mime').map((mimeType) => ({ content: [{ type: 'image', data: 'AAAA', mimeType }], shows: '[Image: no media type]' })), - ...odd('array_mime').map((mimeType) => ({ content: [{ type: 'audio', data: 'AAAA', mimeType }], shows: '[Audio: no media type]' })), + ...odd('array_mime').map((mimeType) => ({ content: [{ type: 'image', data: 'AAAA', mimeType }], shows: IMAGE_NO_MEDIA_TYPE_TEXT })), + ...odd('array_mime').map((mimeType) => ({ content: [{ type: 'audio', data: 'AAAA', mimeType }], shows: AUDIO_NO_MEDIA_TYPE_TEXT })), // A text block whose text isn't a string shows as its placeholder, so the // agent reads that a block was there; the text beside it shows as itself. ...odd('array_text').map((text, i) => ({ @@ -257,10 +267,10 @@ test('a block whose type, text or source isn\'t a string converts on both lanes, })), // Reference-shaped: a uri that isn't a non-empty string never makes a // reference, with a disposition or without. - { content: [{ type: 'resource', uri: 424242 }], shows: '[unrecognized content block: reference without string uri]' }, - { content: [{ type: 'image', uri: 424242, disposition: 'ref' }], shows: '[Image: no data]' }, - { content: [{ type: 'image', uri: '', disposition: 'ref' }], shows: '[Image: no data]' }, - { content: [{ type: 'audio', uri: { toString: 1 }, disposition: 'ref' }], shows: '[Audio: no data]' }, + { content: [{ type: 'resource', uri: 424242 }], shows: REFERENCE_NO_LINK_TEXT }, + { content: [{ type: 'image', uri: 424242, disposition: 'ref' }], shows: IMAGE_NO_DATA_TEXT }, + { content: [{ type: 'image', uri: '', disposition: 'ref' }], shows: IMAGE_NO_DATA_TEXT }, + { content: [{ type: 'audio', uri: { toString: 1 }, disposition: 'ref' }], shows: AUDIO_NO_DATA_TEXT }, ]; const messages: unknown[] = malformed.flatMap(({ content }, i) => [ { ...f.channel(`e${i}`, '', { initial: true }, 'chat', `coalesced${i}`), content }, @@ -340,7 +350,10 @@ test('content whose blocks aren\'t objects, or whose block of another type carri f.alwaysRespond(); // Only a text block shown as the non-string text placeholder counts as // nothing; any other placeholder still counts as something. - for (const [i, content, shows] of [[0, [424242], 'untyped'], [1, [{ type: 'video', text: 5 }], 'video']] as const) { + for (const [i, content, shows] of [ + [0, [424242], "[Content not shown: a part arrived in a form this host can't read.]"], + [1, [{ type: 'video', text: 5 }], `[Content not shown: this host can't display "video" content.]`], + ] as const) { const r = await f.send('channels/incoming', { messages: [ { channelId: 'chat', messageId: `plain${i}`, timestamp: TS, author: { id: 'u', name: 'User' }, content }, ] }); @@ -350,7 +363,7 @@ test('content whose blocks aren\'t objects, or whose block of another type carri assert.equal(plain.error, undefined); assert.equal(plain.result.accepted, true); await f.framework.runUntilIdle(); - assert.equal(occurrences(f.context(), JSON.stringify(`[unrecognized content block: ${shows}]`)), 2); + assert.equal(occurrences(f.context(), JSON.stringify(shows)), 2); } }); @@ -369,7 +382,7 @@ test('a context injection\'s text block whose text isn\'t a string shows as the const hooks = new HookOrchestrator({ getAllServers: () => [server] } as never, { validateInbound: () => {} } as never); const injections = await hooks.beforeInference({} as never); // The words themselves, which every other test takes from the constant. - const placeholder = "[Text not shown: the server's text block had no string text.]"; + const placeholder = '[Text not shown: a text part arrived without readable text.]'; assert.equal(NON_STRING_TEXT_PLACEHOLDER, placeholder); assert.deepEqual(injections.map((injection) => injection.content), [ [{ type: 'text', text: placeholder }, { type: 'text', text: 'injected_text' }], @@ -408,9 +421,9 @@ test('a malformed block in one server\'s injection shows as its placeholder, as const injections = await hooks.beforeInference({} as never); assert.deepEqual(injections.map((injection) => injection.content), [ [ - { type: 'text', text: '[unrecognized content block: untyped]' }, - { type: 'text', text: '[Image: no data]' }, - { type: 'text', text: '[Audio: no data]' }, + { type: 'text', text: NO_TYPE }, + { type: 'text', text: IMAGE_NO_DATA_TEXT }, + { type: 'text', text: AUDIO_NO_DATA_TEXT }, ], [{ type: 'text', text: 'good_text' }], ]); @@ -570,7 +583,7 @@ test('an inference filter reads the text conversion stores, a placeholder includ // doesn't handle, each of which converts to its placeholder; and visible // text. const content = [{ type: 'text', text: { toString: 1 } }, { type: 'video', uri: 'https://example.invalid/v.mp4' }, { type: 'text', text: 'visible_text' }]; - const shown = `${NON_STRING_TEXT_PLACEHOLDER}\n[unrecognized content block: video]\nvisible_text`; + const shown = `${NON_STRING_TEXT_PLACEHOLDER}\n[Content not shown: this host can't display "video" content.]\nvisible_text`; const r = await f.send('channels/incoming', { messages: [ { ...f.channel('e1', '', { initial: true }, 'chat', 'coalesced'), content }, { channelId: 'chat', messageId: 'plain', timestamp: TS, author: { id: 'u', name: 'User' }, content }, diff --git a/test/mcpl-iserror-failure.test.ts b/test/mcpl-iserror-failure.test.ts index 4e6e70422..73962561f 100644 --- a/test/mcpl-iserror-failure.test.ts +++ b/test/mcpl-iserror-failure.test.ts @@ -17,6 +17,10 @@ const ANSWERS: Record = { two: { isError: true, content: [{ type: 'text', text: 'first' }, { type: 'text', text: 'second' }] }, notString: { isError: true, content: [{ type: 'text', text: { a: 1 } }] }, noText: { isError: true, content: [] }, + // Whitespace alone is no words, as a server's error answer reads it. + blankText: { isError: true, content: [{ type: 'text', text: ' ' }, { type: 'text', text: '\n' }] }, + // Words with whitespace around them are the server's, as they came. + padded: { isError: true, content: [{ type: 'text', text: ' no such row\n' }] }, // A nonconformant server's: no content, or content that isn't a list. noContent: { isError: true }, notList: { isError: true, content: 'not a list' }, @@ -27,14 +31,21 @@ const ANSWERS: Record = { okEmpty: { content: [] }, }; -/** What each error answer reads as: its text, or `Tool call failed` with none. */ +/** What an error answer with no text reads as. */ +const NO_WORDS = + 'The "editor" server reported an error for mcpl--editor--lookup without saying what went wrong. ' + + 'Whether it did anything is unknown: check before calling it again.'; + +/** What each error answer reads as: its text, or NO_WORDS with none. */ const ERRORS: Record = { one: 'no such row', two: 'first\nsecond', notString: NON_STRING_TEXT_PLACEHOLDER, - noText: 'Tool call failed', - noContent: 'Tool call failed', - notList: 'Tool call failed', + noText: NO_WORDS, + blankText: NO_WORDS, + padded: ' no such row\n', + noContent: NO_WORDS, + notList: NO_WORDS, blank: 'after a blank', image: 'see the image', }; @@ -82,7 +93,7 @@ const fields = (r: ToolResult) => ({ success: r.success, data: r.data, error: r. const lookup = (id: string, c: string) => ({ id, name: 'mcpl--editor--lookup', input: { case: c } }); -test('a server\'s error reaches an in-process caller as the failure the model\'s call reads, its text joined, a text that isn\'t a string as the placeholder, and none as "Tool call failed"', { timeout: 20_000 }, async (t) => { +test('a server\'s error reaches an in-process caller as the failure the model\'s call reads, its text joined, a text that isn\'t a string as the placeholder, and none as a failure whose effect is unknown', { timeout: 20_000 }, async (t) => { const { f, ctx, viaModel } = await setup(t); for (const [c, error] of Object.entries(ERRORS)) { const failure = { success: false, data: undefined, error, isError: true }; @@ -117,6 +128,15 @@ test('the admin puppet stores a server\'s error as an error result, and traces i assert.equal(stored.isError, true); assert.equal(stored.content, 'no such row'); assert.deepEqual(traces.slice(before).filter((e) => e.type === 'puppet:tool-call').map((e) => e.isError), [true]); + // With no words, the puppet returns and stores the text the other paths read. + const silent = await f.framework.puppetToolCall('agent', 'mcpl--editor--lookup', { case: 'noText' }); + assert.deepEqual(fields(silent.result), { success: false, data: undefined, error: NO_WORDS, isError: true }); + const storedSilent = f.framework.getAgent('agent')!.getContextManager().getAllMessages() + .flatMap((m) => m.content as unknown as Array>) + .find((b) => b.type === 'tool_result' && b.toolUseId === silent.toolUseId); + assert.ok(storedSilent, 'the no-words pair is stored'); + assert.equal(storedSilent.isError, true); + assert.equal(storedSilent.content, NO_WORDS); }); test('a server\'s error through the model\'s call is traced as tool:failed with its error, and a success as tool:completed', { timeout: 20_000 }, async (t) => { @@ -124,7 +144,7 @@ test('a server\'s error through the model\'s call is traced as tool:failed with await viaModel('d-one', 'mcpl--editor--lookup', { case: 'one' }); assert.deepEqual(terminal('d-one'), [{ type: 'tool:failed', module: 'mcpl:editor', error: 'no such row' }]); await viaModel('d-noText', 'mcpl--editor--lookup', { case: 'noText' }); - assert.deepEqual(terminal('d-noText'), [{ type: 'tool:failed', module: 'mcpl:editor', error: 'Tool call failed' }]); + assert.deepEqual(terminal('d-noText'), [{ type: 'tool:failed', module: 'mcpl:editor', error: NO_WORDS }]); await viaModel('d-ok', 'mcpl--editor--lookup', { case: 'ok' }); assert.deepEqual(terminal('d-ok'), [{ type: 'tool:completed', module: 'mcpl:editor' }]); }); diff --git a/test/mcpl-late-render.test.ts b/test/mcpl-late-render.test.ts index 0021df83c..1a3289070 100644 --- a/test/mcpl-late-render.test.ts +++ b/test/mcpl-late-render.test.ts @@ -47,7 +47,7 @@ async function timedOutRender(t: TestContext, params: PushRenderParams = RENDER) const render = connection.sendPushRender(params); const { id } = JSON.parse(transport.lines[0]!) as { id: number }; t.mock.timers.tick(5_000); - await assert.rejects(render, /did not respond to push\/render/); + await assert.rejects(render, /didn't answer within/); const answerLate = (answer: object = { result: { content: [{ type: 'text', text: 'late' }] } }) => transport.emit('line', JSON.stringify({ jsonrpc: '2.0', id, ...answer })); const recorded = () => traces @@ -99,7 +99,7 @@ test('a late answer to a tool call whose deadline passed is still recorded as mc const call = connection.sendToolsCallWithDeadline('stuck', {}, 75); const { id } = JSON.parse(transport.lines.at(-1)!) as { id: number }; t.mock.timers.tick(75); - await assert.rejects(call, /outcome is unknown/); + await assert.rejects(call, /whether the request took effect is unknown/); return id; }; const answered = await lateCall(); @@ -119,7 +119,7 @@ test('a late answer carrying both a result and an error, which JSON-RPC forbids, const call = connection.sendToolsCallWithDeadline('stuck', {}, 75); const { id } = JSON.parse(transport.lines.at(-1)!) as { id: number }; t.mock.timers.tick(75); - await assert.rejects(call, /outcome is unknown/); + await assert.rejects(call, /whether the request took effect is unknown/); transport.emit('line', JSON.stringify({ jsonrpc: '2.0', id, ...both })); assert.deepEqual(recorded(), [ { ...lateRender, error: '[-32603] tool failed' }, diff --git a/test/mcpl-push-convert.test.ts b/test/mcpl-push-convert.test.ts index e9da35c94..71927f89f 100644 --- a/test/mcpl-push-convert.test.ts +++ b/test/mcpl-push-convert.test.ts @@ -45,5 +45,5 @@ test('undecorated uri-form image keeps url-source handling (behavior preserved)' test('unknown block type fails visibly, never undefined', () => { const out = t({ type: 'holo', frames: 9000 }); assert.equal(out.type, 'text'); - assert.ok(out.text!.includes('unrecognized content block')); + assert.equal(out.text, `[Content not shown: this host can't display "holo" content.]`); }); diff --git a/test/mcpl-reconnect-reregistration.test.ts b/test/mcpl-reconnect-reregistration.test.ts index 2484f610a..6a03af945 100644 --- a/test/mcpl-reconnect-reregistration.test.ts +++ b/test/mcpl-reconnect-reregistration.test.ts @@ -107,6 +107,8 @@ async function makeHarness() { fw.mcplTools = []; fw.mcplToolRefreshInFlight = false; fw.mcplToolRefreshPending = false; + // A disconnect reads the servers whose connect is in flight. + fw.mcplConnecting = new Set(); fw.channelRegistry = null; fw.inferenceRouter = null; fw.eventGate = null; diff --git a/test/mcpl-reference-fetcher.test.ts b/test/mcpl-reference-fetcher.test.ts index d69d3216a..4e201885b 100644 --- a/test/mcpl-reference-fetcher.test.ts +++ b/test/mcpl-reference-fetcher.test.ts @@ -149,7 +149,7 @@ test('third-party origin refused (v1 policy: dialed origin only)', async () => { test('vector 16: expired (and unparseable-expiry) references refuse locally, and take no eager slot', async () => { const reg = new ReferenceRegistry(); const f = makeFetcher({ srv: { url: ORIGIN } }); - for (const [expiresAt, why] of [['2001-01-01T00:00:00Z', /has passed/], ['not-a-date', /does not parse/]] as const) { + for (const [expiresAt, why] of [['2001-01-01T00:00:00Z', /^reference expired$/], ['not-a-date', /^reference treated as expired, since its expiry time can't be read$/]] as const) { // Small, sized and from the dialed origin: only its expiry keeps it from the eager default. const rec = makeRecord(reg, `${ORIGIN}/ok?e=${expiresAt}`, { expiresAt, sizeBytes: PAYLOAD.length }); assert.equal(f.isEagerEligible(rec), false, `${expiresAt}: the eager default would only be refused`); @@ -171,7 +171,7 @@ test('vector 16: an expiry rejected as a field refuses locally, with nothing dia assert.equal(f.isEagerEligible(rec), false, `${label}: the eager default would only be refused`); const out = await f.fetch(rec); assert.equal(out.ok, false, label); - assert.match(out.error!, /rejected as a field/, label); + assert.equal(out.error, "reference treated as expired, since its expiry time isn't in a form this host accepts", label); } assert.equal(saved.length, before, 'nothing saved'); assert.equal(lastAuth, 'untouched', 'the origin was never dialed'); diff --git a/test/mcpl-reference-fields.test.ts b/test/mcpl-reference-fields.test.ts index f5d690879..265962122 100644 --- a/test/mcpl-reference-fields.test.ts +++ b/test/mcpl-reference-fields.test.ts @@ -118,8 +118,8 @@ test('the stub leaves out a type, or a provenance, that sanitizes to nothing', ( test("an expired reference's stub says why, where a live one's advises fetch_reference", () => { // Each bound to its server, as a tool result's references are, so the expiry is what's left to say. - const cases = [['2001-01-01T00:00:00Z', /expired: its expiresAt has passed/], ['not-a-date', /treated as expired: its expiresAt does not parse/], - ['x'.repeat(REFERENCE_LIMITS.expiresAt + 1), /treated as expired: its expiresAt was rejected as a field/]] as const; + const cases = [['2001-01-01T00:00:00Z', /[\]—] expired$/], ['not-a-date', /[\]—] treated as expired, since its expiry time can't be read$/], + ['x'.repeat(REFERENCE_LIMITS.expiresAt + 1), /[\]—] treated as expired, since its expiry time isn't in a form this host accepts$/]] as const; for (const [i, [expiresAt, why]] of cases.entries()) { const { testimony } = reference('resource', { uri: `https://example.test/stub-expired-${i}`, expiresAt }); const stub = buildReferenceStub(testimony, undefined, 'srv'); diff --git a/test/mcpl-request-handler-errors.test.ts b/test/mcpl-request-handler-errors.test.ts index 7e255a375..a7c3e0693 100644 --- a/test/mcpl-request-handler-errors.test.ts +++ b/test/mcpl-request-handler-errors.test.ts @@ -741,13 +741,16 @@ test('a server\'s error answer that no template can convert settles the host\'s transport.emit('line', JSON.stringify({ jsonrpc: '2.0', id: fetch.id, error: { code: { toString: 1 }, message: { toString: 1 } } })); await settle(); assert.deepEqual(lines, [ - '[mcpl] srv: mcpl/manifest fetch failed — previous manifest stands (§17.5): MCPL server "srv" returned error for mcpl/manifest: [[object Object]] [object Object]', + `MCPL server "srv" answered mcpl/manifest (id=${fetch.id}) with error "{\\"toString\\":1}": "{\\"toString\\":1}"`, + '[mcpl] srv: mcpl/manifest fetch failed — previous manifest stands (§17.5): The "srv" server answered with an error without saying what went wrong, so whether the request took effect is unknown. Check its effect before trying again.', ]); assert.deepEqual(traces.filter((e) => e.type === 'mcpl:server-error' || e.type === 'mcpl:request-handler-error'), []); assert.equal(framework.manifestRefreshState.get('srv').inFlight, false); }); -test('a server\'s error code and message in the error a host request rejects with are bounded: control, format and line characters replaced, and each cut after 200 code points', async () => { +test('a server\'s error message in the error a host request rejects with is bounded: control, format and line characters replaced, and cut after 200 code points; its code goes to the log line instead, as logValue prints it', async (t) => { + const lines: string[] = []; + t.mock.method(console, 'error', (...args: unknown[]) => { lines.push(args.map(String).join(' ')); }); const transport = new ResponseTransport(); const connection = new (McplServerConnection as any)('srv', null, transport) as McplServerConnection; connection.establishGrant(new CapabilityGrant(new Set(ALL_CAPABILITY_PATHS), [])); @@ -756,7 +759,7 @@ test('a server\'s error code and message in the error a host request rejects wit const fetch = JSON.parse(transport.lines.at(-1)!); transport.emit('line', JSON.stringify({ jsonrpc: '2.0', id: fetch.id, error: { code: -32000, message: `line one\nline two\u202e${'x'.repeat(300)}` } })); await assert.rejects(fetched, { - message: `MCPL server "srv" returned error for mcpl/manifest: [-32000] line one?line two?${'x'.repeat(182)}…`, + message: `The "srv" server answered with an error: line one?line two?${'x'.repeat(182)}…`, }); // JSON-RPC's code is an integer, but a server can send a string, and it's // the server's text as much as the message is. @@ -764,8 +767,10 @@ test('a server\'s error code and message in the error a host request rejects wit const second = JSON.parse(transport.lines.at(-1)!); transport.emit('line', JSON.stringify({ jsonrpc: '2.0', id: second.id, error: { code: `E\nx\u202e${'y'.repeat(300)}`, message: 'refused' } })); await assert.rejects(again, { - message: `MCPL server "srv" returned error for mcpl/manifest: [E?x?${'y'.repeat(196)}…] refused`, + message: 'The "srv" server answered with an error: refused', }); + // The code is the operator's, so it's in the log line, escaped and cut there. + assert.ok(lines.some((line) => line.startsWith(`MCPL server "srv" answered mcpl/manifest (id=${second.id}) with error "E\\nx\\u202e${'y'.repeat(252)}"…(304): refused`)), lines.join('\n')); }); test('a throw from the wired tools/list_changed handler is contained, traced and logged as the other handlers\' are', (t) => { diff --git a/test/mcpl-request-outcome.test.ts b/test/mcpl-request-outcome.test.ts index e75cabc75..27b2a45fd 100644 --- a/test/mcpl-request-outcome.test.ts +++ b/test/mcpl-request-outcome.test.ts @@ -57,17 +57,18 @@ const rejectsWith = async (promise: Promise, outcome: McplRequestError[ return caught as McplRequestError; }; -test('an error response is error-response, with its data kept and its code in the message', async () => { +test('an error response is error-response, with its data kept and the server\'s words in the message', async () => { connection = await McplServerConnection.connect(config(), HOST_CAPS); const err = await rejectsWith(connection.sendToolsCall('fail', {}), 'error-response'); assert.deepEqual(err.data, { posted: ['111'] }); - assert.match(err.message, /returned error for tools\/call: \[-32001\] POSTED: part 1 of 2/); + // The method, id and code are the operator's, in the log (mcpl-agent-words). + assert.equal(err.message, 'The "outcome" server answered with an error: POSTED: part 1 of 2'); }); -test('a request nobody answers is no-response, and the message is unchanged', async () => { +test('a request nobody answers is no-response', async () => { connection = await McplServerConnection.connect(config({ requestTimeoutMs: 200 }), HOST_CAPS); const err = await rejectsWith(connection.sendToolsCall('stuck', {}), 'no-response'); - assert.match(err.message, /did not respond to tools\/call/); + assert.match(err.message, /^The "outcome" server didn't answer within 0\.2 seconds, so whether the request took effect is unknown/); }); test('a connection lost while awaiting is no-response', async () => { @@ -87,14 +88,15 @@ test('channels/open keeps a deadline when requestTimeoutMs is 0, as channels/pub connection = await McplServerConnection.connect(config({ requestTimeoutMs: 0 }), HOST_CAPS); (connection as unknown as { publishDeadlineMs: number }).publishDeadlineMs = 200; const err = await rejectsWith(connection.sendChannelsOpen({ type: 'channel', address: { id: 'nowhere' } } as never), 'no-response'); - assert.match(err.message, /did not respond to channels\/open/); + assert.match(err.message, /^The "outcome" server didn't answer within 0\.2 seconds/); }); test('a request on a closed connection is not-sent', async () => { connection = await McplServerConnection.connect(config(), HOST_CAPS); await connection.close(); const err = await rejectsWith(connection.sendToolsCall('fail', {}), 'not-sent'); - assert.match(err.message, /Cannot send request: connection to "outcome" is closed/); + assert.equal(err.message, 'The request wasn\'t sent: this host closed its connection to the "outcome" server, so nothing ran. ' + + 'If calling it again fails, ask whoever runs this host.'); connection = null; }); @@ -106,6 +108,7 @@ test('McplRequestError is an Error that keeps its message and name', () => { }); test('a synchronous write rejection is not-sent, leaves no pending request, and does not poison the next request', async (t) => { + const errors = t.mock.method(console, 'error', () => {}); connection = await McplServerConnection.connect(config(), HOST_CAPS); const state = connection as unknown as { transport: { writeLine(json: string): void; emit(event: string, line: string): boolean }; @@ -123,7 +126,10 @@ test('a synchronous write rejection is not-sent, leaves no pending request, and lateRenders.length = 0; state.transport.writeLine = (json) => { unwrittenId = JSON.parse(json).id; throw new Error('synthetic write rejection'); }; const err = await rejectsWith(connection.sendPushRender({ featureSet: 'test', key: 'unwritten', eventId: 'e1', notices: [], dropped: 0 }), 'not-sent'); - assert.match(err.message, /synthetic write rejection/); + assert.equal(err.message, 'The request wasn\'t sent: this host couldn\'t write it to the "outcome" server, so nothing ran. Try again later.'); + // The method, id and the transport's words go to the log. + assert.ok(errors.mock.calls.some((call) => /^MCPL server "outcome": writing push\/render \(id=\d+\) failed: "synthetic write rejection"$/.test(String(call.arguments[0]))), + errors.mock.calls.map((call) => String(call.arguments[0])).join('\n')); assert.equal(state.pendingRequests.size, 0, 'nothing remains awaiting an unwritten request'); t.mock.timers.tick(5000); state.transport.emit('line', JSON.stringify({ jsonrpc: '2.0', id: unwrittenId, result: { content: [] } })); diff --git a/test/mcpl-request-timeout.test.ts b/test/mcpl-request-timeout.test.ts index c04d8e147..0bd581bfc 100644 --- a/test/mcpl-request-timeout.test.ts +++ b/test/mcpl-request-timeout.test.ts @@ -72,7 +72,8 @@ afterEach(async () => { connection = null; }); -test('tools/call against a hung server rejects within the configured timeout', async () => { +test('tools/call against a hung server rejects within the configured timeout', async (t) => { + const errors = t.mock.method(console, 'error', () => {}); connection = await McplServerConnection.connect(config({ requestTimeoutMs: 250 }), HOST_CAPS); connection.ready(); @@ -80,9 +81,11 @@ test('tools/call against a hung server rejects within the configured timeout', a await assert.rejects( connection.sendToolsCall('stuck', {}), (err: Error) => { - assert.match(err.message, /did not respond to tools\/call/); - assert.match(err.message, /250ms/); - assert.match(err.message, /hung/); + // The agent's words name no method; the log keeps it and the id. + assert.match(err.message, /^The "hung" server didn't answer within 0\.25 seconds, so whether the request took effect is unknown/); + assert.ok(!err.message.includes('tools/call'), err.message); + assert.ok(errors.mock.calls.some((call) => /^MCPL server "hung" did not respond to tools\/call \(id=\d+\) within 250ms/ + .test(String(call.arguments[0]))), 'the operator log has the method and id'); return true; }, ); @@ -139,7 +142,7 @@ test('requestTimeoutMs: 0 leaves channels/publish its own deadline, whose expiry (err: Error) => { assert.ok(err instanceof McplRequestError); assert.equal(err.outcome, 'no-response'); - assert.match(err.message, /did not respond to channels\/publish .*within 150ms/); + assert.match(err.message, /^The "hung" server didn't answer within 0\.15 seconds, so whether the request took effect is unknown/); return true; }, ); @@ -153,8 +156,8 @@ test('the timeout error warns the model the tool may have completed server-side' connection.sendToolsCall('stuck', {}), (err: Error) => { // The model reads this string; it must not imply the call was cancelled. - assert.match(err.message, /may still have completed server-side/); - assert.match(err.message, /verify state before retrying/); + assert.match(err.message, /it was abandoned here, not cancelled, and may still complete on the server/); + assert.match(err.message, /Check its effect before trying again/); return true; }, ); @@ -178,7 +181,7 @@ test('a timed-out request that later receives a response does not crash (orphane const orphaned: unknown[] = []; connection.on('orphaned-response', (info) => orphaned.push(info)); - await assert.rejects(connection.sendToolsCall('stuck', {}), /did not respond/); + await assert.rejects(connection.sendToolsCall('stuck', {}), /didn't answer within/); liftRequestTimeout(connection); // The next tools/list makes the stub write the late reply ahead of the // list's own, on the same stream, so by the time the list resolves the late @@ -206,7 +209,7 @@ test('a late STATEFUL orphaned response is surfaced via mcpl:orphaned-response ( const orphaned: Array<{ hadState: boolean; hadCheckpoint: boolean }> = []; connection.on('orphaned-response', (info) => orphaned.push(info)); - await assert.rejects(connection.sendToolsCall('stuck', {}), /did not respond/); + await assert.rejects(connection.sendToolsCall('stuck', {}), /didn't answer within/); liftRequestTimeout(connection); // The next tools/list releases the late reply ahead of its own answer, so it // has been handled by the time the list resolves, and the connection still @@ -233,7 +236,7 @@ test('a mandatory call deadline overrides requestTimeoutMs: 0 and surfaces its l connection.on('orphaned-response', (info) => orphaned.push(info)); await assert.rejects( connection.sendToolsCallWithDeadline('stuck', {}, 75), - /outcome is unknown.*verify state before retrying/i, + /took effect is unknown.*Check its effect before trying again/i, ); // Only the deadline could settle the call, since the server sends nothing // until the next tools/list, which releases the late reply ahead of its own. diff --git a/test/mcpl-tool-lifecycle-e2e.test.ts b/test/mcpl-tool-lifecycle-e2e.test.ts index 57940610a..956d8633e 100644 --- a/test/mcpl-tool-lifecycle-e2e.test.ts +++ b/test/mcpl-tool-lifecycle-e2e.test.ts @@ -266,7 +266,7 @@ describe('tool lifecycle end to end (RFC-007)', () => { await waitFor(() => membrane.calls.length > callsBefore && (membrane.lastStream?.receivedToolResults.length ?? 0) >= 1, 'the refused call answered'); const answer = JSON.stringify(membrane.lastStream!.receivedToolResults[0]); - assert.match(answer, /cannot open channels/, 'the guard did refuse it'); + assert.match(answer, /can't open other channels, and nothing was opened/, 'the guard did refuse it'); await new Promise((r) => setTimeout(r, 300)); const after = readLog(obsLog).filter((e) => e.event === 'lifecycle').length; assert.equal(after, before, 'a refused host-tool call produces no events'); diff --git a/test/mcpl-tool-resolution.test.ts b/test/mcpl-tool-resolution.test.ts index 26f5fbfb8..c61ad02f8 100644 --- a/test/mcpl-tool-resolution.test.ts +++ b/test/mcpl-tool-resolution.test.ts @@ -212,7 +212,8 @@ test("a call of a tool its server's policy denies is refused on every path, not const h = await standIns([{ id: 'short', prefix: 'foo', lists: ['secret', 'echo'], disabledTools: ['secret'] }]); try { for (const path of PATHS) { - assert.equal(await h.call(path, 'foo--secret'), "refused: Tool 'foo--secret' is not permitted by this server's tool policy.", `by ${path}`); + assert.equal(await h.call(path, 'foo--secret'), `refused: foo--secret isn't allowed by this host's policy for the "short" server, ` + + "so the call didn't run, and calling it again won't change that. If you need it, ask whoever runs this host.", `by ${path}`); } assert.equal(await h.call('executeToolCall', 'foo--echo'), 'short:echo', 'an allowed tool is sent'); } finally { @@ -239,10 +240,11 @@ test('a name an MCPL prefix fits goes to MCPL on every path, even with no config try { // A prefix with neither a config nor a server: refused as MCPL, never handed to a module. h.internals.mcplPrefixMap.set('ghost', 'ghost'); - for (const path of ['executeToolCall', 'ctx.callTool'] as const) { - assert.equal(await h.call(path, 'ghost--echo'), 'refused: MCPL server ghost not found', `by ${path}`); + // Every path says the same: the server isn't connected, and won't be on its own. + for (const path of ['executeToolCall', 'ctx.callTool', 'model'] as const) { + assert.equal(await h.call(path, 'ghost--echo'), `refused: The "ghost" server that provides ghost--echo isn't connected, ` + + "and this host won't reconnect to it on its own, so the call didn't run. If you need it, ask whoever runs this host.", `by ${path}`); } - assert.equal(await h.call('model', 'ghost--echo'), 'refused: MCPL server not found: ghost', 'by model'); // A server with no config is still sent the call. h.internals.mcplServerConfigs.delete('short'); for (const path of PATHS) assert.equal(await h.call(path, 'foo--echo'), 'short:echo', `by ${path}`); diff --git a/test/mcpl-transport-close.test.ts b/test/mcpl-transport-close.test.ts index 50080d5dc..b9dcdf147 100644 --- a/test/mcpl-transport-close.test.ts +++ b/test/mcpl-transport-close.test.ts @@ -339,7 +339,7 @@ test('a server that closes its stdin is ended: a request fails with the disconne const answer = connection.sendManifestRequest(30_000).then(() => 'answered', (error: Error) => error.message); assert.match( await within(answer, 'still waiting after 15 s', 15_000), - /^MCPL server "deaf-after-initialize" disconnected unexpectedly \(code=n\/a, signal=SIGTERM, reason=child process exited, and its pipe failed with write EPIPE\) while awaiting mcpl\/manifest/, + /^The connection to the "deaf-after-initialize" server closed before it answered, so whether the request took effect is unknown\. Check its effect before trying again; this host is reconnecting to it\.$/, ); assert.equal((await within(closed, null, 15_000))?.reason, 'child process exited, and its pipe failed with write EPIPE'); assert.equal(await within(reconnected.then(() => 'reconnected'), 'no reconnect within 15 s', 15_000), 'reconnected'); @@ -409,7 +409,7 @@ test("a server that stops reading its stdin without closing it stays connected, // Nothing fails the write, so only the request's own 500 ms timeout ends it. const answer = connection.sendManifestRequest(500).then(() => 'answered', (error: Error) => error.message); - assert.match(await within(answer, 'still waiting after 5 s'), /^MCPL server "stops-reading" did not respond to mcpl\/manifest \(id=\d+\) within 500ms/); + assert.match(await within(answer, 'still waiting after 5 s'), /^The "stops-reading" server didn't answer within 0\.5 seconds, so whether the request took effect is unknown/); assert.equal(closed, false, 'nothing closed the connection'); assert.equal(connection.isConnected, true, 'the connection is still up'); }); diff --git a/test/mcpl-ws-transport.test.ts b/test/mcpl-ws-transport.test.ts index 75f407d9c..6a08fe532 100644 --- a/test/mcpl-ws-transport.test.ts +++ b/test/mcpl-ws-transport.test.ts @@ -370,7 +370,8 @@ test('a server that closes during the handshake with a code and no reason gets t } }); -test("a server's close reason stays out of the error a pending request is rejected with", async () => { +test("a server's close reason stays out of the error a pending request is rejected with, and out of the log", async (t) => { + const errors = t.mock.method(console, 'error', () => {}); const wss = startClosingWsServer('tools/call', 4001, PEER_REASON); await waitListening(wss); let conn: McplServerConnection | undefined; @@ -380,10 +381,15 @@ test("a server's close reason stays out of the error a pending request is reject await assert.rejects( withDeadline(conn.sendToolsCall('echo', { text: 'hello' }), 10_000, 'the call to be rejected'), (error: Error) => { - assert.match(error.message, /^MCPL server "editor" disconnected unexpectedly \(code=4001, signal=n\/a, reason=websocket closed\) while awaiting tools\/call \(id=\d+\)$/); + assert.equal(error.message, 'The connection to the "editor" server closed before it answered, so whether the request took effect is unknown. ' + + "This host won't reconnect to it on its own. If you need it, ask whoever runs this host."); return true; }, ); + // The log has the close's code and the transport's words, never the peer's. + const lines = errors.mock.calls.map((call) => String(call.arguments[0])); + assert.ok(lines.some((line) => /^MCPL server "editor" disconnected \(code=4001, signal=n\/a, reason=websocket closed\) with 1 request\(s\) unanswered: tools\/call \(id=\d+\)$/.test(line)), lines.join('\n')); + assert.ok(!lines.some((line) => line.includes(PEER_REASON)), 'no log line carries the peer\'s close text'); } finally { await conn?.close(); wss.close(); diff --git a/test/prose-drafts.test.ts b/test/prose-drafts.test.ts index 210ced87f..4c6fa0a14 100644 --- a/test/prose-drafts.test.ts +++ b/test/prose-drafts.test.ts @@ -770,7 +770,7 @@ describe('held prose drafts, end to end', () => { await h.until(() => h.texts().some((t) => t.startsWith('[discord-send-failed]')), 'the marker'); assert.equal(h.publishes().length, 0); const marker = h.texts().find((t) => t.startsWith('[discord-send-failed]'))!; - assert.match(marker, /was not confirmed \(MCPL server "discord" did not respond to channels\/publish .*within 1000ms.*\(delivery uncertain\)\)\. It may or may not have been posted/); + assert.match(marker, /was not confirmed \(The "discord" server didn't answer within 1 second, so whether the request took effect is unknown: .*may do it twice \(delivery uncertain\)\)\. It may or may not have been posted/); } finally { await h.close(); } diff --git a/test/publish-outcome.test.ts b/test/publish-outcome.test.ts index 6f322f8ef..7ba2d6b7f 100644 --- a/test/publish-outcome.test.ts +++ b/test/publish-outcome.test.ts @@ -345,6 +345,12 @@ describe('channel_publish answers by the receipt rule', () => { it('answers a request refused before it was written as failed, and an error response or a plain throw as unconfirmed', async () => { const notSent = await publishWith(async () => { throw new McplRequestError('Cannot send request: connection to "discord" is closed', 'not-sent'); }); assert.match(notSent.error ?? '', /^Publish to c failed: Cannot send request: connection to "discord" is closed\. Nothing was posted\.$/); + // The connection's messages are sentences, and the answer adds its own ending. + const sentence = await publishWith(async () => { + throw new McplRequestError('The request wasn\'t sent: the connection to the "discord" server is closed, so nothing ran. This host is reconnecting to it; try again later.', 'not-sent'); + }); + assert.equal(sentence.error, 'Publish to c failed: The request wasn\'t sent: the connection to the "discord" server is closed, so nothing ran. ' + + 'This host is reconnecting to it; try again later. Nothing was posted.'); const answered = await publishWith(async () => { throw new McplRequestError('MCPL server "discord" returned error for channels/publish: [-32000] part 2 failed', 'error-response', { posted: ['111'] }); }); assert.match(answered.error ?? '', /^Publish to c wasn't confirmed: .*part 2 failed \(delivery uncertain\)\. It may or may not have been posted/); // What the connector says was posted reaches the answer, as it does the marker. diff --git a/test/reaction-baseline-platform.test.ts b/test/reaction-baseline-platform.test.ts index 85639b922..38d12dc01 100644 --- a/test/reaction-baseline-platform.test.ts +++ b/test/reaction-baseline-platform.test.ts @@ -25,6 +25,7 @@ async function connectionEnv( mcplHostCapabilities: {}, mcplPrefixMap: new Map(), mcplServerConfigs: new Map(), + mcplConnecting: new Set(), channelRegistry: null, discordAwarenessOutbox: null, discordAwarenessEmoji: '🔕', diff --git a/test/save-recent-image.test.ts b/test/save-recent-image.test.ts index 4013197cb..3b7eec8b8 100644 --- a/test/save-recent-image.test.ts +++ b/test/save-recent-image.test.ts @@ -573,7 +573,7 @@ describe('save_recent_image provenance (issue #104)', () => { toolUseId: 'call_cam', toolName: 'mcpl--vst--camera', content: `[${kept.refId}] kept.png — image/png — from tool result — saved: files/refs/kept.png\n` - + `[${expired.refId}] expired.png — image/png — from tool result — expired: its expiresAt has passed`, + + `[${expired.refId}] expired.png — image/png — from tool result — expired`, isError: false, }] as ContentBlock[]); }, @@ -582,7 +582,7 @@ describe('save_recent_image provenance (issue #104)', () => { const refused = await waitForToolResult(h.framework, 'call_save0'); assert.strictEqual(refused.isError, true); assert.match(refused.content, new RegExp(`index 0 is a reference \\(${expired.refId}, image/png, from`)); - assert.match(refused.content, /It can't be fetched either — expired: its expiresAt has passed\./); + assert.match(refused.content, /It can't be fetched either — expired\./); assert.ok(!refused.content.includes('fetch_reference'), refused.content); const keptRefusal = await waitForToolResult(h.framework, 'call_save1'); assert.strictEqual(keptRefusal.isError, true); diff --git a/test/stop-ends-awareness-drains.test.ts b/test/stop-ends-awareness-drains.test.ts index d3f434431..cdee44ca2 100644 --- a/test/stop-ends-awareness-drains.test.ts +++ b/test/stop-ends-awareness-drains.test.ts @@ -143,7 +143,7 @@ describe('stop() and Discord awareness drains', () => { assert.ok(writes.every((at) => at < stopped), 'no ledger write after stop() returned'); const entries = ledger(dir); assert.equal(entries.m1.attempts, 1, 'the call in flight is recorded'); - assert.match(entries.m1.lastError ?? '', /closed while awaiting response/); + assert.match(entries.m1.lastError ?? '', /didn't answer before this host closed its connection/); assert.deepEqual([entries.m2, entries.m3], [{ attempts: 0 }, { attempts: 0 }], 'what the drain had not sent stays pending, unrecorded'); assert.ok(log.some((line) => line.startsWith( '[discord-awareness] discord: 2 mark(s) not sent: the framework is stopping; the next start delivers them')), log.join('\n')); diff --git a/test/tool-result-reference-uri.test.ts b/test/tool-result-reference-uri.test.ts index 27f36dabe..c16402d29 100644 --- a/test/tool-result-reference-uri.test.ts +++ b/test/tool-result-reference-uri.test.ts @@ -24,7 +24,7 @@ const RESOURCE = { type: 'resource', uri: URI, mimeType: 'application/pdf', name const UNKNOWN = { type: 'zz-unknown', note: 'zz-kept' }; const STUB = /\[ref_[^\]]+\] zz\.pdf — application\/pdf — from tool result/; const BASE64 = 'aGVsbG8gd29ybGQgaGVsbG8gd29ybGQ='; -const WITHHELD = '[inline content withheld: inline data that also claims a uri or a bulk disposition]'; +const WITHHELD = '[Content not shown: its inline data arrived marked as a reference too, so this host withheld it.]'; function parsed(text: string): unknown { return JSON.parse(text); @@ -77,11 +77,11 @@ describe('data the serializer doesn\'t read as content blocks', () => { it('withholds what the reference lanes withhold, saying why', () => { const notAString = parsed(toolResultDataToHistoryString([UNKNOWN, { type: 'resource', uri: { sig: SIGNATURE } }])) as [unknown, string]; - assert.equal(notAString[1], '[unrecognized content block: reference without string uri]'); + assert.equal(notAString[1], '[Reference not shown: it arrived without a link.]'); const tooLong = parsed(toolResultDataToHistoryString([UNKNOWN, { type: 'resource', uri: `${URI}${'x'.repeat(5000)}` }])) as [unknown, string]; - assert.equal(tooLong[1], '[unrecognized content block: uri exceeds schema maximum]'); + assert.equal(tooLong[1], '[Reference not shown: its link is longer than this host accepts.]'); const objectUri = parsed(toolResultDataToHistoryString([UNKNOWN, { type: 'image', uri: { href: URI } }])) as [unknown, string]; - assert.equal(objectUri[1], "[unrecognized content block: image block whose uri isn't a string]"); + assert.equal(objectUri[1], '[Image not shown: it arrived with no usable data or link.]'); const smuggled = toolResultDataToHistoryString([UNKNOWN, { type: 'image', data: BASE64, mimeType: 'image/png', uri: URI }]); assert.ok(!smuggled.includes(SIGNATURE) && !smuggled.includes(BASE64), smuggled); assert.deepEqual(parsed(smuggled), [UNKNOWN, WITHHELD]); @@ -145,7 +145,7 @@ describe('data the serializer doesn\'t read as content blocks', () => { it("governs MCP's embedded resource, whose uri sits under resource, at any depth", () => { const embedded = { type: 'resource', resource: { uri: URI, mimeType: 'application/pdf', blob: BASE64 } }; - const line = '[unrecognized content block: reference without string uri]'; + const line = '[Reference not shown: it arrived without a link.]'; const nested = toolResultDataToHistoryString({ result: { content: [embedded] } }); assert.ok(!nested.includes(SIGNATURE) && !nested.includes(BASE64), nested); assert.deepEqual(parsed(nested), { result: { content: [line] } }); @@ -156,18 +156,18 @@ describe('data the serializer doesn\'t read as content blocks', () => { assert.equal(toolResultDataToHistoryString([{ type: 'text', text: 'zz-text' }, embedded]), `zz-text\n${line}`); }); - it('governs media given a uri that is not a string, failing closed and saying which', () => { + it('governs media given a uri that is not a string, failing closed as the push lane does', () => { const text = toolResultDataToHistoryString({ numeric: { type: 'image', uri: 42, name: 'zz-n' }, both: { type: 'audio', data: [4, 5], uri: 43 }, }); assert.ok(!text.includes('4,5'), text); assert.deepEqual(parsed(text), { - numeric: "[unrecognized content block: image block whose uri isn't a string]", - both: "[unrecognized content block: audio block whose data and uri aren't strings]", + numeric: '[Image not shown: it arrived with no usable data or link.]', + both: '[Audio not shown: it arrived with no usable inline data.]', }); - // In the lanes, a media block with neither keeps the line it had. - assert.deepEqual(classifyBlock({ type: 'image' }), { kind: 'invalid', reason: 'image block with neither data nor uri' }); + // A media block with neither reads as it does with fields that aren't strings. + assert.deepEqual(classifyBlock({ type: 'image' }), { kind: 'invalid', shown: '[Image not shown: it arrived with no usable data or link.]' }); }); it("leaves a nested reference's stub out of save_recent_image's inventory, as main left the reference", () => { @@ -337,6 +337,6 @@ describe('a script given such data', () => { } assert.ok(!text.includes(SIGNATURE), text); assert.match(text, /^zz-text\n\[ref_[^\]]+\] zz\.pdf — application\/pdf — from tool result/); - assert.ok(logged.some((line) => /\[rfc005\] script-demand fetch of ref_\S+ failed: reference unbound: no server is recorded to fetch it from/.test(line)), logged.join('\n')); + assert.ok(logged.some((line) => /\[rfc005\] script-demand fetch of ref_\S+ failed: reference not tied to any server, so there's nowhere to fetch it from/.test(line)), logged.join('\n')); }); });