authorgravatar for maria@kuuro.netmaria <maria@kuuro.net> 2026-10-06 13:57:00-03:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-06 13:57:00-03:00
logdff412c3411f34b3150983f00686a98b600526e3
treef082ed5b061fb7a8f144733dfad583d255831a6d
parent17c0878941ab8ab19108fb639c5138d92a89e919
signature Signed by PGP key B5690EEEBB952194

fix(mcp): mark declared tool failures as errors (#15617)

4 files changed, 104 insertions(+), 0 deletions(-)

apps/server/package.json+1
...@@ -51,6 +51,7 @@...@@ -51,6 +51,7 @@
51 },51 },
52 "devDependencies": {52 "devDependencies": {
53 "@effect/vitest": "catalog:",53 "@effect/vitest": "catalog:",
54 "@modelcontextprotocol/sdk": "1.29.0",
54 "@t3tools/contracts": "workspace:*",55 "@t3tools/contracts": "workspace:*",
55 "@t3tools/shared": "workspace:*",56 "@t3tools/shared": "workspace:*",
56 "@t3tools/ssh": "workspace:*",57 "@t3tools/ssh": "workspace:*",
apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts+29
...@@ -1,5 +1,7 @@...@@ -1,5 +1,7 @@
1import * as NodeServices from "@effect/platform-node/NodeServices";1import * as NodeServices from "@effect/platform-node/NodeServices";
2import { describe, expect, it } from "@effect/vitest";2import { describe, expect, it } from "@effect/vitest";
3import { AjvJsonSchemaValidator } from "@modelcontextprotocol/sdk/validation/ajv";
4import type { JsonSchemaType } from "@modelcontextprotocol/sdk/validation";
3import {5import {
4 CommandId,6 CommandId,
5 EnvironmentId,7 EnvironmentId,
...@@ -766,6 +768,7 @@ describe("orchestrator MCP toolkit", () => {...@@ -766,6 +768,7 @@ describe("orchestrator MCP toolkit", () => {
766 invokeAs(invocation, name, args);768 invokeAs(invocation, name, args);
767769
768 const pinned = yield* invoke("t3_thread_organize", { action: "pin" });770 const pinned = yield* invoke("t3_thread_organize", { action: "pin" });
771 expect(pinned.isError).toBe(false);
769 expect(pinned.structuredContent).toHaveProperty("sequence");772 expect(pinned.structuredContent).toHaveProperty("sequence");
770 expect((yield* orchestrator.getThreadShell(parentThreadId))?.pinnedAt).not.toBeNull();773 expect((yield* orchestrator.getThreadShell(parentThreadId))?.pinnedAt).not.toBeNull();
771 yield* invoke("t3_thread_organize", { action: "unpin" });774 yield* invoke("t3_thread_organize", { action: "unpin" });
...@@ -1211,10 +1214,36 @@ describe("orchestrator MCP toolkit", () => {...@@ -1211,10 +1214,36 @@ describe("orchestrator MCP toolkit", () => {
1211 return yield* Effect.die(new Error("Queued user follow-up missing."));1214 return yield* Effect.die(new Error("Queued user follow-up missing."));
1212 }1215 }
1213 const queueFirstPage = yield* invoke("t3_queue_list", { limit: 1 });1216 const queueFirstPage = yield* invoke("t3_queue_list", { limit: 1 });
1217 expect(queueFirstPage.isError).toBe(false);
1214 expect(queueFirstPage.structuredContent).toMatchObject({1218 expect(queueFirstPage.structuredContent).toMatchObject({
1215 items: [{ queuedRunId: queueRace.queuedRun.id }],1219 items: [{ queuedRunId: queueRace.queuedRun.id }],
1216 nextCursor: 1,1220 nextCursor: 1,
1217 });1221 });
1222 const queueDefinition = server.tools.find(({ tool }) => tool.name === "t3_queue_list");
1223 const validateQueue = new AjvJsonSchemaValidator().getValidator(
1224 queueDefinition!.tool.outputSchema! as JsonSchemaType,
1225 );
1226 expect(validateQueue(queueFirstPage.structuredContent).valid).toBe(true);
1227 const missingThreadId = ThreadId.make("00000000-0000-4000-8000-000000000000");
1228 const missingThreadQueue = yield* invoke("t3_queue_list", {
1229 threadId: missingThreadId,
1230 limit: 1,
1231 });
1232 expect(missingThreadQueue.isError).toBe(true);
1233 expect(declaredFailure(missingThreadQueue)).toMatchObject({
1234 _tag: "OrchestratorMcpFailure",
1235 code: "thread_not_found",
1236 message: "The thread was not found.",
1237 });
1238 expect(missingThreadQueue.structuredContent).toBeUndefined();
1239 expect(validateQueue({ items: "invalid", nextCursor: null }).valid).toBe(false);
1240 const missingThreadRead = yield* invoke("t3_thread_read", {
1241 threadId: missingThreadId,
1242 });
1243 expect(missingThreadRead.isError).toBe(true);
1244 expect(declaredFailure(missingThreadRead)).toMatchObject({
1245 _tag: "OrchestratorMcpFailure",
1246 });
1218 const queueSecondPage = yield* invoke("t3_queue_list", { cursor: 1, limit: 1 });1247 const queueSecondPage = yield* invoke("t3_queue_list", { cursor: 1, limit: 1 });
1219 expect(queueSecondPage.structuredContent).toEqual({1248 expect(queueSecondPage.structuredContent).toEqual({
1220 items: [{ queuedRunId: queuedUserRun.id, text: "🙂".repeat(1000), truncated: true }],1249 items: [{ queuedRunId: queuedUserRun.id, text: "🙂".repeat(1000), truncated: true }],
apps/server/src/mcp/toolkits/core.test.ts+71
...@@ -1,6 +1,8 @@...@@ -1,6 +1,8 @@
1import * as NodeCrypto from "@effect/platform-node/NodeCrypto";1import * as NodeCrypto from "@effect/platform-node/NodeCrypto";
2import * as NodeServices from "@effect/platform-node/NodeServices";2import * as NodeServices from "@effect/platform-node/NodeServices";
3import { expect, it } from "@effect/vitest";3import { expect, it } from "@effect/vitest";
4import { AjvJsonSchemaValidator } from "@modelcontextprotocol/sdk/validation/ajv";
5import type { JsonSchemaType } from "@modelcontextprotocol/sdk/validation";
4import {6import {
5 DEFAULT_SERVER_SETTINGS,7 DEFAULT_SERVER_SETTINGS,
6 ChatImageAttachment,8 ChatImageAttachment,
...@@ -136,6 +138,8 @@ it.effect("checks capability before accessing services through the production re...@@ -136,6 +138,8 @@ it.effect("checks capability before accessing services through the production re
136 Effect.provideService(McpSchema.McpServerClient, client),138 Effect.provideService(McpSchema.McpServerClient, client),
137 );139 );
138 expect(declaredFailure(result)).toMatchObject({ code: "capability_denied" });140 expect(declaredFailure(result)).toMatchObject({ code: "capability_denied" });
141 expect(result.isError).toBe(true);
142 expect(result.structuredContent).toBeUndefined();
139 }).pipe(143 }).pipe(
140 Effect.provide(144 Effect.provide(
141 McpHttpServer.layerThreadToolkit.pipe(145 McpHttpServer.layerThreadToolkit.pipe(
...@@ -161,6 +165,22 @@ it.effect("returns a bounded public failure without serializing storage causes",...@@ -161,6 +165,22 @@ it.effect("returns a bounded public failure without serializing storage causes",
161 code: "orchestration_error",165 code: "orchestration_error",
162 message: "The operation could not be completed.",166 message: "The operation could not be completed.",
163 });167 });
168 expect(result.isError).toBe(true);
169 expect(result.content).toEqual([
170 {
171 type: "text",
172 text: '{"_tag":"OrchestratorMcpFailure","code":"orchestration_error","message":"The operation could not be completed."}',
173 },
174 ]);
175 const definition = server.tools.find(({ tool }) => tool.name === "t3_thread_organize");
176 expect(definition?.tool.outputSchema).toBeDefined();
177 const validate = new AjvJsonSchemaValidator().getValidator(
178 definition!.tool.outputSchema! as JsonSchemaType,
179 );
180 expect(result.structuredContent).toBeUndefined();
181 expect(validate({ sequence: 1 }).valid).toBe(true);
182 expect(validate({ code: "orchestration_error" }).valid).toBe(false);
183 expect(validate({ sequence: "invalid" }).valid).toBe(false);
164 }).pipe(184 }).pipe(
165 Effect.provide(185 Effect.provide(
166 McpHttpServer.layerThreadToolkit.pipe(186 McpHttpServer.layerThreadToolkit.pipe(
...@@ -229,6 +249,57 @@ it.effect("returns an HTML render reference that Codex and Claude tool rows both...@@ -229,6 +249,57 @@ it.effect("returns an HTML render reference that Codex and Claude tool rows both
229 ),249 ),
230);250);
231251
252it.effect("returns invalid parameter errors through the production registration", () =>
253 Effect.gen(function* () {
254 const server = yield* McpServer.McpServer;
255 const error = yield* server
256 .callTool({ name: "t3_thread_organize", arguments: { action: "invalid" } })
257 .pipe(
258 Effect.provideService(McpInvocationContext.McpInvocationContext, scope),
259 Effect.provideService(McpSchema.McpServerClient, client),
260 Effect.flip,
261 );
262 expect(error._tag).toBe("InvalidParams");
263 }).pipe(
264 Effect.provide(
265 McpHttpServer.layerThreadToolkit.pipe(
266 Layer.provideMerge(McpServer.McpServer.layer),
267 Layer.provide(NodeCrypto.layer),
268 Layer.provide(Layer.mock(ThreadManagement.ThreadManagementService)({})),
269 ),
270 ),
271 ),
272);
273
274it.effect("keeps unexpected handler defects private through the production registration", () =>
275 Effect.gen(function* () {
276 const server = yield* McpServer.McpServer;
277 const result = yield* server
278 .callTool({ name: "t3_thread_organize", arguments: { action: "pin" } })
279 .pipe(
280 Effect.provideService(McpInvocationContext.McpInvocationContext, scope),
281 Effect.provideService(McpSchema.McpServerClient, client),
282 );
283 expect(result.isError).toBe(true);
284 expect(result.structuredContent).toBeUndefined();
285 expect(result.content).toEqual([
286 { type: "text", text: "Tool execution failed due to an internal server error." },
287 ]);
288 }).pipe(
289 Effect.provide(
290 McpHttpServer.layerThreadToolkit.pipe(
291 Layer.provideMerge(McpServer.McpServer.layer),
292 Layer.provide(NodeCrypto.layer),
293 Layer.provide(
294 Layer.mock(ThreadManagement.ThreadManagementService)({
295 getThreadShell: () => Effect.die(new Error("private-storage-path")),
296 }),
297 ),
298 ),
299 ),
300 ),
301);
302
232it("keeps MCP preference output allowlisted and Unicode-bounded", () => {303it("keeps MCP preference output allowlisted and Unicode-bounded", () => {
233 const settings = {304 const settings = {
234 ...DEFAULT_SERVER_SETTINGS,305 ...DEFAULT_SERVER_SETTINGS,
pnpm-lock.yaml+3
...@@ -586,6 +586,9 @@ importers:...@@ -586,6 +586,9 @@ importers:
586 '@effect/vitest':586 '@effect/vitest':
587 specifier: 4.0.1587 specifier: 4.0.1
588 version: 4.0.1(patch_hash=359f6fb2f7b3ec145bb72208edb9034f02489791aa2491a55cdbd69bd56ee0d2)(@types/node@24.12.4)(@vitest/ui@5.0.1)(bufferutil@4.1.0)(effect@4.0.1(patch_hash=a33cba07c41f32374c2aaa86ea4a84d3ab9a872a78c94b65e929b19f8361856f))(esbuild@0.28.2)(jiti@2.7.0)(jsdom@30.1.0(@noble/hashes@1.8.0))(msw@2.12.11(@types/node@24.12.4)(typescript@7.0.2))(terser@5.48.0)(typescript@7.0.2)(unrun@0.2.39)(utf-8-validate@6.0.6)(yaml@2.9.0)588 version: 4.0.1(patch_hash=359f6fb2f7b3ec145bb72208edb9034f02489791aa2491a55cdbd69bd56ee0d2)(@types/node@24.12.4)(@vitest/ui@5.0.1)(bufferutil@4.1.0)(effect@4.0.1(patch_hash=a33cba07c41f32374c2aaa86ea4a84d3ab9a872a78c94b65e929b19f8361856f))(esbuild@0.28.2)(jiti@2.7.0)(jsdom@30.1.0(@noble/hashes@1.8.0))(msw@2.12.11(@types/node@24.12.4)(typescript@7.0.2))(terser@5.48.0)(typescript@7.0.2)(unrun@0.2.39)(utf-8-validate@6.0.6)(yaml@2.9.0)
589 '@modelcontextprotocol/sdk':
590 specifier: 1.29.0
591 version: 1.29.0(zod@4.6.5)
589 '@t3tools/contracts':592 '@t3tools/contracts':
590 specifier: workspace:*593 specifier: workspace:*
591 version: link:../../packages/contracts594 version: link:../../packages/contracts