From 42f078c717c0c991970e90a994f129a966a10298 Mon Sep 17 00:00:00 2001 From: baggiiiie Date: Mon, 21 Sep 2026 07:26:07 +0000 Subject: [PATCH] fix(mcp-apps-shell): surface artifact tool-call failures instead of empty data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An artifact call is a single `return await tools.…`, so a code-mode `{ ok: false, error }` value is the call's failure, not a value to branch on. The shell proxy passed it through as query data, which settled the query as success with no `.data` — every artifact then coerced it to an empty list, so a deleted or renamed binding read as "0 results, synced just now" instead of an error. Detect the failure envelope after unwrapping and throw, so the query rejects and the component renders ArtifactError. --- .changeset/artifact-tool-call-failures.md | 5 ++ .../openapi-unreachable-artifact.test.ts | 22 ++++---- .../hosts/mcp-apps-shell/src/shell/proxy.ts | 17 +++++- .../src/shell/tool-call-failure.test.ts | 53 +++++++++++++++++++ 4 files changed, 87 insertions(+), 10 deletions(-) create mode 100644 .changeset/artifact-tool-call-failures.md create mode 100644 packages/hosts/mcp-apps-shell/src/shell/tool-call-failure.test.ts 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 }] }, + }); + }); +});