fix(ui): collapse failed internal tool calls when the turn still replied (#89683) (#90122)

harjothkhara · claude · web-flow · commit 355c43fe0c9e · 2026-06-29T15:23:46.000+03:00
A non-zero internal tool exit (Codex marks any non-zero exit as failed,
e.g. a no-match shell search) was promoted to a primary red "Tool error"
banner in Control UI even when the turn produced a clean assistant reply.
Compute a per-tool-group turnSucceeded signal at the chat projection
boundary and de-promote such non-terminal failures to a collapsed tool
summary; detail stays available on expand. Genuinely terminal tool
failures still surface.

Co-authored-by: Claude Opus 4.8 &lt;noreply@anthropic.com&gt;
diff --git a/ui/src/ui/chat/build-chat-items.test.ts b/ui/src/ui/chat/build-chat-items.test.ts
@@ -1138,3 +1138,60 @@ function createAssistantCanvasBlock(params: { suffix: string }) {
     },
   };
 }
+
+describe("tool turn outcome annotation (#89683)", () => {
+  function failedTool(timestamp: number) {
+    return {
+      role: "toolResult",
+      toolName: "shell",
+      content: JSON.stringify({ status: "failed", exitCode: 1 }),
+      isError: true,
+      timestamp,
+    };
+  }
+  function userMsg(text: string, timestamp: number) {
+    return { role: "user", content: text, timestamp };
+  }
+  function assistantReply(text: string, timestamp: number) {
+    return { role: "assistant", content: [{ type: "text", text }], timestamp };
+  }
+  function toolGroups(messages: unknown[]): MessageGroup[] {
+    return messageGroups({ messages }).filter((group) => group.role === "tool");
+  }
+
+  it("marks a failed tool followed by an assistant reply as turnSucceeded", () => {
+    const tools = toolGroups([
+      userMsg("search foo", 1),
+      failedTool(2),
+      assistantReply("No matches found.", 3),
+    ]);
+    expect(tools).toHaveLength(1);
+    expect(tools[0].turnSucceeded).toBe(true);
+  });
+
+  it("leaves a terminal failed tool (no assistant reply) as not-succeeded", () => {
+    const tools = toolGroups([userMsg("search foo", 1), failedTool(2)]);
+    expect(tools).toHaveLength(1);
+    expect(tools[0].turnSucceeded).toBe(false);
+  });
+
+  it("does not count an assistant group without reply text as success", () => {
+    const tools = toolGroups([
+      userMsg("search foo", 1),
+      failedTool(2),
+      { role: "assistant", content: [], timestamp: 3 },
+    ]);
+    expect(tools[0].turnSucceeded).toBe(false);
+  });
+
+  it("scopes the outcome per turn at user boundaries", () => {
+    const tools = toolGroups([
+      userMsg("first", 1),
+      failedTool(2),
+      assistantReply("done", 3),
+      userMsg("second", 4),
+      failedTool(5),
+    ]);
+    expect(tools.map((group) => group.turnSucceeded)).toEqual([true, false]);
+  });
+});
diff --git a/ui/src/ui/chat/build-chat-items.ts b/ui/src/ui/chat/build-chat-items.ts
@@ -338,6 +338,41 @@ function isSameSourceRelayNativeDuplicate(previousMessage: unknown, nextMessage:
   );
 }
 
+function assistantGroupHasReplyText(group: MessageGroup): boolean {
+  // A real reply is assistant text; a tool-only assistant group does not count.
+  return group.messages.some(({ message }) => Boolean(extractTextCached(message)?.trim()));
+}
+
+// Stamp each tool group with whether its turn ended in a successful assistant
+// reply. Codex marks any non-zero exec exit as failed, so a benign internal tool
+// failure (e.g. a no-match search) must not render as a primary error banner
+// once a clean reply exists. Backward pass: a user group ends the turn
+// downstream; an assistant reply marks success for earlier tool groups in the
+// same turn. turnSucceeded stays undefined for terminal or in-progress failures,
+// preserving the existing error banner.
+function annotateToolTurnOutcome(
+  items: Array<ChatItem | MessageGroup>,
+): Array<ChatItem | MessageGroup> {
+  let sawAssistantReply = false;
+  for (let i = items.length - 1; i >= 0; i -= 1) {
+    const item = items[i];
+    if (item.kind !== "group") {
+      continue;
+    }
+    const role = item.role.toLowerCase();
+    if (role === "user") {
+      sawAssistantReply = false;
+    } else if (role === "assistant") {
+      if (assistantGroupHasReplyText(item)) {
+        sawAssistantReply = true;
+      }
+    } else if (role === "tool") {
+      item.turnSucceeded = sawAssistantReply;
+    }
+  }
+  return items;
+}
+
 function collapseDuplicateDisplaySignature(message: unknown): string | null {
   if (isPendingSendMessage(message)) {
     return null;
@@ -817,7 +852,9 @@ export function buildChatItems(props: BuildChatItemsProps): Array<ChatItem | Mes
     }
   }
 
-  return groupMessages(collapseSequentialDuplicateMessages(sortChatItemsByVisibleTime(items)));
+  return annotateToolTurnOutcome(
+    groupMessages(collapseSequentialDuplicateMessages(sortChatItemsByVisibleTime(items))),
+  );
 }
 
 function messageKey(message: unknown, index: number): string {
diff --git a/ui/src/ui/chat/grouped-render.test.ts b/ui/src/ui/chat/grouped-render.test.ts
@@ -1375,6 +1375,98 @@ describe("grouped chat rendering", () => {
     ).toEqual({ status: "error" });
   });
 
+  describe("non-terminal internal tool failure de-promotion (#89683)", () => {
+    function failedToolMessage(callId = "call-failed") {
+      return {
+        id: `tool-${callId}`,
+        role: "toolResult",
+        toolCallId: callId,
+        toolName: "shell",
+        content: JSON.stringify({ status: "failed", exitCode: 1, stdout: "" }, null, 2),
+        isError: true,
+        timestamp: Date.now(),
+      };
+    }
+
+    it("renders a failed tool collapsed (no error banner) when the turn succeeded", () => {
+      const container = document.createElement("div");
+      const group: MessageGroup = {
+        ...createMessageGroup(failedToolMessage(), "tool"),
+        turnSucceeded: true,
+      };
+      renderMessageGroups(container, [group], { isToolMessageExpanded: () => false });
+
+      const summary = expectElement(container, ".chat-tool-msg-summary", HTMLButtonElement);
+      expect(summary.classList.contains("chat-tool-msg-summary--error")).toBe(false);
+      expect(summary.querySelector(".chat-tool-msg-summary__label")?.textContent).not.toBe(
+        "Tool error",
+      );
+      expect(summary.querySelector(".chat-tool-msg-summary__error-badge")).toBeNull();
+    });
+
+    it("still surfaces a failed tool as an error when the turn did not produce a reply", () => {
+      const container = document.createElement("div");
+      // turnSucceeded undefined = terminal/in-progress failure; behavior unchanged.
+      const group = createMessageGroup(failedToolMessage(), "tool");
+      renderMessageGroups(container, [group], { isToolMessageExpanded: () => false });
+
+      const summary = expectElement(container, ".chat-tool-msg-summary", HTMLButtonElement);
+      expect(summary.classList.contains("chat-tool-msg-summary--error")).toBe(true);
+      expect(summary.querySelector(".chat-tool-msg-summary__label")?.textContent).toBe(
+        "Tool error",
+      );
+      expect(summary.querySelector(".chat-tool-msg-summary__error-badge")).not.toBeNull();
+    });
+
+    it("keeps the failed tool detail on expand even when de-promoted", () => {
+      const container = document.createElement("div");
+      const group: MessageGroup = {
+        ...createMessageGroup(failedToolMessage(), "tool"),
+        turnSucceeded: true,
+      };
+      renderMessageGroups(container, [group], { isToolMessageExpanded: () => true });
+      // Info is not deleted: the expanded body still shows the failed tool output.
+      const body = container.querySelector(".chat-tool-msg-body");
+      expect(body).not.toBeNull();
+      expect(body?.textContent).toContain("failed");
+    });
+
+    it("de-promotes a multi-tool activity group when the turn succeeded", () => {
+      const container = document.createElement("div");
+      const group: MessageGroup = {
+        kind: "group",
+        key: "tool:multi",
+        role: "tool",
+        messages: [
+          { key: "t1", message: failedToolMessage("call-1") },
+          { key: "t2", message: failedToolMessage("call-2") },
+        ],
+        timestamp: Date.now(),
+        isStreaming: false,
+        turnSucceeded: true,
+      };
+      renderMessageGroups(container, [group], { isToolMessageExpanded: () => false });
+
+      const summary = expectElement(container, ".chat-activity-group__summary", HTMLButtonElement);
+      expect(summary.classList.contains("chat-activity-group__summary--error")).toBe(false);
+      expect(summary.querySelector(".chat-activity-group__badge")).toBeNull();
+      expect(summary.getAttribute("aria-expanded")).toBe("false");
+    });
+
+    it("keeps failed tools hidden when showToolCalls is false regardless of outcome", () => {
+      const container = document.createElement("div");
+      const group: MessageGroup = {
+        ...createMessageGroup(failedToolMessage(), "tool"),
+        turnSucceeded: true,
+      };
+      renderMessageGroups(container, [group], {
+        showToolCalls: false,
+        isToolMessageExpanded: () => false,
+      });
+      expect(container.querySelector(".chat-tool-msg-summary")).toBeNull();
+    });
+  });
+
   it("collapses an inline tool call while keeping matching tool output visible", () => {
     const container = document.createElement("div");
     const groups = [
diff --git a/ui/src/ui/chat/grouped-render.ts b/ui/src/ui/chat/grouped-render.ts
@@ -451,6 +451,7 @@ function buildGroupedMessageRenderOptions(
     duplicateCount: item.duplicateCount ?? 1,
     showReasoning: opts.showReasoning,
     showToolCalls: opts.showToolCalls ?? true,
+    turnSucceeded: group.turnSucceeded,
     autoExpandToolCalls: opts.autoExpandToolCalls ?? false,
     isToolMessageExpanded: opts.isToolMessageExpanded,
     onToggleToolMessageExpanded: opts.onToggleToolMessageExpanded,
@@ -520,7 +521,9 @@ export function renderMessageGroup(group: MessageGroup, opts: RenderMessageGroup
         : toolLabels.length <= 3
           ? toolLabels.join(", ")
           : `${toolLabels.slice(0, 2).join(", ")} +${toolLabels.length - 2} more`;
-    const hasError = cards.some(isToolCardError);
+    // Non-terminal internal tool failures (turn produced a clean reply) stay
+    // collapsed and unstyled; detail remains available on expand. #89683
+    const hasError = cards.some(isToolCardError) && group.turnSucceeded !== true;
     const activityDisclosureId = `activity:${group.key}`;
     const activityExpanded = opts.isToolMessageExpanded?.(activityDisclosureId) ?? hasError;
 
@@ -1618,6 +1621,9 @@ function renderGroupedMessage(
     duplicateCount?: number;
     showReasoning: boolean;
     showToolCalls?: boolean;
+    // True when the tool's turn still produced a clean assistant reply: a failed
+    // internal tool then renders collapsed, not as a primary error banner. #89683
+    turnSucceeded?: boolean;
     autoExpandToolCalls?: boolean;
     isToolMessageExpanded?: (messageId: string) => boolean | undefined;
     onToggleToolMessageExpanded?: (messageId: string, expanded?: boolean) => void;
@@ -1730,7 +1736,7 @@ function renderGroupedMessage(
   const toolMessageExpanded = opts.isToolMessageExpanded?.(toolMessageDisclosureId) ?? false;
   const toolNames = [...new Set(toolCards.map((c) => c.name))];
   const singleToolCard = toolCards.length === 1 ? toolCards[0] : null;
-  const toolMessageHasError = toolCards.some(isToolCardError);
+  const toolMessageHasError = toolCards.some(isToolCardError) && opts.turnSucceeded !== true;
   const singleToolDisplay = singleToolCard
     ? resolveToolDisplay({
         name: singleToolCard.name,
diff --git a/ui/src/ui/types/chat-types.ts b/ui/src/ui/types/chat-types.ts
@@ -25,6 +25,11 @@ export type MessageGroup = {
   messages: Array<{ message: unknown; key: string; duplicateCount?: number }>;
   timestamp: number;
   isStreaming: boolean;
+  // Tool groups only: true when the turn still produced a successful assistant
+  // reply, so a failed internal tool (Codex marks any non-zero exit as failed)
+  // renders collapsed instead of as a primary red error banner. Undefined for
+  // non-tool groups and for terminal/in-progress tool failures.
+  turnSucceeded?: boolean;
 };
 
 /** Content item types in a normalized message */
