authorgravatar for maria@kuuro.netmaria <maria@kuuro.net> 2026-10-06 14:21:23-03:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-06 14:21:23-03:00
logb38aa18365fe314e079d99670932bedfc7407e5a
tree59310c5f44bff97363c6f55fe0a3165d226e9792
parentfd1c3386c4d60f3477ab3f13c87537848de099f5
signature Signed by PGP key B5690EEEBB952194

fix(mcp): preserve thread command rejection reasons (#15627)

4 files changed, 92 insertions(+), 7 deletions(-)

apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts+28
...@@ -767,6 +767,20 @@ describe("orchestrator MCP toolkit", () => {...@@ -767,6 +767,20 @@ describe("orchestrator MCP toolkit", () => {
767 const invoke = (name: string, args: Record<string, unknown>) =>767 const invoke = (name: string, args: Record<string, unknown>) =>
768 invokeAs(invocation, name, args);768 invokeAs(invocation, name, args);
769769
770 const refusedSettle = yield* invoke("t3_thread_organize", { action: "settle" });
771 expect(refusedSettle.isError).toBe(true);
772 expect(refusedSettle.structuredContent).toBeUndefined();
773 expect(declaredFailure(refusedSettle)).toEqual({
774 _tag: "OrchestratorMcpFailure",
775 code: "orchestration_error",
776 message: `Thread ${parentThreadId} has active or blocked work and cannot be settled.`,
777 });
778 const afterRefusedSettle = yield* orchestrator.getThreadProjection(parentThreadId);
779 expect(afterRefusedSettle.thread.settledOverride).not.toBe("settled");
780 expect(afterRefusedSettle.runs.find((run) => run.id === parentRun?.id)?.status).toBe(
781 "running",
782 );
783
770 const pinned = yield* invoke("t3_thread_organize", { action: "pin" });784 const pinned = yield* invoke("t3_thread_organize", { action: "pin" });
771 expect(pinned.isError).toBe(false);785 expect(pinned.isError).toBe(false);
772 expect(pinned.structuredContent).toHaveProperty("sequence");786 expect(pinned.structuredContent).toHaveProperty("sequence");
...@@ -777,6 +791,20 @@ describe("orchestrator MCP toolkit", () => {...@@ -777,6 +791,20 @@ describe("orchestrator MCP toolkit", () => {
777 if (parentRun === undefined || parentRun.rootNodeId === null) {791 if (parentRun === undefined || parentRun.rootNodeId === null) {
778 return yield* Effect.die(new Error("Parent run missing."));792 return yield* Effect.die(new Error("Parent run missing."));
779 }793 }
794 for (const name of ["t3_queue_edit", "t3_queue_cancel"]) {
795 const refusedQueueMutation = yield* invoke(name, {
796 queuedRunId: parentRun.id,
797 ...(name === "t3_queue_edit" ? { text: "Keep the active turn." } : {}),
798 });
799 expect(refusedQueueMutation.isError).toBe(true);
800 expect(refusedQueueMutation.structuredContent).toBeUndefined();
801 expect(declaredFailure(refusedQueueMutation)).toEqual({
802 _tag: "OrchestratorMcpFailure",
803 code: "orchestration_error",
804 message: `Run ${parentRun.id} is not queued.`,
805 });
806 }
807
780 let parentRootNodeId = parentRun.rootNodeId;808 let parentRootNodeId = parentRun.rootNodeId;
781 const queueAutomaticCompletion = (suffix: string, taskText: string) =>809 const queueAutomaticCompletion = (suffix: string, taskText: string) =>
782 Effect.gen(function* () {810 Effect.gen(function* () {
apps/server/src/mcp/threadAccess.ts+13
...@@ -11,6 +11,7 @@ import {...@@ -11,6 +11,7 @@ import {
11import * as Crypto from "effect/Crypto";11import * as Crypto from "effect/Crypto";
12import * as Effect from "effect/Effect";12import * as Effect from "effect/Effect";
1313
14import type { OrchestratorV2Error } from "../orchestration-v2/Orchestrator.ts";
14import * as ThreadManagement from "../orchestration-v2/ThreadManagementService.ts";15import * as ThreadManagement from "../orchestration-v2/ThreadManagementService.ts";
15import * as OrchestrationMcp from "./OrchestratorMcpService.ts";16import * as OrchestrationMcp from "./OrchestratorMcpService.ts";
16import * as McpInvocationContext from "./McpInvocationContext.ts";17import * as McpInvocationContext from "./McpInvocationContext.ts";
...@@ -21,6 +22,18 @@ export const unavailable = () =>...@@ -21,6 +22,18 @@ export const unavailable = () =>
21 message: "The operation could not be completed.",22 message: "The operation could not be completed.",
22 });23 });
2324
25/** Decider string rejections are public; wrapped storage and hydration causes are not. */
26export const dispatchFailure = (error: OrchestratorV2Error) =>
27 (error._tag === "OrchestratorDispatchError" ||
28 error._tag === "OrchestratorCommandRejectedError") &&
29 typeof error.cause === "string" &&
30 error.cause.length > 0
31 ? new OrchestratorMcpFailure({
32 code: "orchestration_error",
33 message: Array.from(error.cause).slice(0, 1000).join(""),
34 })
35 : unavailable();
36
24/**37/**
25 * The most a caller may hand to the threads it targets. A thread caller is38 * The most a caller may hand to the threads it targets. A thread caller is
26 * capped by its own thread's modes; an OAuth client by the ceiling chosen39 * capped by its own thread's modes; an OAuth client by the ceiling chosen
apps/server/src/mcp/toolkits/core.test.ts+44-1
...@@ -6,6 +6,7 @@ import type { JsonSchemaType } from "@modelcontextprotocol/sdk/validation";...@@ -6,6 +6,7 @@ import type { JsonSchemaType } from "@modelcontextprotocol/sdk/validation";
6import {6import {
7 DEFAULT_SERVER_SETTINGS,7 DEFAULT_SERVER_SETTINGS,
8 ChatImageAttachment,8 ChatImageAttachment,
9 CommandId,
9 EnvironmentId,10 EnvironmentId,
10 ProviderInstanceId,11 ProviderInstanceId,
11 RunId,12 RunId,
...@@ -19,8 +20,13 @@ import { McpAttachmentInput } from "./attachment/input.ts";...@@ -19,8 +20,13 @@ import { McpAttachmentInput } from "./attachment/input.ts";
19import { McpSchema, McpServer, Tool } from "effect/ai";20import { McpSchema, McpServer, Tool } from "effect/ai";
20import { FetchHttpClient } from "effect/http";21import { FetchHttpClient } from "effect/http";
2122
23import {
24 OrchestratorCommandRejectedError,
25 OrchestratorDispatchError,
26 OrchestratorProjectionError,
27} from "../../orchestration-v2/Orchestrator.ts";
28
22import * as ServerConfig from "../../config.ts";29import * as ServerConfig from "../../config.ts";
23import { OrchestratorProjectionError } from "../../orchestration-v2/Orchestrator.ts";
24import * as ProviderAdapterRegistry from "../../orchestration-v2/ProviderAdapterRegistry.ts";30import * as ProviderAdapterRegistry from "../../orchestration-v2/ProviderAdapterRegistry.ts";
25import * as ThreadManagement from "../../orchestration-v2/ThreadManagementService.ts";31import * as ThreadManagement from "../../orchestration-v2/ThreadManagementService.ts";
26import * as PreviewBrowser from "../../preview/PreviewBrowser.ts";32import * as PreviewBrowser from "../../preview/PreviewBrowser.ts";
...@@ -30,6 +36,7 @@ import * as SecretRequests from "../../secrets/SecretRequests.ts";...@@ -30,6 +36,7 @@ import * as SecretRequests from "../../secrets/SecretRequests.ts";
30import * as ScheduledTaskService from "../../scheduledTasks/ScheduledTaskService.ts";36import * as ScheduledTaskService from "../../scheduledTasks/ScheduledTaskService.ts";
31import * as McpHttpServer from "../McpHttpServer.ts";37import * as McpHttpServer from "../McpHttpServer.ts";
32import * as McpInvocationContext from "../McpInvocationContext.ts";38import * as McpInvocationContext from "../McpInvocationContext.ts";
39import { dispatchFailure } from "../threadAccess.ts";
33import { OrchestratorToolkit } from "./orchestrator/tools.ts";40import { OrchestratorToolkit } from "./orchestrator/tools.ts";
34import { PreviewToolkit } from "./preview/tools.ts";41import { PreviewToolkit } from "./preview/tools.ts";
35import { PreviewControlsToolkit } from "./previewControls/tools.ts";42import { PreviewControlsToolkit } from "./previewControls/tools.ts";
...@@ -202,6 +209,42 @@ it.effect("returns a bounded public failure without serializing storage causes",...@@ -202,6 +209,42 @@ it.effect("returns a bounded public failure without serializing storage causes",
202 ),209 ),
203);210);
204211
212it("bounds public command rejections and redacts internal dispatch causes", () => {
213 const command = { commandId: CommandId.make("mcp-core-command"), commandType: "thread.settle" };
214 expect(
215 dispatchFailure(new OrchestratorDispatchError({ ...command, cause: "🙂".repeat(1001) }))
216 .message,
217 ).toBe("🙂".repeat(1000));
218 expect(
219 dispatchFailure(
220 new OrchestratorCommandRejectedError({ ...command, cause: "Run is not queued." }),
221 ).message,
222 ).toBe("Run is not queued.");
223 for (const cause of [
224 undefined,
225 "",
226 new Error("private-storage-path"),
227 { message: "private-storage-path" },
228 ]) {
229 expect(dispatchFailure(new OrchestratorDispatchError({ ...command, cause }))).toMatchObject({
230 code: "orchestration_error",
231 message: "The operation could not be completed.",
232 });
233 expect(
234 dispatchFailure(new OrchestratorCommandRejectedError({ ...command, cause })),
235 ).toMatchObject({
236 code: "orchestration_error",
237 message: "The operation could not be completed.",
238 });
239 }
240 expect(
241 dispatchFailure(new OrchestratorProjectionError({ threadId, cause: "private-storage-path" })),
242 ).toMatchObject({
243 code: "orchestration_error",
244 message: "The operation could not be completed.",
245 });
246});
247
205it.effect("returns an HTML render reference that Codex and Claude tool rows both carry", () =>248it.effect("returns an HTML render reference that Codex and Claude tool rows both carry", () =>
206 Effect.gen(function* () {249 Effect.gen(function* () {
207 const server = yield* McpServer.McpServer;250 const server = yield* McpServer.McpServer;
apps/server/src/mcp/toolkits/thread/handlers.ts+7-6
...@@ -11,6 +11,7 @@ import * as Effect from "effect/Effect";...@@ -11,6 +11,7 @@ import * as Effect from "effect/Effect";
11import { modelSelectionCommandType } from "@t3tools/shared/model";11import { modelSelectionCommandType } from "@t3tools/shared/model";
1212
13import {13import {
14 dispatchFailure,
14 newCommandId,15 newCommandId,
15 readCaller,16 readCaller,
16 readFullAccessCaller,17 readFullAccessCaller,
...@@ -45,7 +46,7 @@ const dispatch = Effect.fn("mcp.dispatchThreadCommand")(function* (...@@ -45,7 +46,7 @@ const dispatch = Effect.fn("mcp.dispatchThreadCommand")(function* (
45 const { threads, projection } = yield* readWritableThread(threadId);46 const { threads, projection } = yield* readWritableThread(threadId);
46 const result = yield* threads47 const result = yield* threads
47 .dispatch(command({ commandId: yield* newCommandId(), threadId: projection.thread.id }))48 .dispatch(command({ commandId: yield* newCommandId(), threadId: projection.thread.id }))
48 .pipe(Effect.mapError(unavailable));49 .pipe(Effect.mapError(dispatchFailure));
49 return { sequence: result.sequence };50 return { sequence: result.sequence };
50});51});
5152
...@@ -131,7 +132,7 @@ export const layer = ThreadToolkit.toLayer({...@@ -131,7 +132,7 @@ export const layer = ThreadToolkit.toLayer({
131 createdBy: "agent",132 createdBy: "agent",
132 creationSource: "mcp",133 creationSource: "mcp",
133 })134 })
134 .pipe(Effect.mapError(unavailable));135 .pipe(Effect.mapError(dispatchFailure));
135 return { sequence: result.sequence, targetThreadId };136 return { sequence: result.sequence, targetThreadId };
136 }),137 }),
137 t3_thread_merge_back: (input) =>138 t3_thread_merge_back: (input) =>
...@@ -148,7 +149,7 @@ export const layer = ThreadToolkit.toLayer({...@@ -148,7 +149,7 @@ export const layer = ThreadToolkit.toLayer({
148 createdBy: "agent",149 createdBy: "agent",
149 creationSource: "mcp",150 creationSource: "mcp",
150 })151 })
151 .pipe(Effect.mapError(unavailable));152 .pipe(Effect.mapError(dispatchFailure));
152 return { sequence: result.sequence, targetThreadId: input.targetThreadId };153 return { sequence: result.sequence, targetThreadId: input.targetThreadId };
153 }),154 }),
154 t3_thread_transfers: (input) =>155 t3_thread_transfers: (input) =>
...@@ -191,7 +192,7 @@ export const layer = ThreadToolkit.toLayer({...@@ -191,7 +192,7 @@ export const layer = ThreadToolkit.toLayer({
191 commandId: yield* newCommandId(),192 commandId: yield* newCommandId(),
192 modelSelection: input.modelSelection,193 modelSelection: input.modelSelection,
193 })194 })
194 .pipe(Effect.mapError(unavailable));195 .pipe(Effect.mapError(dispatchFailure));
195 return { sequence: result.sequence };196 return { sequence: result.sequence };
196 }),197 }),
197 t3_pending_request_list: (input) =>198 t3_pending_request_list: (input) =>
...@@ -219,7 +220,7 @@ export const layer = ThreadToolkit.toLayer({...@@ -219,7 +220,7 @@ export const layer = ThreadToolkit.toLayer({
219 requestId: input.requestId,220 requestId: input.requestId,
220 answers: input.answers,221 answers: input.answers,
221 })222 })
222 .pipe(Effect.mapError(unavailable));223 .pipe(Effect.mapError(dispatchFailure));
223 return { sequence: result.sequence };224 return { sequence: result.sequence };
224 }),225 }),
225 t3_queue_list: (input) =>226 t3_queue_list: (input) =>
...@@ -300,7 +301,7 @@ export const layer = ThreadToolkit.toLayer({...@@ -300,7 +301,7 @@ export const layer = ThreadToolkit.toLayer({
300 default:301 default:
301 command = { ...common, type: `thread.${input.action}` };302 command = { ...common, type: `thread.${input.action}` };
302 }303 }
303 const result = yield* threads.dispatch(command).pipe(Effect.mapError(unavailable));304 const result = yield* threads.dispatch(command).pipe(Effect.mapError(dispatchFailure));
304 return { sequence: result.sequence };305 return { sequence: result.sequence };
305 }),306 }),
306});307});