From 14fa5176199451ebc69844956b06a941395c714d Mon Sep 17 00:00:00 2001 From: Samartha Bapat Date: Sun, 23 Aug 2026 15:37:42 -0500 Subject: [PATCH 1/2] fix(server): classify successful MCP results correctly --- .../src/provider/Layers/CodexAdapter.test.ts | 15 ++++++- .../src/provider/Layers/CodexAdapter.ts | 39 +++++++++++++++---- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/apps/server/src/provider/Layers/CodexAdapter.test.ts b/apps/server/src/provider/Layers/CodexAdapter.test.ts index 26fb1b166f61..ffd262e80c20 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.test.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.test.ts @@ -741,6 +741,16 @@ lifecycleLayer("CodexAdapterLive lifecycle", (it) => { error: { message: "Build failed" }, status: "failed", }, + { + type: "mcpToolCall", + id: "successful-mcp-with-failed-status", + server: "t3-code", + tool: "inspectTaskChanges", + arguments: {}, + error: null, + result: { content: [{ type: "text", text: "diff details" }] }, + status: "failed", + }, { type: "fileChange", id: "declined-change", @@ -774,7 +784,10 @@ lifecycleLayer("CodexAdapterLive lifecycle", (it) => { if (firstEvent._tag !== "Some" || firstEvent.value.type !== "item.completed") { return; } - NodeAssert.equal(firstEvent.value.payload.status, item.status); + NodeAssert.equal( + firstEvent.value.payload.status, + item.id === "successful-mcp-with-failed-status" ? "completed" : item.status, + ); } }), ); diff --git a/apps/server/src/provider/Layers/CodexAdapter.ts b/apps/server/src/provider/Layers/CodexAdapter.ts index bc48f94b3866..26fb20009049 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.ts @@ -294,6 +294,36 @@ function itemDetail(itemType: CanonicalItemType, item: CodexLifecycleItem): stri return undefined; } +function hasSuccessfulToolResult(item: CodexLifecycleItem): boolean { + if (item.type !== "mcpToolCall") { + return false; + } + return item.result != null && item.error == null; +} + +function itemLifecycleStatus( + item: CodexLifecycleItem, + lifecycle: "item.started" | "item.updated" | "item.completed", +): "inProgress" | "failed" | "declined" | "completed" | undefined { + if (lifecycle === "item.started") { + return "inProgress"; + } + + if (lifecycle !== "item.completed") { + return undefined; + } + + if ( + "status" in item && + (item.status === "failed" || item.status === "declined") && + !hasSuccessfulToolResult(item) + ) { + return item.status; + } + + return "completed"; +} + function toRequestTypeFromMethod(method: string): CanonicalRequestType { switch (method) { case "item/commandExecution/requestApproval": @@ -476,14 +506,7 @@ function mapItemLifecycle( } const detail = itemDetail(itemType, item); - const status = - lifecycle === "item.started" - ? "inProgress" - : lifecycle === "item.completed" - ? "status" in item && (item.status === "failed" || item.status === "declined") - ? item.status - : "completed" - : undefined; + const status = itemLifecycleStatus(item, lifecycle); return { ...runtimeEventBase(event, canonicalThreadId), From da849275c53f4531f63a7d80e598aa5168d3941f Mon Sep 17 00:00:00 2001 From: Samartha Bapat Date: Sun, 23 Aug 2026 15:47:03 -0500 Subject: [PATCH 2/2] fix(server): preserve successful MCP activity status --- .../ActivityPayloadProjection.test.ts | 19 +++++++++++++++++++ .../ActivityPayloadProjection.ts | 9 ++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/apps/server/src/orchestration/ActivityPayloadProjection.test.ts b/apps/server/src/orchestration/ActivityPayloadProjection.test.ts index 2cdfef19fd18..b2f16e362fc8 100644 --- a/apps/server/src/orchestration/ActivityPayloadProjection.test.ts +++ b/apps/server/src/orchestration/ActivityPayloadProjection.test.ts @@ -168,6 +168,25 @@ describe("projectActivityPayload", () => { expect(JSON.stringify(projected.payload).length).toBeLessThan(500); }); + it("does not restore a failed status for a successful Codex MCP result", () => { + const projected = projectActivityPayload( + activity({ + itemType: "mcp_tool_call", + status: "completed", + data: { + item: { + type: "mcpToolCall", + status: "failed", + result: { content: [{ type: "text", text: "diff details" }] }, + error: null, + }, + }, + }), + ); + + expect((projected.payload as Record).status).toBe("completed"); + }); + it("slims Claude-shaped mcp_tool_call data (toolName/input/result block)", () => { const projected = projectActivityPayload( activity({ diff --git a/apps/server/src/orchestration/ActivityPayloadProjection.ts b/apps/server/src/orchestration/ActivityPayloadProjection.ts index 32f249c251d5..cd4cb82d3db0 100644 --- a/apps/server/src/orchestration/ActivityPayloadProjection.ts +++ b/apps/server/src/orchestration/ActivityPayloadProjection.ts @@ -329,6 +329,11 @@ function projectAcpContent(value: unknown): Record | undefined return summary ? { content: summary } : undefined; } +function hasSuccessfulMcpResult(data: Record): boolean { + const item = asRecord(data.item); + return item?.type === "mcpToolCall" && item.result != null && item.error == null; +} + /** * Removes activity payload fields that no current client reads while retaining * the full payload in persistence and the event store. @@ -344,7 +349,9 @@ export function projectActivityPayload( const itemStatus = asRecord(data.item)?.status; const projectedPayload = - payload.status === "completed" && (itemStatus === "failed" || itemStatus === "declined") + payload.status === "completed" && + (itemStatus === "failed" || itemStatus === "declined") && + !hasSuccessfulMcpResult(data) ? { ...payload, status: itemStatus } : payload;