fix(mcp): make execute results the same in every client

cli/src/mcp/execute.ts

@@ -2,7 +2,7 @@ import { Effect } from "effect"
 import * as RelayClient from "../relay/client.ts"
 import * as RelayLifecycle from "../relay/lifecycle.ts"
 import { registerMcpTools } from "./registry.ts"
-import { ExecuteToolOutput, executeToolResult, objectSchema, parseExecuteArguments } from "./schema.ts"
+import { executeToolResult, objectSchema, parseExecuteArguments } from "./schema.ts"
 
 registerMcpTools({
   order: 100,
@@ -46,7 +46,7 @@ registerMcpTools({
       destructive: true,
       idempotent: false,
       openWorld: true,
-      outputSchema: ExecuteToolOutput,
+      // No outputSchema: clients that validate one reject image results, which carry no structured content.
       result: executeToolResult,
       handle: (input) =>
         Effect.gen(function* () {

cli/src/mcp/schema.ts

@@ -269,13 +269,67 @@ export function executeToolOutput(result: ExecuteResponse & { readonly notice?:
   }
 }
 
+// Many clients show only the text of an error result, and one cuts error text past 10,000 characters from the middle,
+// so an error's text carries its warnings and context and stays under that.
+const errorMessageLimit = 3000
+const errorLogLimit = 20
+const errorLogLineLimit = 300
+const errorLogsLimit = 2500
+
+function clip(text: string, limit: number): string {
+  if (text.length <= limit) return text
+  const head = Math.ceil(limit * 2 / 3)
+  const tail = limit - head
+  return `${text.slice(0, head)}\n… ${text.length - limit} characters cut …\n${text.slice(-tail)}`
+}
+
+function aftermathLine(aftermath: ExecuteAftermath | undefined): string | undefined {
+  if (!aftermath) return undefined
+  const parts: string[] = []
+  if (aftermath.endUrl === null) parts.push("the page closed")
+  else if (aftermath.startUrl !== aftermath.endUrl) parts.push(`moved from ${clip(aftermath.startUrl ?? "nowhere", 200)} to ${clip(aftermath.endUrl, 200)}`)
+  if (aftermath.navigations.length > 0) parts.push(`${aftermath.navigations.length} navigation${aftermath.navigations.length === 1 ? "" : "s"}`)
+  if (aftermath.consoleErrorCount > 0) parts.push(`${aftermath.consoleErrorCount} console error${aftermath.consoleErrorCount === 1 ? "" : "s"}`)
+  if (aftermath.pageErrorCount > 0) parts.push(`${aftermath.pageErrorCount} page error${aftermath.pageErrorCount === 1 ? "" : "s"}`)
+  if (aftermath.handoffs > 0) parts.push(`${aftermath.handoffs} handoff${aftermath.handoffs === 1 ? "" : "s"}`)
+  return parts.length > 0 ? `Page: ${parts.join("; ")}` : undefined
+}
+
+function errorLogsBlock(logs: readonly ExecuteLogEntry[]): string | undefined {
+  if (logs.length === 0) return undefined
+  const lines: string[] = []
+  let size = 0
+  for (const entry of logs.slice(-errorLogLimit).reverse()) {
+    const prefix = entry.source === "page" ? `[page ${entry.type}]` : `[${entry.type}]`
+    const repeat = entry.repeatCount && entry.repeatCount > 1 ? ` (×${entry.repeatCount})` : ""
+    const line = clip(`${prefix} ${entry.text}${repeat}`, errorLogLineLimit).replaceAll("\n", " ")
+    if (size + line.length > errorLogsLimit) break
+    lines.unshift(line)
+    size += line.length + 1
+  }
+  const shown = lines.length < logs.length ? `last ${lines.length} of ${logs.length}` : `${logs.length}`
+  return `Logs (${shown}):\n${lines.join("\n")}`
+}
+
+/** The text of a failed execute: the error first, then the warnings, diagnostic, page movement and recent logs. */
+export function executeErrorText(result: ExecuteResponse): string {
+  const warnings = result.warnings ?? []
+  return [
+    clip(result.text, errorMessageLimit),
+    warnings.length > 0 ? `Warnings:\n${warnings.map((warning) => `- ${warning}`).join("\n")}` : undefined,
+    result.diagnostic ? `Diagnostic: ${result.diagnostic}` : undefined,
+    aftermathLine(result.aftermath),
+    errorLogsBlock(result.logs),
+  ].filter((part): part is string => part !== undefined).join("\n\n")
+}
+
 /**
  * The text fallback for an execute. A string return is the agent's own readable text, not JSON; a structured return is
  * its compact JSON value. That keeps one copy of the value outside the structured result.
  */
 export function executeToolText(result: ExecuteResponse & { readonly notice?: string }): string {
   const text = result.isError
-    ? result.text
+    ? executeErrorText(result)
     : Predicate.isString(result.value)
       ? result.value
       : result.value === undefined
@@ -284,13 +338,16 @@ export function executeToolText(result: ExecuteResponse & { readonly notice?: st
   return result.notice ? `${text}\n\n${result.notice}` : text
 }
 
-/** The MCP result for one execute: images keep their content blocks and text, everything else gets structured output. */
+/**
+ * The MCP result for one execute: images keep their content blocks and text, errors are text only, and everything else
+ * also gets structured output.
+ */
 export function executeToolResult(value: unknown): McpSchema.CallToolResult {
   const result = value as ExecuteResponse & { readonly notice?: string }
   if (result.media && result.media.length > 0) {
     return new McpSchema.CallToolResult({
       content: [
-        McpSchema.TextContent.make({ text: result.text }),
+        McpSchema.TextContent.make({ text: result.isError ? executeErrorText(result) : result.text }),
         ...result.media.map((image) => McpSchema.ImageContent.make({
           data: new Uint8Array(Buffer.from(image.data, "base64")),
           mimeType: image.mimeType,
@@ -299,6 +356,9 @@ export function executeToolResult(value: unknown): McpSchema.CallToolResult {
       isError: result.isError,
     })
   }
+  // A failed execute is text only: a client that prefers structuredContent would otherwise show the context without
+  // the error.
+  if (result.isError) return toolResult({ text: executeToolText(result), isError: true })
   return toolResultForValue(executeToolOutput(result), {
     outputSchema: ExecuteToolOutput,
     text: executeToolText(result),

cli/test/mcp-output-schema.test.ts

@@ -96,7 +96,6 @@ describe("tool output schemas", () => {
   test("the main tools advertise an object-root output schema", () => {
     const byName = new Map(specs().map((spec) => [spec.name, spec]))
     const expected = [
-      "execute",
       "status",
       "session_new",
       "session_allowlist",
@@ -125,6 +124,21 @@ describe("tool output schemas", () => {
     }
   })
 
+  test("execute advertises no output schema, so a client that validates one accepts its image results", () => {
+    const execute = specs().find((spec) => spec.name === "execute")!
+    expect(execute.outputSchema).toBeUndefined()
+    const result = executeToolResult(executeResponse({
+      text: "Returned 1 image",
+      media: [{ type: "image", mimeType: "image/png", data: Buffer.from("png").toString("base64"), size: 3 }],
+    }))
+    expect(result.structuredContent).toBeUndefined()
+    expect(result.content.map((item) => item.type)).toEqual(["text", "image"])
+    // The MCP SDK client rejects a non-error result without structured content from a tool that declares an output
+    // schema. Every screenshot hit that while execute declared one.
+    const rejected = execute.outputSchema !== undefined && result.structuredContent === undefined && !result.isError
+    expect(rejected).toBe(false)
+  })
+
   test("tool annotations describe what each tool can do", () => {
     const byName = new Map(specs().map((spec) => [spec.name, spec]))
     const annotation = (name: string) => {
@@ -358,28 +372,52 @@ describe("tool output schemas", () => {
     expect(textBlocks(result)).toEqual(["42"])
   })
 
-  test("execute errors are MCP errors and keep diagnostics, not an isError field", () => {
+  test("execute errors put their warnings, diagnostic, page movement and logs in the text, with no structured content", () => {
     const result = executeToolResult(executeResponse({
       text: "Error: boom\n    at script:1:1",
       isError: true,
+      logs: [{ source: "script", type: "log", text: "step 1" }, { source: "page", type: "error", text: "bad", repeatCount: 3 }],
       warnings: ["the page navigated"],
+      diagnostic: "execution-context/context-destroyed; pageClosed=false; urlChanged=true; mainFrameNavigations=1",
       aftermath: {
         startUrl: "https://example.com/one",
-        endUrl: "https://example.com/one",
-        navigations: [],
+        endUrl: "https://example.com/two",
+        navigations: ["https://example.com/two"],
         consoleErrorCount: 1,
         pageErrorCount: 0,
         handoffs: 0,
       },
     }))
     expect(result.isError).toBe(true)
+    expect(result.structuredContent).toBeUndefined()
+    expect(textBlocks(result)).toEqual([[
+      "Error: boom\n    at script:1:1",
+      "Warnings:\n- the page navigated",
+      "Diagnostic: execution-context/context-destroyed; pageClosed=false; urlChanged=true; mainFrameNavigations=1",
+      "Page: moved from https://example.com/one to https://example.com/two; 1 navigation; 1 console error",
+      "Logs (2):\n[log] step 1\n[page error] bad (×3)",
+    ].join("\n\n")])
+  })
+
+  test("a bare execute error stays just the message", () => {
+    const result = executeToolResult(executeResponse({ text: "Error: boom\n    at script:1:1", isError: true }))
     expect(textBlocks(result)).toEqual(["Error: boom\n    at script:1:1"])
-    const structured = result.structuredContent as typeof ExecuteToolOutput.Type
-    expect(Schema.is(ExecuteToolOutput)(structured)).toBe(true)
-    expect(structured).not.toHaveProperty("isError")
-    expect(structured).not.toHaveProperty("value")
-    expect(structured.warnings).toEqual(["the page navigated"])
-    expect(structured.aftermath?.consoleErrorCount).toBe(1)
+  })
+
+  test("execute error text stays under 10,000 characters and keeps its warnings", () => {
+    const result = executeToolResult(executeResponse({
+      text: `locator.click: Timeout 30000ms exceeded.\nCall log:\n${"  - retrying click action\n".repeat(2000)}    at execute.js:5:1`,
+      isError: true,
+      logs: Array.from({ length: 500 }, (_, index) => ({ source: "script" as const, type: "log", text: `line ${index} ${"x".repeat(400)}` })),
+      warnings: ["Pointer action was blocked"],
+    }))
+    const text = textBlocks(result).join("")
+    expect(text.length).toBeLessThan(10_000)
+    expect(text).toStartWith("locator.click: Timeout 30000ms exceeded.")
+    expect(text).toContain("at execute.js:5:1")
+    expect(text).toContain("Warnings:\n- Pointer action was blocked")
+    expect(text).toContain("line 499")
+    expect(text).not.toContain("line 479 ")
   })
 
   test("status structured content validates and its fallback text is compact", () => {