Skip to content

Commit 5f6dec0

Browse files
committed
fix(app): registerTool edge cases + example tool correctness
App.registerTool: - Skip outputSchema validation when result.isError (matches AppToolResult type) - Pin callback arity to original config.inputSchema; only the validation schema is mutable via update() - Default missing tools/call arguments to {} - remove() is a no-op if the handle is stale (re-registered or already removed) - Gate list_changed notifications on the listChanged capability Examples: - threejs: propagate executeThreeCode failures to onSceneError/onSceneRendering; catch updateModelContext rejection - wiki-explorer: search-article preserves graph on no-match - budget-allocator: fix category/stage values in tool descriptions
1 parent 68fc188 commit 5f6dec0

6 files changed

Lines changed: 88 additions & 29 deletions

File tree

examples/budget-allocator-server/src/mcp-app.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,7 @@ app.registerTool(
685685
categoryId: z
686686
.string()
687687
.describe(
688-
"Category ID (e.g., 'rd', 'sales', 'marketing', 'ops', 'ga')",
688+
"Category ID (e.g., 'marketing', 'engineering', 'operations', 'sales', 'rd')",
689689
),
690690
percent: z
691691
.number()
@@ -795,7 +795,7 @@ app.registerTool(
795795
{
796796
title: "Set Company Stage",
797797
description:
798-
"Set the company stage for benchmark comparison (seed, series_a, series_b, growth)",
798+
"Set the company stage for benchmark comparison (e.g., 'Seed', 'Series A', 'Series B', 'Growth')",
799799
inputSchema: z.object({
800800
stage: z.string().describe("Company stage ID"),
801801
}),

examples/threejs-server/src/mcp-app-wrapper.tsx

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -272,19 +272,21 @@ function McpAppWrapper() {
272272

273273
// Send errors to model context for awareness
274274
if (sceneError && appRef.current) {
275-
appRef.current.updateModelContext({
276-
content: [
277-
{
278-
type: "text" as const,
279-
text: `Three.js Scene Error: ${sceneError}`,
275+
void appRef.current
276+
.updateModelContext({
277+
content: [
278+
{
279+
type: "text" as const,
280+
text: `Three.js Scene Error: ${sceneError}`,
281+
},
282+
],
283+
structuredContent: {
284+
type: "scene_error",
285+
error: sceneError,
286+
timestamp: new Date().toISOString(),
280287
},
281-
],
282-
structuredContent: {
283-
type: "scene_error",
284-
error: sceneError,
285-
timestamp: new Date().toISOString(),
286-
},
287-
});
288+
})
289+
.catch(() => {});
288290
}
289291
}, []);
290292

examples/threejs-server/src/threejs-app.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,12 +321,18 @@ export default function ThreeJSApp({
321321
w,
322322
h,
323323
animControllerRef.current.visibilityAwareRAF,
324-
).catch((e) => setError(e instanceof Error ? e.message : "Unknown error"));
324+
).catch((e) => {
325+
const msg = e instanceof Error ? e.message : "Unknown error";
326+
setError(msg);
327+
onSceneError(msg);
328+
onSceneRendering(false);
329+
});
325330

326331
return () => {
327332
canvas.removeEventListener("touchstart", preventDefault);
328333
canvas.removeEventListener("touchmove", preventDefault);
329334
animControllerRef.current?.cleanup();
335+
onSceneRendering(false);
330336
};
331337
}, [code, height, containerWidth, isFullscreen, hostHeight]);
332338

examples/wiki-explorer-server/src/mcp-app.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -425,12 +425,11 @@ app.registerTool(
425425
arguments: { url: searchUrl },
426426
});
427427

428-
// Clear existing graph and start fresh with this article
429-
graphData.nodes = [];
430-
graphData.links = [];
431-
432428
const response = result.structuredContent as unknown as ToolResponse;
433429
if (response && response.page) {
430+
// Clear existing graph and start fresh with this article
431+
graphData.nodes = [];
432+
graphData.links = [];
434433
initialUrl = response.page.url;
435434
addNode(response.page.url, response.page.title, "default", {
436435
x: 0,

src/app-bridge.test.ts

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -692,8 +692,11 @@ describe("App <-> AppBridge integration", () => {
692692

693693
describe("App tool registration", () => {
694694
beforeEach(async () => {
695-
// App needs tool capabilities to register tools
696-
app = new App(testAppInfo, { tools: {} }, { autoResize: false });
695+
app = new App(
696+
testAppInfo,
697+
{ tools: { listChanged: true } },
698+
{ autoResize: false },
699+
);
697700
await bridge.connect(bridgeTransport);
698701
});
699702

@@ -799,6 +802,46 @@ describe("App <-> AppBridge integration", () => {
799802
expect(receivedExtra.signal).toBeInstanceOf(AbortSignal);
800803
});
801804

805+
it("isError result skips output schema validation", async () => {
806+
await app.connect(appTransport);
807+
app.registerTool(
808+
"errs",
809+
{ outputSchema: z.object({ ok: z.boolean() }) },
810+
async () => ({
811+
content: [{ type: "text" as const, text: "boom" }],
812+
isError: true,
813+
}),
814+
);
815+
const res = await bridge.callTool({ name: "errs", arguments: {} });
816+
expect(res.isError).toBe(true);
817+
expect(res.structuredContent).toBeUndefined();
818+
});
819+
820+
it("stale handle remove() does not delete a re-registered tool", async () => {
821+
const t1 = app.registerTool("phoenix", {}, async () => ({ content: [] }));
822+
t1.remove();
823+
app.registerTool("phoenix", {}, async () => ({ content: [] }));
824+
t1.remove();
825+
await app.connect(appTransport);
826+
const list = await bridge.listTools({});
827+
expect(list.tools.map((t) => t.name)).toContain("phoenix");
828+
});
829+
830+
it("host omitting arguments defaults to empty object", async () => {
831+
await app.connect(appTransport);
832+
let received: unknown;
833+
app.registerTool(
834+
"noargs2",
835+
{ inputSchema: z.object({}) },
836+
async (args) => {
837+
received = args;
838+
return { content: [] };
839+
},
840+
);
841+
await bridge.callTool({ name: "noargs2" });
842+
expect(received).toEqual({});
843+
});
844+
802845
it("update({inputSchema}) is honored by handler validation", async () => {
803846
await app.connect(appTransport);
804847
const tool = app.registerTool(

src/app.ts

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -400,8 +400,13 @@ export class App extends ProtocolWithEvents<
400400
}
401401
const app = this;
402402
const notify = () => {
403-
if (app.transport) void app.sendToolListChanged();
403+
if (app.transport && app._capabilities.tools?.listChanged) {
404+
void app.sendToolListChanged();
405+
}
404406
};
407+
// Arity is fixed at registration time even if inputSchema is later
408+
// update()d, since cb's signature can't change.
409+
const cbTakesArgs = config.inputSchema !== undefined;
405410
const registeredTool: RegisteredAppTool = {
406411
title: config.title,
407412
description: config.description,
@@ -423,6 +428,7 @@ export class App extends ProtocolWithEvents<
423428
notify();
424429
},
425430
remove() {
431+
if (app._registeredTools[name] !== registeredTool) return;
426432
delete app._registeredTools[name];
427433
notify();
428434
},
@@ -431,12 +437,15 @@ export class App extends ProtocolWithEvents<
431437
throw new Error(`Tool ${name} is disabled`);
432438
}
433439
let result: CallToolResult;
434-
if (registeredTool.inputSchema) {
435-
const parsedArgs = await validateStandardSchema(
436-
registeredTool.inputSchema,
437-
rawArgs,
438-
`Invalid input for tool ${name}: `,
439-
);
440+
if (cbTakesArgs) {
441+
const schema = registeredTool.inputSchema;
442+
const parsedArgs = schema
443+
? await validateStandardSchema(
444+
schema,
445+
rawArgs ?? {},
446+
`Invalid input for tool ${name}: `,
447+
)
448+
: (rawArgs ?? {});
440449
result = await (
441450
cb as AppToolCallback<StandardSchemaV1, StandardSchemaV1>
442451
)(parsedArgs, extra);
@@ -445,7 +454,7 @@ export class App extends ProtocolWithEvents<
445454
extra,
446455
);
447456
}
448-
if (registeredTool.outputSchema) {
457+
if (registeredTool.outputSchema && !result.isError) {
449458
result.structuredContent = (await validateStandardSchema(
450459
registeredTool.outputSchema,
451460
result.structuredContent,

0 commit comments

Comments
 (0)