feat(mcp): update thread metadata#8690

merged5
t3code/codex-turn-mapping
agents/mcp-thread-metadata/update
openedAug 29, 2026byJJulius Marminge
View on GitHubTwo-way sync enabledPR last imported 4d ago
4 unresolved threads · Jump to file
Selected thread: apps/server/src/mcp/ThreadMetadataMcpService.ts:153 — <!-- MURMUR_IGNORE --> 🟡 **Medium** `mcp/ThreadMetadataMcpService.ts:153` When the calling thread no longer exists, `getThreadProjection(scope.threadId)` fails with `OrchestratorProjectionError`, but this mapper always returns `orchestration_error`. The update tool therefore reports a server failure instead of `thread_not_found`, so MCP clients cannot handle stale calling threads; map the unavailable-thread projection error to `thread_not_found`. ✅ Resolved in a965fc4eda71a107953d2c01f013b2b6755b247d <!-- macroscope-meta: {"kind":"code_review"} -->
0 / 16 viewed16 files changedLine totals unavailable

docs/orchestration-v2/orchestrator-mcp-server.md · Earlier revision 4736cd66 · right 393–393

Original diff
@@ -376,7 +391,8 @@ orchestration_error
 ## Code Ownership
 
 - Shared schemas: `packages/contracts/src/orchestratorMcp.ts`

docs/orchestration-v2/orchestrator-mcp-server.md · right 393–393

M
macroscopeappBotAug 30, 2026

🟢 Low orchestration-v2/orchestrator-mcp-server.md:393

The ownership list points maintainers only to packages/contracts/src/orchestratorMcp.ts, so they cannot find the ThreadMetadataMcpUpdateInput and result contract used by ThreadMetadataMcpService. Add packages/contracts/src/threadMetadataMcp.ts to the shared schemas entry.

No longer relevant as of a965fc4eda71a107953d2c01f013b2b6755b247d

apps/server/src/mcp/ThreadMetadataMcpService.ts · Earlier revision 4736cd66 · right 153

Original diff
@@ -0,0 +1,218 @@
+import {
+  CommandId,
+  OrchestratorMcpFailure,
+  type OrchestrationV2AppThread,
+  type OrchestrationV2Command,
+  type ThreadMetadataMcpAction,
+  type ThreadMetadataMcpUpdateInput,
+  type ThreadMetadataMcpUpdateResult,
+  ThreadId,
+} from "@t3tools/contracts";
+import * as Context from "effect/Context";
+import * as Crypto from "effect/Crypto";
+import * as DateTime from "effect/DateTime";
+import * as Effect from "effect/Effect";
+import * as Layer from "effect/Layer";
+
+import {
+  ThreadManagementError,
+  ThreadManagementService,
+} from "../orchestration-v2/ThreadManagementService.ts";
+import type { McpInvocationScope } from "./McpInvocationContext.ts";
+
+export class ThreadMetadataMcpService extends Context.Service<
+  ThreadMetadataMcpService,
+  {
+    readonly update: (
+      scope: McpInvocationScope,
+      input: ThreadMetadataMcpUpdateInput,
+    ) => Effect.Effect<ThreadMetadataMcpUpdateResult, OrchestratorMcpFailure>;
+  }
+>()("t3/mcp/ThreadMetadataMcpService") {}
+
+function failure(code: OrchestratorMcpFailure["code"], message: string): OrchestratorMcpFailure {
+  return new OrchestratorMcpFailure({ code, message });
+}
+
+function errorMessage(error: unknown): string {
+  return error instanceof Error ? error.message : String(error);
+}
+
+function threadLookupFailure(error: ThreadManagementError): OrchestratorMcpFailure {
+  return error._tag === "ThreadManagementThreadNotFoundError"
+    ? failure("thread_not_found", error.message)
+    : failure("orchestration_error", error.message);
+}
+
+function stablePart(value: string): string {
+  return encodeURIComponent(value);
+}
+
+function commandId(input: {
+  readonly scope: McpInvocationScope;
+  readonly threadId: ThreadId;
+  readonly action: ThreadMetadataMcpAction;
+  readonly requestKey: string;
+}): CommandId {
+  return CommandId.make(
+    [
+      "command",
+      "mcp",
+      stablePart(input.scope.providerSessionId),
+      "thread-update",
+      stablePart(input.threadId),
+      stablePart(input.action),
+      stablePart(input.requestKey),
+    ].join(":"),
+  );
+}
+
+function metadataCommand(input: {
+  readonly commandId: CommandId;
+  readonly threadId: ThreadId;
+  readonly projectId: OrchestrationV2AppThread["projectId"];
+  readonly update: ThreadMetadataMcpUpdateInput;
+}): Extract<OrchestrationV2Command, { readonly type: "thread.metadata.update" }> {
+  switch (input.update.action) {
+    case "rename":
+      return {
+        type: "thread.metadata.update",
+        commandId: input.commandId,
+        threadId: input.threadId,
+        title: input.update.title!,
+      };
+    case "regenerate_title":
+      return {
+        type: "thread.metadata.update",
+        commandId: input.commandId,
+        threadId: input.threadId,
+        regenerateTitle: true,
+      };
+    case "link_pull_request":
+      return {
+        type: "thread.metadata.update",
+        commandId: input.commandId,
+        threadId: input.threadId,
+        linkedPullRequest: {
+          projectId: input.projectId,
+          ...input.update.pullRequest!,
+        },
+      };
+    case "unlink_pull_request":
+      return {
+        type: "thread.metadata.update",
+        commandId: input.commandId,
+        threadId: input.threadId,
+        linkedPullRequest: null,
+      };
+  }
+}
+
+function resultFromThread(input: {
+  readonly action: ThreadMetadataMcpAction;
+  readonly commandId: CommandId;
+  readonly sequence: number;
+  readonly thread: OrchestrationV2AppThread;
+}): ThreadMetadataMcpUpdateResult {
+  return {
+    threadId: input.thread.id,
+    action: input.action,
+    commandId: input.commandId,
+    sequence: input.sequence,
+    title: input.thread.title,
+    titleRegeneration:
+      input.thread.titleRegeneration === undefined || input.thread.titleRegeneration === null
+        ? null
+        : {
+            requestId: input.thread.titleRegeneration.requestId,
+            startedAt: DateTime.formatIso(input.thread.titleRegeneration.startedAt),
+          },
+    linkedPullRequest: input.thread.linkedPullRequest ?? null,
+    updatedAt: DateTime.formatIso(input.thread.updatedAt),
+  };
+}
+
+const make = Effect.gen(function* () {
+  const crypto = yield* Crypto.Crypto;
+  const threadManagement = yield* ThreadManagementService;
+
+  const update = Effect.fn("ThreadMetadataMcpService.update")(function* (
+    scope: McpInvocationScope,
+    input: ThreadMetadataMcpUpdateInput,
+  ) {
+    if (!scope.capabilities.has("orchestration")) {
+      return yield* failure(
+        "capability_denied",
+        "This MCP credential does not grant orchestration capabilities.",
+      );
+    }
+
+    const parent = yield* threadManagement
+      .getThreadProjection(scope.threadId)
+      .pipe(
+        Effect.mapError((error) =>
M
macroscopeappBotAug 30, 2026

🟡 Medium mcp/ThreadMetadataMcpService.ts:153

When the calling thread no longer exists, getThreadProjection(scope.threadId) fails with OrchestratorProjectionError, but this mapper always returns orchestration_error. The update tool therefore reports a server failure instead of thread_not_found, so MCP clients cannot handle stale calling threads; map the unavailable-thread projection error to thread_not_found.

✅ Resolved in a965fc4eda71a107953d2c01f013b2b6755b247d

J
Julius MarmingeAug 30, 2026

Fixed in a965fc4eda71a107953d2c01f013b2b6755b247d. The service now checks the typed ThreadManagement shell first: a null shell maps to thread_not_found, while a shell read failure or a subsequent Orchestrator projection/storage failure remains orchestration_error. Focused tests cover both the missing caller and an OrchestratorProjectionError so storage failures are not misclassified.

apps/server/src/mcp/ThreadMetadataMcpService.ts · Earlier revision cce20c4f · right 23–33

Original diff
@@ -0,0 +1,220 @@
+import {
+  CommandId,
+  OrchestratorMcpFailure,
+  type OrchestrationV2AppThread,
+  type OrchestrationV2Command,
+  type ThreadMetadataMcpAction,
+  type ThreadMetadataMcpUpdateInput,
+  type ThreadMetadataMcpUpdateResult,
+  ThreadId,
+} from "@t3tools/contracts";
+import * as Context from "effect/Context";
+import * as Crypto from "effect/Crypto";
+import * as DateTime from "effect/DateTime";
+import * as Effect from "effect/Effect";
+import * as Layer from "effect/Layer";
+
+import {
+  ThreadManagementError,
+  ThreadManagementService,
+} from "../orchestration-v2/ThreadManagementService.ts";
+import type { McpInvocationScope } from "./McpInvocationContext.ts";
+
+export interface ThreadMetadataMcpServiceShape {
+  readonly update: (
+    scope: McpInvocationScope,
+    input: ThreadMetadataMcpUpdateInput,
+  ) => Effect.Effect<ThreadMetadataMcpUpdateResult, OrchestratorMcpFailure>;
+}
+
+export class ThreadMetadataMcpService extends Context.Service<
+  ThreadMetadataMcpService,
+  ThreadMetadataMcpServiceShape
+>()("t3/mcp/ThreadMetadataMcpService") {}

apps/server/src/mcp/ThreadMetadataMcpService.ts · right 23–33

M
macroscopeappBotAug 29, 2026

New service modules should declare the interface inline in the Context.Service declaration rather than keeping a standalone FooServiceShape type; refer to the inferred interface as ThreadMetadataMcpService["Service"] if a name is needed. (ThreadMetadataMcpServiceShape has no other references.)

suggestion
1export class ThreadMetadataMcpService extends Context.Service<
2 ThreadMetadataMcpService,
3 {
4 readonly update: (
5 scope: McpInvocationScope,
6 input: ThreadMetadataMcpUpdateInput,
7 ) => Effect.Effect<ThreadMetadataMcpUpdateResult, OrchestratorMcpFailure>;
8 }
9>()("t3/mcp/ThreadMetadataMcpService") {}

Posted via Macroscope — Effect Service Conventions

J
Julius MarmingeAug 29, 2026

Fixed in 6c22941568. The service now declares its interface inline in Context.Service; the standalone, otherwise-unused shape type is removed.

apps/server/src/mcp/toolkits/orchestrator/tools.ts · Earlier revision cce20c4f · right 211–211

Original diff
@@ -190,6 +197,19 @@ export const ThreadReadTool = Tool.make("t3_thread_read", {
   .annotate(Tool.Destructive, false)
   .annotate(Tool.Idempotent, true);
 
+export const ThreadUpdateTool = Tool.make("t3_thread_update", {
+  description:
+    "Update metadata for a thread in the calling project. Omit threadId to update this thread. Use action='rename' with title, action='regenerate_title' with no extra field, action='link_pull_request' with pullRequest, or action='unlink_pull_request'. Workspace and branch changes are intentionally not supported. clientRequestId makes retries idempotent.",
+  parameters: ThreadMetadataMcpUpdateInput,
+  success: ThreadMetadataMcpUpdateResult,
+  failure: OrchestratorMcpFailure,
+  failureMode: "return",
+  dependencies: threadMetadataDependencies,
+})
+  .annotate(Tool.Title, "Update T3 thread metadata")
+  .annotate(Tool.Destructive, true)
+  .annotate(Tool.Idempotent, true);

apps/server/src/mcp/toolkits/orchestrator/tools.ts · right 211–211

M
macroscopeappBotAug 29, 2026

🟡 Medium orchestrator/tools.ts:211

ThreadUpdateTool is published as idempotent, but repeating action: "regenerate_title" without clientRequestId generates a new command each time and schedules another title-generation effect. Marking the tool idempotent is therefore incorrect; set Tool.Idempotent to false unless idempotency is enforced for requests without clientRequestId.

suggestion
1 .annotate(Tool.Idempotent, false);
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @apps/server/src/mcp/toolkits/orchestrator/tools.ts around line 211:
2
3`ThreadUpdateTool` is published as idempotent, but repeating `action: "regenerate_title"` without `clientRequestId` generates a new command each time and schedules another title-generation effect. Marking the tool idempotent is therefore incorrect; set `Tool.Idempotent` to `false` unless idempotency is enforced for requests without `clientRequestId`.
J
Julius MarmingeAug 29, 2026

Fixed in 6c22941568. The tool-level idempotency hint is now false because calls without clientRequestId create fresh commands; the description still documents clientRequestId-based retry idempotency, and the production MCP annotation assertion now verifies false.