Skip to content

Commit 9071855

Browse files
committed
fix(mcp-apps-shell): surface artifact tool-call failures instead of empty data
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.
1 parent 9e143d9 commit 9071855

4 files changed

Lines changed: 87 additions & 10 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"executor": patch
3+
---
4+
5+
**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.

‎e2e/scenarios/openapi-unreachable-artifact.test.ts‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Cross-target: an artifact whose OpenAPI query cannot reach its upstream gets
2-
// an actionable network error, not the opaque defect mask. This walks the real
3-
// path from a saved artifact through the nested shell, execute-action, sandbox,
4-
// OpenAPI transport, and back into ArtifactError.
2+
// an actionable query error, not a success-looking empty state. This walks the
3+
// real path from a saved artifact through the nested shell, execute-action,
4+
// sandbox, OpenAPI transport, and back into ArtifactError.
55
import { randomBytes } from "node:crypto";
66
import { createServer } from "node:http";
77

@@ -107,7 +107,7 @@ const executeApproved = (session: McpSession, code: string) =>
107107
const artifactSource = (slug: string) => `
108108
function App() {
109109
const query = useQuery(tools.${slug}.things.listThings.queryOptions({}));
110-
const result = query.data;
110+
const things = query.data?.data ?? [];
111111
return (
112112
<div className="flex h-full flex-col gap-4">
113113
<h2>Upstream status</h2>
@@ -116,10 +116,10 @@ function App() {
116116
<ArtifactLoading />
117117
) : query.error ? (
118118
<ArtifactError error={query.error} onRetry={query.refetch} />
119-
) : result?.ok === false ? (
120-
<ArtifactError error={result.error} onRetry={query.refetch} />
119+
) : things.length === 0 ? (
120+
<ArtifactEmpty title="No things returned" />
121121
) : (
122-
<p>Unexpected upstream success</p>
122+
<p>{things.length} things returned</p>
123123
)}
124124
</div>
125125
</div>
@@ -135,7 +135,7 @@ const artifactContent = (page: Page) =>
135135
page.frameLocator('[data-testid="artifact-shell-frame"]').frameLocator("iframe");
136136

137137
scenario(
138-
"Artifacts · an unreachable OpenAPI host shows actionable retry guidance instead of an internal error",
138+
"Artifacts · a failed tool call shows an error instead of empty data",
139139
{ timeout: 180_000 },
140140
Effect.scoped(
141141
Effect.gen(function* () {
@@ -184,12 +184,16 @@ scenario(
184184
});
185185

186186
await step(
187-
"The artifact explains that the upstream host could not be reached",
187+
"The failed tool call renders an error instead of an empty state",
188188
async () => {
189189
const state = artifactContent(page).getByTestId("upstream-state");
190190
await state.locator('[data-slot="artifact-error"]').waitFor({ timeout: 30_000 });
191191
const message = await state.innerText();
192192

193+
expect(
194+
await state.getByText("No things returned").count(),
195+
"the failure is not empty data",
196+
).toBe(0);
193197
expect(message, "the user gets actionable network guidance").toContain(
194198
`Could not reach the upstream server for "${slug}"`,
195199
);

‎packages/hosts/mcp-apps-shell/src/shell/proxy.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,22 @@ async function resolveToolResult(
142142
return resolveToolResult(app, resumed, requestTrustedInteraction);
143143
}
144144

145-
return unwrapResult(structured) ?? parseTextContent(result);
145+
const value = unwrapResult(structured) ?? parseTextContent(result);
146+
147+
const failure = toolCallFailure(value);
148+
if (failure !== null) throw new Error(failure);
149+
150+
return value;
151+
}
152+
153+
function toolCallFailure(value: unknown): string | null {
154+
if (typeof value !== "object" || value === null) return null;
155+
const { ok, error } = value as { ok?: unknown; error?: unknown };
156+
if (ok !== false) return null;
157+
const { message, code } = (error ?? {}) as { message?: unknown; code?: unknown };
158+
if (typeof message === "string" && message.length > 0) return message;
159+
if (typeof code === "string" && code.length > 0) return code;
160+
return "Tool call failed";
146161
}
147162

148163
function parseTrustedInteraction(
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { describe, expect, it } from "@effect/vitest";
2+
import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
3+
4+
import { createToolCaller } from "./proxy";
5+
6+
describe("artifact tool-call failure surfacing", () => {
7+
const hostReturning = (structuredContent: Record<string, unknown>) => ({
8+
callServerTool: (): Promise<CallToolResult> =>
9+
Promise.resolve({
10+
content: [{ type: "text", text: "" }],
11+
structuredContent,
12+
}),
13+
});
14+
15+
const call = (host: { callServerTool: () => Promise<CallToolResult> }) =>
16+
createToolCaller(host, () => Promise.resolve({ action: "cancel" as const }))(
17+
["github_com", "search_pull_requests"],
18+
[{ query: "is:open" }],
19+
);
20+
21+
it("rejects with the error message of a code-mode failure envelope", async () => {
22+
const host = hostReturning({
23+
status: "completed",
24+
result: {
25+
ok: false,
26+
error: {
27+
code: "tool_not_found",
28+
message: "Tool not found: github_com.search_pull_requests",
29+
},
30+
},
31+
});
32+
await expect(call(host)).rejects.toThrow("Tool not found: github_com.search_pull_requests");
33+
});
34+
35+
it("falls back to the error code when no message is present", async () => {
36+
const host = hostReturning({
37+
status: "completed",
38+
result: { ok: false, error: { code: "binding_unresolved" } },
39+
});
40+
await expect(call(host)).rejects.toThrow("binding_unresolved");
41+
});
42+
43+
it("passes a successful envelope through as data", async () => {
44+
const host = hostReturning({
45+
status: "completed",
46+
result: { ok: true, data: { total_count: 2, items: [{ id: 1 }, { id: 2 }] } },
47+
});
48+
await expect(call(host)).resolves.toEqual({
49+
ok: true,
50+
data: { total_count: 2, items: [{ id: 1 }, { id: 2 }] },
51+
});
52+
});
53+
});

0 commit comments

Comments
 (0)