diff --git a/packages/agent-core-v2/src/_base/tools/support/image-compress.ts b/packages/agent-core-v2/src/_base/tools/support/image-compress.ts index fb98431de..62d1d6578 100644 --- a/packages/agent-core-v2/src/_base/tools/support/image-compress.ts +++ b/packages/agent-core-v2/src/_base/tools/support/image-compress.ts @@ -76,8 +76,26 @@ export interface CompressImageOptions { readonly byteBudget?: number; /** Override the raw-byte ceiling above which compression is skipped. */ readonly maxDecodeBytes?: number; + readonly telemetry?: ImageCompressionTelemetry; } +export interface ImageCompressionTelemetryClient { + track(event: string, properties?: Readonly>): void; +} + +export interface ImageCompressionTelemetry { + readonly client: ImageCompressionTelemetryClient; + readonly source: string; +} + +type CompressOutcome = + | 'compressed' + | 'passthrough_fast' + | 'passthrough_guard' + | 'passthrough_unsupported' + | 'passthrough_unhelpful' + | 'passthrough_error'; + export interface CompressImageResult { /** Bytes to send: the re-encoded image, or the original when unchanged. */ readonly data: Uint8Array; @@ -109,6 +127,7 @@ export async function compressImageForModel( mimeType: string, options: CompressImageOptions = {}, ): Promise { + const startedAt = Date.now(); const maxEdge = options.maxEdge ?? MAX_IMAGE_EDGE_PX; const byteBudget = options.byteBudget ?? IMAGE_BYTE_BUDGET; const maxDecodeBytes = options.maxDecodeBytes ?? MAX_DECODE_BYTES; @@ -126,23 +145,36 @@ export async function compressImageForModel( originalByteLength: bytes.length, finalByteLength: bytes.length, }); + const finish = (outcome: CompressOutcome, result: CompressImageResult): CompressImageResult => { + reportCompressEvent(options.telemetry, { + outcome, + startedAt, + inputMime: normalizedMime, + result, + }); + return result; + }; - if (bytes.length === 0) return passthrough(); + if (bytes.length === 0) return finish('passthrough_unsupported', passthrough()); // Only re-encode formats the codec handles; everything else passes through. - if (!RECODABLE_MIME.has(normalizedMime)) return passthrough(); + if (!RECODABLE_MIME.has(normalizedMime)) return finish('passthrough_unsupported', passthrough()); // Fast path: already within both budgets — no codec load, no allocation. const longestEdge = dims ? Math.max(dims.width, dims.height) : 0; const withinBytes = bytes.length <= byteBudget; const withinEdge = longestEdge > 0 && longestEdge <= maxEdge; - if (withinBytes && (withinEdge || longestEdge === 0)) return passthrough(); + if (withinBytes && (withinEdge || longestEdge === 0)) { + return finish('passthrough_fast', passthrough()); + } // Decompression-bomb guard: refuse to decode absurd pixel counts. The sniff // above gave us the dimensions without decoding, so this costs nothing. - if (dims && dims.width * dims.height > MAX_DECODE_PIXELS) return passthrough(); + if (dims && dims.width * dims.height > MAX_DECODE_PIXELS) { + return finish('passthrough_guard', passthrough()); + } // Refuse to decode very large byte payloads (e.g. a huge or invalid image // from an MCP tool) that would be loaded just to be dropped downstream. - if (bytes.length > maxDecodeBytes) return passthrough(); + if (bytes.length > maxDecodeBytes) return finish('passthrough_guard', passthrough()); try { const { Jimp } = await import('jimp'); @@ -166,9 +198,9 @@ export async function compressImageForModel( const finalPixels = encoded.width * encoded.height; const shrankBytes = encoded.data.length < bytes.length; const shrankPixels = originalPixels > 0 && finalPixels < originalPixels; - if (!shrankBytes && !shrankPixels) return passthrough(); + if (!shrankBytes && !shrankPixels) return finish('passthrough_unhelpful', passthrough()); - return { + return finish('compressed', { data: encoded.data, mimeType: encoded.mimeType, width: encoded.width, @@ -178,10 +210,10 @@ export async function compressImageForModel( changed: true, originalByteLength: bytes.length, finalByteLength: encoded.data.length, - }; + }); } catch { // Decode/encode failure — keep the original bytes. - return passthrough(); + return finish('passthrough_error', passthrough()); } } @@ -214,10 +246,11 @@ export async function compressBase64ForModel( // Skip very large payloads before allocating: base64 decodes to ~3/4 its // length, so a payload whose decoded size would exceed the cap is passed // through without the Buffer.from allocation (and without touching Jimp). + const startedAt = Date.now(); const maxDecodeBytes = options.maxDecodeBytes ?? MAX_DECODE_BYTES; const approxBytes = Math.floor((base64.length * 3) / 4); if (approxBytes > maxDecodeBytes) { - return { + const result: CompressBase64Result = { base64, mimeType, width: 0, @@ -228,12 +261,19 @@ export async function compressBase64ForModel( originalByteLength: approxBytes, finalByteLength: approxBytes, }; + reportCompressEvent(options.telemetry, { + outcome: 'passthrough_guard', + startedAt, + inputMime: normalizeMime(mimeType), + result, + }); + return result; } let bytes: Buffer; try { bytes = Buffer.from(base64, 'base64'); } catch { - return { + const result: CompressBase64Result = { base64, mimeType, width: 0, @@ -244,6 +284,13 @@ export async function compressBase64ForModel( originalByteLength: 0, finalByteLength: 0, }; + reportCompressEvent(options.telemetry, { + outcome: 'passthrough_error', + startedAt, + inputMime: normalizeMime(mimeType), + result, + }); + return result; } const result = await compressImageForModel(bytes, mimeType, options); if (!result.changed) { @@ -279,18 +326,21 @@ export async function compressBase64ForModel( * remote http(s) image) are passed through, as are non-image parts. Best * effort: a part that fails to compress is left unchanged. * - * With `annotate` set, every image that was actually re-encoded gains a - * {@link buildImageCompressionCaption} text part immediately before it, so the - * model knows it is looking at a downsampled copy. `annotate.persistOriginal` - * additionally saves the pre-compression bytes and puts the returned path in - * the caption so the model can read the original back; persistence failures - * degrade to a caption without a path. + * With `annotate` set, every image that was actually re-encoded produces a + * {@link buildImageCompressionCaption} string in the returned `captions` array + * (in encounter order), so the caller can place it next to the image — e.g. as + * a leading text part for the model, or on a `note` side channel kept out of + * UI-visible output. `annotate.persistOriginal` additionally saves the + * pre-compression bytes and puts the returned path in the caption so the model + * can read the original back; persistence failures degrade to a caption + * without a path. */ export async function compressImageContentParts( parts: readonly ContentPart[], options: CompressImageOptions & { readonly annotate?: CompressAnnotateOptions } = {}, -): Promise { +): Promise<{ parts: ContentPart[]; captions: readonly string[] }> { const { annotate, ...compressOptions } = options; + const captions: string[] = []; const out: ContentPart[] = []; for (const part of parts) { if (part.type === 'image_url') { @@ -310,9 +360,8 @@ export async function compressImageContentParts( originalPath = null; } } - out.push({ - type: 'text', - text: buildImageCompressionCaption({ + captions.push( + buildImageCompressionCaption({ original: { width: result.originalWidth, height: result.originalHeight, @@ -327,7 +376,7 @@ export async function compressImageContentParts( }, originalPath, }), - }); + ); } out.push({ type: 'image_url', @@ -339,7 +388,7 @@ export async function compressImageContentParts( } out.push(part); } - return out; + return { parts: out, captions }; } export interface CompressAnnotateOptions { @@ -350,6 +399,46 @@ export interface CompressAnnotateOptions { readonly persistOriginal?: (bytes: Uint8Array, mimeType: string) => Promise; } +interface CompressEventResult { + readonly mimeType: string; + readonly width: number; + readonly height: number; + readonly originalWidth: number; + readonly originalHeight: number; + readonly originalByteLength: number; + readonly finalByteLength: number; +} + +function reportCompressEvent( + telemetry: ImageCompressionTelemetry | undefined, + input: { + readonly outcome: CompressOutcome; + readonly startedAt: number; + readonly inputMime: string; + readonly result: CompressEventResult; + }, +): void { + if (telemetry === undefined) return; + try { + telemetry.client.track('image_compress', { + source: telemetry.source, + outcome: input.outcome, + input_mime: input.inputMime, + output_mime: normalizeMime(input.result.mimeType), + original_bytes: input.result.originalByteLength, + final_bytes: input.result.finalByteLength, + original_width: input.result.originalWidth, + original_height: input.result.originalHeight, + final_width: input.result.width, + final_height: input.result.height, + exif_transposed: false, + duration_ms: Date.now() - input.startedAt, + }); + } catch { + return; + } +} + // ── crop ───────────────────────────────────────────────────────────── /** Crop rectangle in ORIGINAL-image pixel coordinates. */ diff --git a/packages/agent-core-v2/src/agent/contextMemory/loopEventFold.ts b/packages/agent-core-v2/src/agent/contextMemory/loopEventFold.ts index 7ce9ae4dd..ebe14bf52 100644 --- a/packages/agent-core-v2/src/agent/contextMemory/loopEventFold.ts +++ b/packages/agent-core-v2/src/agent/contextMemory/loopEventFold.ts @@ -93,7 +93,11 @@ export type LoopRecordedEvent = | { readonly type: 'tool.result'; readonly toolCallId: string; - readonly result: { readonly output: string | readonly ContentPart[]; readonly isError?: boolean }; + readonly result: { + readonly output: string | readonly ContentPart[]; + readonly isError?: boolean; + readonly note?: string; + }; readonly parentUuid?: string; }; @@ -250,24 +254,33 @@ function interruptedToolMessage(toolCallId: string): ContextMessage { function toolResultOutputForModel(result: { readonly output: string | readonly ContentPart[]; readonly isError?: boolean; + readonly note?: string; }): string | ContentPart[] { - const { output, isError } = result; + const { output, isError, note } = result; + let base: string | ContentPart[]; if (typeof output === 'string') { if (isError === true) { - if (output.length === 0) return TOOL_EMPTY_ERROR_STATUS; - if (output.trimStart().startsWith('ERROR:')) return output; - return `${TOOL_ERROR_STATUS}\n${output}`; + if (output.length === 0) base = TOOL_EMPTY_ERROR_STATUS; + else if (output.trimStart().startsWith('ERROR:')) base = output; + else base = `${TOOL_ERROR_STATUS}\n${output}`; + } else if (output.length === 0 || output.trim() === TOOL_OUTPUT_EMPTY_TEXT) { + base = TOOL_EMPTY_STATUS; + } else { + base = output; } - if (output.length === 0 || output.trim() === TOOL_OUTPUT_EMPTY_TEXT) { - return TOOL_EMPTY_STATUS; - } - return output; + } else if (output.length === 0) { + base = [{ type: 'text', text: isError === true ? TOOL_EMPTY_ERROR_STATUS : TOOL_EMPTY_STATUS }]; + } else if (isError === true) { + base = [{ type: 'text', text: TOOL_ERROR_STATUS }, ...output]; + } else { + base = [...output]; } - if (output.length === 0) { - return [{ type: 'text', text: isError === true ? TOOL_EMPTY_ERROR_STATUS : TOOL_EMPTY_STATUS }]; + if (note === undefined || note.length === 0) return base; + const notePart: ContentPart = { type: 'text', text: note }; + if (typeof base === 'string') return `${base}\n${note}`; + const only = base[0]; + if (base.length === 1 && only?.type === 'text') { + return [{ type: 'text', text: `${only.text}\n${note}` }]; } - if (isError === true) { - return [{ type: 'text', text: TOOL_ERROR_STATUS }, ...output]; - } - return [...output]; + return [...base, notePart]; } diff --git a/packages/agent-core-v2/src/agent/loop/loopService.ts b/packages/agent-core-v2/src/agent/loop/loopService.ts index 3b6a46921..db059b7cd 100644 --- a/packages/agent-core-v2/src/agent/loop/loopService.ts +++ b/packages/agent-core-v2/src/agent/loop/loopService.ts @@ -234,7 +234,7 @@ export class AgentLoopService implements IAgentLoopService { type: 'tool.result', parentUuid: toolCallUuids.get(toolResult.toolCallId) ?? randomUUID(), toolCallId: toolResult.toolCallId, - result: { output: result.output, isError: result.isError }, + result: { output: result.output, isError: result.isError, note: result.note }, }); if (result.stopTurn === true) stopTurn = true; } diff --git a/packages/agent-core-v2/src/agent/mcp/connection-manager.ts b/packages/agent-core-v2/src/agent/mcp/connection-manager.ts index 48cbf04c1..b51e95f16 100644 --- a/packages/agent-core-v2/src/agent/mcp/connection-manager.ts +++ b/packages/agent-core-v2/src/agent/mcp/connection-manager.ts @@ -21,7 +21,7 @@ import { SseMcpClient } from './client-sse'; import type { UnexpectedCloseReason } from './client-shared'; import { StdioMcpClient } from './client-stdio'; import type { McpOAuthService } from '#/agent/mcp/oauth/service'; -import { assertMcpInputSchema, type MCPClient } from './types'; +import { assertMcpInputSchema, type MCPClient, type MCPToolDefinition } from './types'; export type McpServerStatus = 'pending' | 'connected' | 'failed' | 'disabled' | 'needs-auth'; @@ -39,6 +39,7 @@ interface InternalEntry { attemptId: number; status: McpServerStatus; tools?: readonly Tool[]; + rawTools?: readonly MCPToolDefinition[]; enabledNames?: ReadonlySet; error?: string; client?: RuntimeMcpClient; @@ -158,12 +159,18 @@ export class McpConnectionManager { resolved( name: string, ): - | { client: MCPClient; tools: readonly Tool[]; enabledNames: ReadonlySet } + | { + client: MCPClient; + tools: readonly Tool[]; + rawTools: readonly MCPToolDefinition[]; + enabledNames: ReadonlySet; + } | undefined { const entry = this.entries.get(name); if ( entry?.status !== 'connected' || entry.tools === undefined || + entry.rawTools === undefined || entry.client === undefined ) { return undefined; @@ -171,6 +178,7 @@ export class McpConnectionManager { return { client: entry.client, tools: entry.tools, + rawTools: entry.rawTools, enabledNames: entry.enabledNames ?? new Set(entry.tools.map((t) => t.name)), }; } @@ -214,6 +222,7 @@ export class McpConnectionManager { entry.status = 'disabled'; entry.tools = undefined; entry.enabledNames = undefined; + entry.rawTools = undefined; entry.error = undefined; this.emit(entry); this.entries.delete(name); @@ -265,6 +274,7 @@ export class McpConnectionManager { entry.status = 'pending'; entry.tools = undefined; entry.enabledNames = undefined; + entry.rawTools = undefined; entry.error = undefined; this.emit(entry); await this.connectOne(entry, attemptId); @@ -285,7 +295,7 @@ export class McpConnectionManager { const startupClient = await this.createClient(entry.config, entry.name); client = startupClient; entry.client = startupClient; - const tools = await withTimeout( + const discovered = await withTimeout( this.connectAndDiscoverTools(startupClient), timeoutMs, () => { @@ -297,8 +307,9 @@ export class McpConnectionManager { await this.closeRuntimeClient(startupClient); return; } - entry.tools = tools; - entry.enabledNames = computeEnabledNames(entry.config, tools); + entry.tools = discovered.tools; + entry.rawTools = discovered.rawTools; + entry.enabledNames = computeEnabledNames(entry.config, discovered.tools); entry.status = 'connected'; this.watchForUnexpectedClose(entry, startupClient, attemptId); } catch (error) { @@ -317,6 +328,7 @@ export class McpConnectionManager { } entry.tools = undefined; entry.enabledNames = undefined; + entry.rawTools = undefined; // Drop the client reference so a later reconnect builds a fresh one. await this.closeClient(entry); } @@ -338,6 +350,7 @@ export class McpConnectionManager { entry.error = formatUnexpectedCloseError(entry.name, reason); entry.tools = undefined; entry.enabledNames = undefined; + entry.rawTools = undefined; entry.client = undefined; // Best-effort close; the transport is already gone, but this lets the // SDK release timers and pending request handlers. @@ -398,14 +411,19 @@ export class McpConnectionManager { return isUnauthorizedLikeError(error); } - private async connectAndDiscoverTools(client: RuntimeMcpClient): Promise { + private async connectAndDiscoverTools( + client: RuntimeMcpClient, + ): Promise<{ tools: Tool[]; rawTools: MCPToolDefinition[] }> { await client.connect(); const mcpTools = await client.listTools(); - return mcpTools.map((mcpTool) => ({ - name: mcpTool.name, - description: mcpTool.description, - parameters: assertMcpInputSchema(mcpTool.name, mcpTool.inputSchema), - })); + return { + rawTools: mcpTools, + tools: mcpTools.map((mcpTool) => ({ + name: mcpTool.name, + description: mcpTool.description, + parameters: assertMcpInputSchema(mcpTool.name, mcpTool.inputSchema), + })), + }; } private async closeClient(entry: InternalEntry): Promise { diff --git a/packages/agent-core-v2/src/agent/mcp/mcp.ts b/packages/agent-core-v2/src/agent/mcp/mcp.ts index 3668a9146..a35ca3b5a 100644 --- a/packages/agent-core-v2/src/agent/mcp/mcp.ts +++ b/packages/agent-core-v2/src/agent/mcp/mcp.ts @@ -7,11 +7,12 @@ import type { McpServerEntry, } from './connection-manager'; import type { McpOAuthService } from '#/agent/mcp/oauth/service'; -import type { MCPClient } from './types'; +import type { MCPClient, MCPToolDefinition } from './types'; export interface McpResolvedServer { readonly client: MCPClient; readonly tools: readonly KosongTool[]; + readonly rawTools: readonly MCPToolDefinition[]; readonly enabledNames: ReadonlySet; } diff --git a/packages/agent-core-v2/src/agent/mcp/mcpDiscoveryOps.ts b/packages/agent-core-v2/src/agent/mcp/mcpDiscoveryOps.ts new file mode 100644 index 000000000..c0cd0ce0a --- /dev/null +++ b/packages/agent-core-v2/src/agent/mcp/mcpDiscoveryOps.ts @@ -0,0 +1,48 @@ +/** + * `mcp` domain (L5) — MCP tool-discovery wire state. + * + * Restores the per-agent de-dup cursor for durable MCP discovery records, + * keyed by `${serverName}\n${hash}` entries already present in this log. + */ + +import { defineModel } from '#/wire/model'; +import { defineOp } from '#/wire/op'; +import type { MCPToolDefinition } from './types'; + +export interface McpToolCollision { + readonly qualified: string; + readonly toolName: string; + readonly collidesWith: + | { readonly kind: 'same_server'; readonly toolName: string } + | { readonly kind: 'other_server'; readonly serverName: string }; +} + +export interface McpDiscoveryState { + readonly seen: readonly string[]; +} + +export const McpDiscoveryModel = defineModel('mcp.discovery', () => ({ + seen: [], +})); + +export interface McpToolsDiscoveredPayload { + readonly serverName: string; + readonly hash: string; + readonly tools: readonly MCPToolDefinition[]; + readonly enabledNames: readonly string[]; + readonly collisions?: readonly McpToolCollision[]; +} + +export const mcpToolsDiscovered = defineOp(McpDiscoveryModel, 'mcp.tools_discovered', { + apply: (s, p: McpToolsDiscoveredPayload): McpDiscoveryState => { + const key = `${p.serverName}\n${p.hash}`; + if (s.seen.includes(key)) return s; + return { seen: [...s.seen, key] }; + }, +}); + +declare module '#/agent/wireRecord/wireRecord' { + interface WireRecordMap { + 'mcp.tools_discovered': McpToolsDiscoveredPayload; + } +} diff --git a/packages/agent-core-v2/src/agent/mcp/mcpService.ts b/packages/agent-core-v2/src/agent/mcp/mcpService.ts index b05cac09a..7fb1c5d81 100644 --- a/packages/agent-core-v2/src/agent/mcp/mcpService.ts +++ b/packages/agent-core-v2/src/agent/mcp/mcpService.ts @@ -1,3 +1,5 @@ +import { createHash } from 'node:crypto'; + import { InstantiationType } from '#/_base/di/extensions'; import { LifecycleScope, registerScopedService } from '#/_base/di/scope'; import type { Tool as KosongTool } from '#/app/llmProtocol/tool'; @@ -17,7 +19,14 @@ import { createMcpTool } from '#/agent/mcp/tools/mcp'; import type { McpServerEntry } from './connection-manager'; import { IAgentMcpService, type McpServiceOptions } from './mcp'; import { qualifyMcpToolName } from './tool-naming'; -import type { MCPClient } from './types'; +import type { MCPClient, MCPToolDefinition } from './types'; +import { IAgentWireService } from '#/wire/tokens'; +import type { IWireService } from '#/wire/wireService'; +import { + McpDiscoveryModel, + mcpToolsDiscovered, + type McpToolCollision, +} from './mcpDiscoveryOps'; declare module '#/app/event/eventBus' { interface DomainEventMap { @@ -34,24 +43,19 @@ interface McpToolRegistration { readonly serverName: string; } -interface McpToolCollision { - readonly qualified: string; - readonly toolName: string; - readonly collidesWith: - | { readonly kind: 'same_server'; readonly toolName: string } - | { readonly kind: 'other_server'; readonly serverName: string }; -} - export class AgentMcpService extends Disposable implements IAgentMcpService { declare readonly _serviceBrand: undefined; private readonly mcpTools = new Map(); private readonly mcpToolsByServer = new Map(); + private readonly pendingDiscoveries: Array<() => void> = []; + private discoveryWritesReady = false; constructor( private readonly options: McpServiceOptions = {}, @IAgentToolRegistryService private readonly registry: IAgentToolRegistryService, @IEventBus private readonly eventBus: IEventBus, @IAgentToolExecutorService toolExecutor: IAgentToolExecutorService, + @IAgentWireService private readonly wire: IWireService, ) { super(); this.attachMcpTools(); @@ -64,6 +68,8 @@ export class AgentMcpService extends Disposable implements IAgentMcpService { }, ), ); + this._register(this.wire.onRestored(() => this.flushPendingDiscoveries())); + this._register(this.wire.onEmission(() => this.flushPendingDiscoveries())); } get oauthService() { @@ -164,6 +170,7 @@ export class AgentMcpService extends Disposable implements IAgentMcpService { resolved.enabledNames, ); this.emitMcpToolCollisions(entry.name, result.collisions); + this.recordDiscovery(entry.name, resolved.rawTools, resolved.enabledNames, result.collisions); this.eventBus.publish({ type: 'tool.list.updated', reason: 'mcp.connected', @@ -252,6 +259,44 @@ export class AgentMcpService extends Disposable implements IAgentMcpService { return true; } + private recordDiscovery( + serverName: string, + rawTools: readonly MCPToolDefinition[], + enabledNames: ReadonlySet, + collisions: readonly McpToolCollision[], + ): void { + const enabledNamesSnapshot = [...enabledNames].toSorted((a, b) => a.localeCompare(b)); + const work = (): void => { + const hash = createHash('sha256') + .update(JSON.stringify({ tools: rawTools, enabledNames: enabledNamesSnapshot, collisions })) + .digest('hex'); + const key = `${serverName}\n${hash}`; + if (this.wire.getModel(McpDiscoveryModel).seen.includes(key)) return; + this.wire.dispatch( + mcpToolsDiscovered({ + serverName, + hash, + tools: rawTools, + enabledNames: enabledNamesSnapshot, + collisions: collisions.length > 0 ? collisions : undefined, + }), + ); + }; + if (!this.discoveryWritesReady) { + this.pendingDiscoveries.push(work); + return; + } + work(); + } + + private flushPendingDiscoveries(): void { + this.discoveryWritesReady = true; + const pending = this.pendingDiscoveries.splice(0); + for (const work of pending) { + work(); + } + } + private emitMcpToolCollisions( serverName: string, collisions: readonly McpToolCollision[], diff --git a/packages/agent-core-v2/src/agent/mcp/output.ts b/packages/agent-core-v2/src/agent/mcp/output.ts index 990297e69..a377d0946 100644 --- a/packages/agent-core-v2/src/agent/mcp/output.ts +++ b/packages/agent-core-v2/src/agent/mcp/output.ts @@ -14,7 +14,9 @@ * would silently reintroduce the very degradation the caption reports. * 4. Compress oversized inline images, announcing each compression with a * caption (original vs. sent size, readback path to the persisted - * original) so downsampling is never silent. + * original) so downsampling is never silent. The captions ride the + * result's `note` side channel — projected to the model at fold time, but + * kept out of `output` so UIs never render them. * 5. Apply the per-part 10 MB binary cap: oversized binary parts * (image/audio/video URLs) collapse to a notice, so a single * screenshot cannot evict every text part. @@ -26,6 +28,7 @@ */ import type { ContentPart } from '#/app/llmProtocol/message'; +import type { ITelemetryService } from '#/app/telemetry/telemetry'; import { compressImageContentParts } from '#/_base/tools/support/image-compress'; import { persistOriginalImage } from '#/_base/tools/support/image-originals'; @@ -38,6 +41,7 @@ export interface McpOutputOptions { * Falls back to the shared temp-dir cache when absent. */ readonly originalsDir?: string; + readonly telemetry?: ITelemetryService; } // MCP servers can produce arbitrarily large outputs; cap what we feed back to @@ -151,7 +155,12 @@ export async function mcpResultToExecutableOutput( result: MCPToolResult, qualifiedToolName: string, options: McpOutputOptions = {}, -): Promise<{ output: string | ContentPart[]; isError: boolean; truncated?: true }> { +): Promise<{ + output: string | ContentPart[]; + isError: boolean; + note?: string; + truncated?: true; +}> { const converted: ContentPart[] = []; for (const block of result.content) { const part = convertMCPContentBlock(block); @@ -174,6 +183,10 @@ export async function mcpResultToExecutableOutput( // known) so the model can read detail back via ReadMediaFile + region. // Parts that cannot be compressed pass through. const compressed = await compressImageContentParts(budgeted.parts, { + telemetry: + options.telemetry === undefined + ? undefined + : { client: options.telemetry, source: 'mcp_tool_result' }, annotate: { persistOriginal: (bytes, mimeType) => persistOriginalImage( @@ -183,12 +196,16 @@ export async function mcpResultToExecutableOutput( ), }, }); - const capped = applyBinaryPartCap(compressed); + const capped = applyBinaryPartCap(compressed.parts); const truncated = budgeted.truncated || capped.truncated; const output = collapseSingleText(capped.parts); - return truncated - ? { output, isError: result.isError, truncated: true } - : { output, isError: result.isError }; + const note = compressed.captions.length > 0 ? compressed.captions.join('\n') : undefined; + return { + output, + isError: result.isError, + note, + truncated: truncated ? true : undefined, + }; } /** diff --git a/packages/agent-core-v2/src/agent/mcp/tools/mcp.ts b/packages/agent-core-v2/src/agent/mcp/tools/mcp.ts index fe28cc984..b8d918889 100644 --- a/packages/agent-core-v2/src/agent/mcp/tools/mcp.ts +++ b/packages/agent-core-v2/src/agent/mcp/tools/mcp.ts @@ -43,14 +43,15 @@ export function createMcpTool( function normalizeMcpToolResult(result: { readonly output: ExecutableToolResult['output']; readonly isError: boolean; + readonly note?: string; readonly truncated?: true; }): ExecutableToolResult { if (result.isError) { return result.truncated === true - ? { output: result.output, isError: true, truncated: true } - : { output: result.output, isError: true }; + ? { output: result.output, isError: true, note: result.note, truncated: true } + : { output: result.output, isError: true, note: result.note }; } return result.truncated === true - ? { output: result.output, truncated: true } - : { output: result.output }; + ? { output: result.output, note: result.note, truncated: true } + : { output: result.output, note: result.note }; } diff --git a/packages/agent-core-v2/src/agent/tool/toolContract.ts b/packages/agent-core-v2/src/agent/tool/toolContract.ts index fd1f1d536..10eb6cdac 100644 --- a/packages/agent-core-v2/src/agent/tool/toolContract.ts +++ b/packages/agent-core-v2/src/agent/tool/toolContract.ts @@ -51,6 +51,7 @@ export interface ExecutableToolSuccessResult { readonly stopTurn?: boolean | undefined; readonly message?: string | undefined; readonly truncated?: boolean | undefined; + readonly note?: string; readonly delivery?: ToolDelivery | undefined; } @@ -60,6 +61,7 @@ export interface ExecutableToolErrorResult { readonly message?: string | undefined; readonly stopTurn?: boolean | undefined; readonly truncated?: boolean | undefined; + readonly note?: string; readonly delivery?: ToolDelivery | undefined; } diff --git a/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts b/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts index b93e72045..817168716 100644 --- a/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts +++ b/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts @@ -731,9 +731,20 @@ function normalizeToolResult(result: ExecutableToolResult): ToolResult { } } if (result.isError === true) { - return { output, isError: true, stopTurn: result.stopTurn }; + return { + output, + isError: true, + stopTurn: result.stopTurn, + truncated: result.truncated, + note: result.note, + }; } - return { output, stopTurn: result.stopTurn }; + return { + output, + stopTurn: result.stopTurn, + truncated: result.truncated, + note: result.note, + }; } function toolTelemetryOutcome(result: ToolResult): 'success' | 'error' | 'cancelled' { diff --git a/packages/agent-core-v2/src/session/agentLifecycle/agentLifecycleService.ts b/packages/agent-core-v2/src/session/agentLifecycle/agentLifecycleService.ts index ac71cd53a..23c3b946f 100644 --- a/packages/agent-core-v2/src/session/agentLifecycle/agentLifecycleService.ts +++ b/packages/agent-core-v2/src/session/agentLifecycle/agentLifecycleService.ts @@ -26,6 +26,8 @@ import { registerScopedService, } from '#/_base/di/scope'; import { IBootstrapService } from '#/app/bootstrap/bootstrap'; +import { IEventBus } from '#/app/event/eventBus'; +import { ErrorCodes, makeErrorPayload } from '#/errors'; import { ITelemetryService } from '#/app/telemetry/telemetry'; import { ILogService } from '#/_base/log/log'; import { IAgentProfileCatalogService } from '#/app/agentProfileCatalog/agentProfileCatalog'; @@ -245,6 +247,11 @@ export class AgentLifecycleService extends Disposable implements IAgentLifecycle const manager = this.getMcpManager(); const initialLoad = this.connectMcpServers(manager).catch((error: unknown) => { this.log.error('mcp initial load failed', { error }); + const message = error instanceof Error ? error.message : String(error); + this.handles.get('main')?.accessor.get(IEventBus)?.publish({ + type: 'error', + ...makeErrorPayload(ErrorCodes.MCP_STARTUP_FAILED, message), + }); }); this.mcpInitialLoad = initialLoad; return initialLoad; diff --git a/packages/agent-core-v2/test/contextMemory/loop-event-fold-parity.test.ts b/packages/agent-core-v2/test/contextMemory/loop-event-fold-parity.test.ts index 0f3705316..fea6de6fd 100644 --- a/packages/agent-core-v2/test/contextMemory/loop-event-fold-parity.test.ts +++ b/packages/agent-core-v2/test/contextMemory/loop-event-fold-parity.test.ts @@ -106,4 +106,45 @@ describe('loop-event fold parity', () => { expect(folded).toEqual(baseline); }); + + it('folds a tool-result note as trailing model text without splitting text-only output', () => { + context.append( + { + role: 'assistant', + content: [], + toolCalls: [{ type: 'function', id: 'c3', name: 'Screenshot', arguments: '{}' }], + }, + { + role: 'tool', + content: [{ type: 'text', text: 'result text\nImage compressed.' }], + toolCalls: [], + toolCallId: 'c3', + isError: false, + }, + ); + const baseline = comparable(context.get()); + context.clear(); + + context.appendLoopEvent({ type: 'step.begin', uuid: 's3' }); + context.appendLoopEvent({ + type: 'tool.call', + stepUuid: 's3', + toolCallId: 'c3', + name: 'Screenshot', + args: {}, + }); + context.appendLoopEvent({ + type: 'tool.result', + toolCallId: 'c3', + result: { + output: 'result text', + isError: false, + note: 'Image compressed.', + }, + }); + context.appendLoopEvent({ type: 'step.end', uuid: 's3' }); + const folded = comparable(context.get()); + + expect(folded).toEqual(baseline); + }); }); diff --git a/packages/agent-core-v2/test/mcp/mcp.test.ts b/packages/agent-core-v2/test/mcp/mcp.test.ts index 73021ecd8..563e1ce53 100644 --- a/packages/agent-core-v2/test/mcp/mcp.test.ts +++ b/packages/agent-core-v2/test/mcp/mcp.test.ts @@ -10,7 +10,12 @@ import type { McpConnectionManager, McpServerEntry } from '#/agent/mcp/connectio import { IAgentMcpService } from '#/agent/mcp/mcp'; import { AgentMcpService } from '#/agent/mcp/mcpService'; import type { McpOAuthService } from '#/agent/mcp/oauth/service'; -import type { MCPClient } from '#/agent/mcp/types'; +import type { MCPClient, MCPToolDefinition } from '#/agent/mcp/types'; +import { IAgentWireService } from '#/wire/tokens'; +import { WireService } from '#/wire/wireServiceImpl'; +import { McpDiscoveryModel } from '#/agent/mcp/mcpDiscoveryOps'; +import { AGENT_WIRE_PROTOCOL_VERSION } from '#/agent/wireRecord/wireRecord'; +import { wireMetadata } from '#/agent/wireRecord/metadataOps'; import { AgentToolExecutorService } from '#/agent/toolExecutor/toolExecutorService'; import { IAgentToolExecutorService } from '#/agent/toolExecutor/toolExecutor'; import { IAgentToolRegistryService } from '#/agent/toolRegistry/toolRegistry'; @@ -29,6 +34,7 @@ const MCP_OUTPUT_TRUNCATED_TEXT = interface ResolvedServer { readonly client: MCPClient; readonly tools: readonly KosongTool[]; + readonly rawTools: readonly MCPToolDefinition[]; readonly enabledNames: ReadonlySet; } @@ -74,8 +80,21 @@ class FakeMcpManager { client: MCPClient, tools: readonly KosongTool[], enabledNames = new Set(tools.map((tool) => tool.name)), + rawTools?: readonly MCPToolDefinition[], ): void { - this.resolvedEntries.set(name, { client, tools, enabledNames }); + const resolvedRawTools = + rawTools ?? + tools.map((tool) => ({ + name: tool.name, + description: tool.description ?? '', + inputSchema: (tool.parameters ?? {}) as MCPToolDefinition['inputSchema'], + })); + this.resolvedEntries.set(name, { + client, + tools, + rawTools: resolvedRawTools, + enabledNames, + }); } connect(name: string, options: { readonly transport?: 'stdio' | 'http' | 'sse' } = {}): void { @@ -128,6 +147,7 @@ describe('AgentMcpService', () => { let disposables: DisposableStore; let ix: TestInstantiationService; let events: DomainEvent[]; + let wire: WireService; beforeEach(() => { disposables = new DisposableStore(); @@ -142,6 +162,8 @@ describe('AgentMcpService', () => { ix.set(IAgentToolRegistryService, new SyncDescriptor(AgentToolRegistryService)); ix.set(IAgentToolExecutorService, new SyncDescriptor(AgentToolExecutorService)); ix.stub(IAgentTurnService, stubTurnWithHooks()); + wire = disposables.add(new WireService({ logScope: 'mcp-test', logKey: 'wire.jsonl' })); + ix.stub(IAgentWireService, wire); }); afterEach(() => { disposables.dispose(); @@ -469,6 +491,200 @@ describe('AgentMcpService', () => { }), ); }); + + const RAW_QUERY: MCPToolDefinition = { + name: 'query_range', + description: 'Query a metrics range', + inputSchema: { + type: 'object', + properties: { query: { type: 'string' } }, + required: ['query'], + }, + }; + + function collectDiscoveries(): { + records: { type: string; [key: string]: unknown }[]; + off: { dispose(): void }; + } { + const records: { type: string; [key: string]: unknown }[] = []; + const off = wire.onEmission((e) => { + if (e.record.type === 'mcp.tools_discovered') { + records.push(e.record as { type: string; [key: string]: unknown }); + } + }); + return { records, off }; + } + + it('records tools/list once after restore and dedups unchanged reconnects', async () => { + const manager = new FakeMcpManager(); + const client = fakeMcpClient([RAW_QUERY]); + const rawTools = await client.listTools(); + manager.setResolved( + 'grafana', + client, + await discoverTools(client), + new Set(['query_range']), + rawTools, + ); + createService(manager); + + const { records, off } = collectDiscoveries(); + try { + manager.connect('grafana'); + expect(records).toHaveLength(0); // parked until restore + await wire.replay(); + expect(records).toHaveLength(1); + expect(records[0]).toMatchObject({ + type: 'mcp.tools_discovered', + serverName: 'grafana', + tools: rawTools, + enabledNames: ['query_range'], + }); + expect(records[0]!['collisions']).toBeUndefined(); + + // identical content -> no second record + manager.connect('grafana'); + expect(records).toHaveLength(1); + + // allow-list change is a different gating decision -> record again + manager.setResolved('grafana', client, await discoverTools(client), new Set(), rawTools); + manager.connect('grafana'); + expect(records).toHaveLength(2); + } finally { + off.dispose(); + } + }); + + it('parks a discovery observed before restore and flushes it after replay', async () => { + const manager = new FakeMcpManager(); + const client = fakeMcpClient([RAW_QUERY]); + const rawTools = await client.listTools(); + manager.setResolved( + 'grafana', + client, + await discoverTools(client), + new Set(['query_range']), + rawTools, + ); + createService(manager); + + const { records, off } = collectDiscoveries(); + try { + manager.connect('grafana'); + expect(records).toHaveLength(0); // parked, not yet durable + await wire.replay(); + expect(records).toHaveLength(1); + } finally { + off.dispose(); + } + }); + + it('snapshots enabledNames when parking a discovery before restore', async () => { + const manager = new FakeMcpManager(); + const client = fakeMcpClient([RAW_QUERY]); + const rawTools = await client.listTools(); + const enabledNames = new Set(['query_range']); + manager.setResolved( + 'grafana', + client, + await discoverTools(client), + enabledNames, + rawTools, + ); + createService(manager); + + const { records, off } = collectDiscoveries(); + try { + manager.connect('grafana'); + enabledNames.clear(); + enabledNames.add('mutated_after_observation'); + await wire.replay(); + + expect(records).toHaveLength(1); + expect(records[0]).toMatchObject({ + type: 'mcp.tools_discovered', + serverName: 'grafana', + tools: rawTools, + enabledNames: ['query_range'], + }); + } finally { + off.dispose(); + } + }); + + it('flushes a parked discovery after the first live wire record on a fresh session', async () => { + const manager = new FakeMcpManager(); + const client = fakeMcpClient([RAW_QUERY]); + const rawTools = await client.listTools(); + manager.setResolved( + 'grafana', + client, + await discoverTools(client), + new Set(['query_range']), + rawTools, + ); + createService(manager); + + const { records, off } = collectDiscoveries(); + try { + manager.connect('grafana'); + expect(records).toHaveLength(0); + wire.dispatch( + wireMetadata({ + protocol_version: AGENT_WIRE_PROTOCOL_VERSION, + created_at: 1, + }), + ); + expect(records).toHaveLength(1); + expect(records[0]).toMatchObject({ + type: 'mcp.tools_discovered', + serverName: 'grafana', + tools: rawTools, + enabledNames: ['query_range'], + }); + } finally { + off.dispose(); + } + }); + + it('re-records when only the collision outcome changes', async () => { + const manager = new FakeMcpManager(); + const occupant = fakeMcpClient([RAW_QUERY]); + const occupantRaw = await occupant.listTools(); + manager.setResolved( + 'graf.ana', + occupant, + await discoverTools(occupant), + new Set(['query_range']), + occupantRaw, + ); + createService(manager); + manager.connect('graf.ana'); + await wire.replay(); // restore; occupant discovery recorded (before we subscribe) + + const { records, off } = collectDiscoveries(); + try { + const client = fakeMcpClient([RAW_QUERY]); + const rawTools = await client.listTools(); + manager.setResolved( + 'graf_ana', + client, + await discoverTools(client), + new Set(['query_range']), + rawTools, + ); + manager.connect('graf_ana'); // collides with the occupant's qualified name + expect(records).toHaveLength(1); + expect(records[0]!['collisions']).toHaveLength(1); + + manager.disconnect('graf.ana'); // occupant gone + manager.connect('graf_ana'); // same rawTools/allow-list, collision flipped + expect(records).toHaveLength(2); + expect(records[1]!['collisions']).toBeUndefined(); + } finally { + off.dispose(); + } + }); }); describe('AgentMcpService + AgentProfileService', () => { diff --git a/packages/agent-core-v2/test/mcp/output.test.ts b/packages/agent-core-v2/test/mcp/output.test.ts index 41eba2b1b..5c532e8fa 100644 --- a/packages/agent-core-v2/test/mcp/output.test.ts +++ b/packages/agent-core-v2/test/mcp/output.test.ts @@ -6,6 +6,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { describe, expect, test } from 'vitest'; +import type { ITelemetryService } from '#/app/telemetry/telemetry'; import { convertMCPContentBlock, mcpResultToExecutableOutput } from '#/agent/mcp/output'; import { createMcpTool } from '#/agent/mcp/tools/mcp'; import type { MCPClient, MCPContentBlock, MCPToolResult } from '#/agent/mcp/types'; @@ -28,6 +29,29 @@ function assertValidMcpBlock(block: T): T { return block; } +interface TelemetryRecord { + readonly event: string; + readonly properties: Readonly> | undefined; +} + +function recordingTelemetry(records: TelemetryRecord[]): ITelemetryService { + const telemetry: ITelemetryService = { + _serviceBrand: undefined, + track(event, properties) { + records.push({ event, properties }); + }, + withContext: () => telemetry, + setContext: () => {}, + addAppender: () => ({ dispose: () => {} }), + removeAppender: () => {}, + setAppender: () => {}, + setEnabled: () => {}, + flush: async () => {}, + shutdown: async () => {}, + }; + return telemetry; +} + describe('convertMCPContentBlock', () => { test('converts text block to TextPart', () => { const block: MCPContentBlock = { type: 'text', text: 'hello' }; @@ -321,7 +345,7 @@ describe('mcpResultToExecutableOutput', () => { { type: 'text', text: 'A'.repeat(100_000) }, { type: 'image_url', imageUrl: { url: 'data:image/png;base64,' + 'B'.repeat(500_000) } }, ]); - expect(out).not.toHaveProperty('truncated'); + expect(out.truncated).toBeUndefined(); }); test('downsamples an oversized real image instead of leaving it full-size', async () => { @@ -358,15 +382,12 @@ describe('mcpResultToExecutableOutput', () => { ); const parts = out.output as ContentPart[]; - const captionIndex = parts.findIndex( - (p) => p.type === 'text' && p.text.includes('Image compressed'), - ); - expect(captionIndex).toBeGreaterThanOrEqual(0); - const caption = (parts[captionIndex] as { text: string }).text; + const caption = out.note; + expect(caption).toContain('Image compressed'); expect(caption).toContain('2600x2600'); - expect(parts[captionIndex + 1]?.type).toBe('image_url'); + expect(parts.some((p) => p.type === 'image_url')).toBe(true); - const pathMatch = /saved at "([^"]+)"/.exec(caption); + const pathMatch = /saved at "([^"]+)"/.exec(caption!); expect(pathMatch).not.toBeNull(); const persisted = await readFile(pathMatch![1]!); expect(persisted.equals(bigBytes)).toBe(true); @@ -383,9 +404,38 @@ describe('mcpResultToExecutableOutput', () => { 'mcp__s__shot', ); - const parts = out.output as ContentPart[]; - const joined = parts.map((p) => (p.type === 'text' ? p.text : '')).join(''); - expect(joined).not.toContain('Image compressed'); + expect(out.note).toBeUndefined(); + }); + + test('reports MCP image compression telemetry with the MCP tool-result source', async () => { + const records: TelemetryRecord[] = []; + const big = Buffer.from( + await new Jimp({ width: 2600, height: 2600, color: 0x3366ccff }).getBuffer('image/png'), + ).toString('base64'); + + await mcpResultToExecutableOutput( + result([{ type: 'image', data: big, mimeType: 'image/png' }]), + 'mcp__s__shot', + { telemetry: recordingTelemetry(records) }, + ); + + const events = records.filter((record) => record.event === 'image_compress'); + expect(events).toHaveLength(1); + const properties = events[0]!.properties; + expect(properties).toEqual( + expect.objectContaining({ + source: 'mcp_tool_result', + outcome: 'compressed', + input_mime: 'image/png', + output_mime: 'image/png', + original_width: 2600, + original_height: 2600, + exif_transposed: false, + }), + ); + expect(properties?.['final_width']).toBeLessThanOrEqual(2000); + expect(properties?.['final_height']).toBeLessThanOrEqual(2000); + expect(properties?.['duration_ms']).toEqual(expect.any(Number)); }); test('persists originals into the provided session originals dir', async () => { @@ -400,10 +450,9 @@ describe('mcpResultToExecutableOutput', () => { { originalsDir: dir }, ); - const parts = out.output as ContentPart[]; - const caption = parts.find((p) => p.type === 'text' && p.text.includes('Image compressed')); - if (caption?.type !== 'text') throw new Error('expected a compression caption'); - const pathMatch = /saved at "([^"]+)"/.exec(caption.text); + const caption = out.note; + expect(caption).toContain('Image compressed'); + const pathMatch = /saved at "([^"]+)"/.exec(caption!); expect(pathMatch).not.toBeNull(); expect(pathMatch![1]!.startsWith(dir)).toBe(true); const persisted = await readFile(pathMatch![1]!); @@ -432,10 +481,8 @@ describe('mcpResultToExecutableOutput', () => { const toolText = parts[0]; if (toolText?.type !== 'text') throw new Error('expected the tool text part first'); expect(toolText.text).toContain('Output truncated'); - const caption = parts.find((p) => p.type === 'text' && p.text.includes('Image compressed')); - if (caption?.type !== 'text') throw new Error('expected a compression caption'); - expect(caption.text).toMatch(/<\/system>$/); - expect(caption.text).toContain('saved at'); + expect(out.note).toMatch(/<\/system>$/); + expect(out.note).toContain('saved at'); await rm(dir, { recursive: true, force: true }); }); @@ -454,13 +501,11 @@ describe('mcpResultToExecutableOutput', () => { { originalsDir: dir }, ); - const parts = out.output as ContentPart[]; expect(out.truncated).toBeUndefined(); - const caption = parts.find((p) => p.type === 'text' && p.text.includes('Image compressed')); - if (caption?.type !== 'text') throw new Error('expected a compression caption'); - expect(caption.text).toMatch(/^Image compressed/); - expect(caption.text).toMatch(/<\/system>$/); - expect(caption.text).toContain('saved at'); + expect(out.note).toMatch(/^Image compressed/); + expect(out.note).toMatch(/<\/system>$/); + expect(out.note).toContain('saved at'); + const parts = out.output as ContentPart[]; const joined = parts.map((p) => (p.type === 'text' ? p.text : '')).join(''); expect(joined).not.toContain('Output truncated'); await rm(dir, { recursive: true, force: true }); diff --git a/packages/agent-core-v2/test/toolExecutor/tool-executor.test.ts b/packages/agent-core-v2/test/toolExecutor/tool-executor.test.ts index 557c0f17d..e9349cfbe 100644 --- a/packages/agent-core-v2/test/toolExecutor/tool-executor.test.ts +++ b/packages/agent-core-v2/test/toolExecutor/tool-executor.test.ts @@ -100,6 +100,33 @@ describe('AgentToolExecutorService', () => { }); }); + it('preserves internal result notes without exposing them on protocol tool.result events', async () => { + const tool = new TestTool('captioned', { + result: { + output: 'image sent', + note: 'Image compressed.', + }, + }); + registry.register(tool); + + const results = await execute([toolCall('call_captioned', 'captioned', {})]); + + expect(results[0]).toMatchObject({ + output: 'image sent', + note: 'Image compressed.', + }); + const protocolResult = protocolEvents.find( + (event): event is Extract => + event.type === 'tool.result', + ); + expect(protocolResult).toMatchObject({ + type: 'tool.result', + toolCallId: 'call_captioned', + output: 'image sent', + }); + expect(protocolResult as unknown as Record).not.toHaveProperty('note'); + }); + it('records an error tool.result when the tool name is unknown', async () => { const results = await execute([toolCall('call_missing', 'missing', { text: 'hi' })]);