Skip to content

Commit b38aa18

Browse files
authored
fix(mcp): preserve thread command rejection reasons (#15627)
1 parent fd1c338 commit b38aa18

4 files changed

Lines changed: 92 additions & 7 deletions

File tree

‎apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,20 @@ describe("orchestrator MCP toolkit", () => {
767767
const invoke = (name: string, args: Record<string, unknown>) =>
768768
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+
770784
const pinned = yield* invoke("t3_thread_organize", { action: "pin" });
771785
expect(pinned.isError).toBe(false);
772786
expect(pinned.structuredContent).toHaveProperty("sequence");
@@ -777,6 +791,20 @@ describe("orchestrator MCP toolkit", () => {
777791
if (parentRun === undefined || parentRun.rootNodeId === null) {
778792
return yield* Effect.die(new Error("Parent run missing."));
779793
}
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+
780808
let parentRootNodeId = parentRun.rootNodeId;
781809
const queueAutomaticCompletion = (suffix: string, taskText: string) =>
782810
Effect.gen(function* () {

‎apps/server/src/mcp/threadAccess.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
import * as Crypto from "effect/Crypto";
1212
import * as Effect from "effect/Effect";
1313

14+
import type { OrchestratorV2Error } from "../orchestration-v2/Orchestrator.ts";
1415
import * as ThreadManagement from "../orchestration-v2/ThreadManagementService.ts";
1516
import * as OrchestrationMcp from "./OrchestratorMcpService.ts";
1617
import * as McpInvocationContext from "./McpInvocationContext.ts";
@@ -21,6 +22,18 @@ export const unavailable = () =>
2122
message: "The operation could not be completed.",
2223
});
2324

25+
/** Decider string rejections are public; wrapped storage and hydration causes are not. */
26+
export 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+
2437
/**
2538
* The most a caller may hand to the threads it targets. A thread caller is
2639
* capped by its own thread's modes; an OAuth client by the ceiling chosen

‎apps/server/src/mcp/toolkits/core.test.ts‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { JsonSchemaType } from "@modelcontextprotocol/sdk/validation";
66
import {
77
DEFAULT_SERVER_SETTINGS,
88
ChatImageAttachment,
9+
CommandId,
910
EnvironmentId,
1011
ProviderInstanceId,
1112
RunId,
@@ -19,8 +20,13 @@ import { McpAttachmentInput } from "./attachment/input.ts";
1920
import { McpSchema, McpServer, Tool } from "effect/ai";
2021
import { FetchHttpClient } from "effect/http";
2122

23+
import {
24+
OrchestratorCommandRejectedError,
25+
OrchestratorDispatchError,
26+
OrchestratorProjectionError,
27+
} from "../../orchestration-v2/Orchestrator.ts";
28+
2229
import * as ServerConfig from "../../config.ts";
23-
import { OrchestratorProjectionError } from "../../orchestration-v2/Orchestrator.ts";
2430
import * as ProviderAdapterRegistry from "../../orchestration-v2/ProviderAdapterRegistry.ts";
2531
import * as ThreadManagement from "../../orchestration-v2/ThreadManagementService.ts";
2632
import * as PreviewBrowser from "../../preview/PreviewBrowser.ts";
@@ -30,6 +36,7 @@ import * as SecretRequests from "../../secrets/SecretRequests.ts";
3036
import * as ScheduledTaskService from "../../scheduledTasks/ScheduledTaskService.ts";
3137
import * as McpHttpServer from "../McpHttpServer.ts";
3238
import * as McpInvocationContext from "../McpInvocationContext.ts";
39+
import { dispatchFailure } from "../threadAccess.ts";
3340
import { OrchestratorToolkit } from "./orchestrator/tools.ts";
3441
import { PreviewToolkit } from "./preview/tools.ts";
3542
import { PreviewControlsToolkit } from "./previewControls/tools.ts";
@@ -202,6 +209,42 @@ it.effect("returns a bounded public failure without serializing storage causes",
202209
),
203210
);
204211

212+
it("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+
205248
it.effect("returns an HTML render reference that Codex and Claude tool rows both carry", () =>
206249
Effect.gen(function* () {
207250
const server = yield* McpServer.McpServer;

‎apps/server/src/mcp/toolkits/thread/handlers.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import * as Effect from "effect/Effect";
1111
import { modelSelectionCommandType } from "@t3tools/shared/model";
1212

1313
import {
14+
dispatchFailure,
1415
newCommandId,
1516
readCaller,
1617
readFullAccessCaller,
@@ -45,7 +46,7 @@ const dispatch = Effect.fn("mcp.dispatchThreadCommand")(function* (
4546
const { threads, projection } = yield* readWritableThread(threadId);
4647
const result = yield* threads
4748
.dispatch(command({ commandId: yield* newCommandId(), threadId: projection.thread.id }))
48-
.pipe(Effect.mapError(unavailable));
49+
.pipe(Effect.mapError(dispatchFailure));
4950
return { sequence: result.sequence };
5051
});
5152

@@ -131,7 +132,7 @@ export const layer = ThreadToolkit.toLayer({
131132
createdBy: "agent",
132133
creationSource: "mcp",
133134
})
134-
.pipe(Effect.mapError(unavailable));
135+
.pipe(Effect.mapError(dispatchFailure));
135136
return { sequence: result.sequence, targetThreadId };
136137
}),
137138
t3_thread_merge_back: (input) =>
@@ -148,7 +149,7 @@ export const layer = ThreadToolkit.toLayer({
148149
createdBy: "agent",
149150
creationSource: "mcp",
150151
})
151-
.pipe(Effect.mapError(unavailable));
152+
.pipe(Effect.mapError(dispatchFailure));
152153
return { sequence: result.sequence, targetThreadId: input.targetThreadId };
153154
}),
154155
t3_thread_transfers: (input) =>
@@ -191,7 +192,7 @@ export const layer = ThreadToolkit.toLayer({
191192
commandId: yield* newCommandId(),
192193
modelSelection: input.modelSelection,
193194
})
194-
.pipe(Effect.mapError(unavailable));
195+
.pipe(Effect.mapError(dispatchFailure));
195196
return { sequence: result.sequence };
196197
}),
197198
t3_pending_request_list: (input) =>
@@ -219,7 +220,7 @@ export const layer = ThreadToolkit.toLayer({
219220
requestId: input.requestId,
220221
answers: input.answers,
221222
})
222-
.pipe(Effect.mapError(unavailable));
223+
.pipe(Effect.mapError(dispatchFailure));
223224
return { sequence: result.sequence };
224225
}),
225226
t3_queue_list: (input) =>
@@ -300,7 +301,7 @@ export const layer = ThreadToolkit.toLayer({
300301
default:
301302
command = { ...common, type: `thread.${input.action}` };
302303
}
303-
const result = yield* threads.dispatch(command).pipe(Effect.mapError(unavailable));
304+
const result = yield* threads.dispatch(command).pipe(Effect.mapError(dispatchFailure));
304305
return { sequence: result.sequence };
305306
}),
306307
});

0 commit comments

Comments
 (0)