diff --git a/.changeset/artifact-tool-call-failures.md b/.changeset/artifact-tool-call-failures.md new file mode 100644 index 0000000000..809ff05552 --- /dev/null +++ b/.changeset/artifact-tool-call-failures.md @@ -0,0 +1,5 @@ +--- +"executor": patch +--- + +**Fix: broken artifact bindings showed empty data instead of an error.** An artifact call is a single tool invocation, so a code-mode `{ ok: false, error }` result is the call's failure — but the shell passed it through as query data, settling the query as success with no payload. Every artifact then coerced that to an empty list, so a deleted or renamed connection read as "0 results, synced just now" rather than an error. The shell now rejects on that envelope, so the component renders its error state. diff --git a/e2e/scenarios/openapi-unreachable-artifact.test.ts b/e2e/scenarios/openapi-unreachable-artifact.test.ts index 1b5adae46e..5ac303d3a4 100644 --- a/e2e/scenarios/openapi-unreachable-artifact.test.ts +++ b/e2e/scenarios/openapi-unreachable-artifact.test.ts @@ -1,7 +1,7 @@ // Cross-target: an artifact whose OpenAPI query cannot reach its upstream gets -// an actionable network error, not the opaque defect mask. This walks the real -// path from a saved artifact through the nested shell, execute-action, sandbox, -// OpenAPI transport, and back into ArtifactError. +// an actionable query error, not a success-looking empty state. This walks the +// real path from a saved artifact through the nested shell, execute-action, +// sandbox, OpenAPI transport, and back into ArtifactError. import { randomBytes } from "node:crypto"; import { createServer } from "node:http"; @@ -107,7 +107,7 @@ const executeApproved = (session: McpSession, code: string) => const artifactSource = (slug: string) => ` function App() { const query = useQuery(tools.${slug}.things.listThings.queryOptions({})); - const result = query.data; + const things = query.data?.data ?? []; return (

Upstream status

@@ -116,10 +116,10 @@ function App() { ) : query.error ? ( - ) : result?.ok === false ? ( - + ) : things.length === 0 ? ( + ) : ( -

Unexpected upstream success

+

{things.length} things returned

)}
@@ -135,7 +135,7 @@ const artifactContent = (page: Page) => page.frameLocator('[data-testid="artifact-shell-frame"]').frameLocator("iframe"); scenario( - "Artifacts · an unreachable OpenAPI host shows actionable retry guidance instead of an internal error", + "Artifacts · a failed tool call shows an error instead of empty data", { timeout: 180_000 }, Effect.scoped( Effect.gen(function* () { @@ -184,12 +184,16 @@ scenario( }); await step( - "The artifact explains that the upstream host could not be reached", + "The failed tool call renders an error instead of an empty state", async () => { const state = artifactContent(page).getByTestId("upstream-state"); await state.locator('[data-slot="artifact-error"]').waitFor({ timeout: 30_000 }); const message = await state.innerText(); + expect( + await state.getByText("No things returned").count(), + "the failure is not empty data", + ).toBe(0); expect(message, "the user gets actionable network guidance").toContain( `Could not reach the upstream server for "${slug}"`, ); diff --git a/packages/hosts/mcp-apps-shell/src/shell/proxy.ts b/packages/hosts/mcp-apps-shell/src/shell/proxy.ts index b0983344d9..2dbeca5517 100644 --- a/packages/hosts/mcp-apps-shell/src/shell/proxy.ts +++ b/packages/hosts/mcp-apps-shell/src/shell/proxy.ts @@ -142,7 +142,22 @@ async function resolveToolResult( return resolveToolResult(app, resumed, requestTrustedInteraction); } - return unwrapResult(structured) ?? parseTextContent(result); + const value = unwrapResult(structured) ?? parseTextContent(result); + + const failure = toolCallFailure(value); + if (failure !== null) throw new Error(failure); + + return value; +} + +function toolCallFailure(value: unknown): string | null { + if (typeof value !== "object" || value === null) return null; + const { ok, error } = value as { ok?: unknown; error?: unknown }; + if (ok !== false) return null; + const { message, code } = (error ?? {}) as { message?: unknown; code?: unknown }; + if (typeof message === "string" && message.length > 0) return message; + if (typeof code === "string" && code.length > 0) return code; + return "Tool call failed"; } function parseTrustedInteraction( diff --git a/packages/hosts/mcp-apps-shell/src/shell/tool-call-failure.test.ts b/packages/hosts/mcp-apps-shell/src/shell/tool-call-failure.test.ts new file mode 100644 index 0000000000..da84498ee2 --- /dev/null +++ b/packages/hosts/mcp-apps-shell/src/shell/tool-call-failure.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from "@effect/vitest"; +import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; + +import { createToolCaller } from "./proxy"; + +describe("artifact tool-call failure surfacing", () => { + const hostReturning = (structuredContent: Record) => ({ + callServerTool: (): Promise => + Promise.resolve({ + content: [{ type: "text", text: "" }], + structuredContent, + }), + }); + + const call = (host: { callServerTool: () => Promise }) => + createToolCaller(host, () => Promise.resolve({ action: "cancel" as const }))( + ["github_com", "search_pull_requests"], + [{ query: "is:open" }], + ); + + it("rejects with the error message of a code-mode failure envelope", async () => { + const host = hostReturning({ + status: "completed", + result: { + ok: false, + error: { + code: "tool_not_found", + message: "Tool not found: github_com.search_pull_requests", + }, + }, + }); + await expect(call(host)).rejects.toThrow("Tool not found: github_com.search_pull_requests"); + }); + + it("falls back to the error code when no message is present", async () => { + const host = hostReturning({ + status: "completed", + result: { ok: false, error: { code: "binding_unresolved" } }, + }); + await expect(call(host)).rejects.toThrow("binding_unresolved"); + }); + + it("passes a successful envelope through as data", async () => { + const host = hostReturning({ + status: "completed", + result: { ok: true, data: { total_count: 2, items: [{ id: 1 }, { id: 2 }] } }, + }); + await expect(call(host)).resolves.toEqual({ + ok: true, + data: { total_count: 2, items: [{ id: 1 }, { id: 2 }] }, + }); + }); +});