Skip to content

Agent chat: a failed tool call says what happened in a word and why in one line - #24339

Merged
teamleaderleo merged 9 commits into
feat-cmux-nextfrom
tp/prettier-errors
Oct 23, 2026
Merged

teamleaderleo merged 9 commits into
feat-cmux-nextfrom
tp/prettier-errors

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Leo-ask: ask-427cc7d8f5

Leo: "we need to have prettier errors"

This is the first part of that ask: failed tool calls. Turn-level and connection errors come in a follow-up that builds on #24312's turn error card.

What changes

A failed tool call used to show its label, then a dim "failed" (or a red "Exit 1" on a command row). Why it failed was only in the raw result, and you had to open the call to see it: Claude Code's Exit code 1 line over a traceback, a hook's PreToolUse:Bash hook error: Blocked: …, a long "The user doesn't want to proceed with this tool use…" paragraph, a <tool_use_error> tag, Codex's {"status":"declined"}, or an MCP server's JSON error.

Now a failed call has two parts:

  • The row says what happened in one word, in the theme's error tone: blocked (a hook refused it), denied (you declined it), timed out, Exit N, or failed. This applies to lone calls, command rows and MCP rows.

  • One quiet line under the row says why, in the tool's own words without the envelope:

    • a hook's reason, after the hook's name in mono;
    • a shell's last line that names an error (the traceback's TypeError: …, error: could not compile …);
    • an MCP server's error.message (code) or its isError text;
    • the text inside <tool_use_error>;
    • what you said when you declined, if you said anything.

    The line is cut to one line. The opened call still has the raw result, and Find in Chat still counts the result rather than this line.

  • Claude Code's Exit code N line is said once. It becomes the row's Exit N and the Shell block's exit line, so the output in the opened block starts at the real output.

  • The error tone stays on the one word. There is no red block, and the cause uses the transcript's dim text.

toolFailure.ts reads only failed calls, and only the first and last 4,000 characters of a long result. Nothing changes for calls that succeeded.

Versions

There are three looks as the tool-error-look experiment, side by side in the gallery entry agent-pane.tool-errors → Compare arms:

  • Quiet line (line, shipped): the cause on a dim line under the row.
  • On the row (inline): no new line; the cause follows the call's words and is cut with them.
  • Compact card (card): a small glass card with a thin error-tone rail, up to three lines, and Copy error.

I picked Quiet line. It keeps a run of calls scannable, it reads like Claude Code's own ⎿ result line, and it adds no surface. The card is heavier than a single failed call warrants in a list of calls. The inline look loses the cause on narrow panes, where the label already fills the row. To dogfood another arm, set localStorage["cmux.experiments"] = {"tool-error-look":"card"}.

Gallery variants: claude-code (hook refusal, Exit 1 with a traceback, a declined edit, a tool_use_error, a timeout), codex (a declined command, a build that exited 101), mcp (an MCP JSON error, an isError result, a 404 fetch), and exit-opened.

Gallery: agent-pane.tool-errors

Before / after

The gallery entry agent-pane.tool-errors uses synthetic fixtures only, rendered by the gallery matrix runner (Chromium, 460 px). Before is base feat-cmux-next rows; after is this PR's shipped line look.

Before After (line, shipped)
Claude Code: hook block, declined push, Exit 1 traceback, tool_use_error, timeout before after
Codex: declined command, cargo exit 101 before after
MCP: JSON error, isError text, 404 fetch before after
Light theme before light after light

The three versions side by side (dark):

line (shipped) inline card
line inline card
line inline card

Relation to sibling PRs

Tests

  • New test/tool-failure.test.tsx covers:
    • classification of each harness shape;
    • the cause line and the exit-code line;
    • long results (head and tail windows, whole-line tail, over-long last line);
    • non-Latin text, fences and ANSI;
    • injected context, including inside nested MCP JSON;
    • denials quoted by a command that ran (still read as its exit);
    • hook commands in brackets;
    • CR-redrawn progress lines;
    • background calls;
    • the three arms' DOM.
  • New test/tool-failure-find.test.tsx renders lone and grouped rows in JSDOM and checks that Find in Chat counts exactly what is painted outside data-find-skip. It covers an Exit code 1 result, a failed call that exited 0, and a completed call with a nonzero exit. A mutation check showed it fails without the fix.
  • On huge-blue at the head: 39 of 39 pass in the tool-failure files. The related transcript, tool, MCP, find and gallery tests show 375 pass and 3 fail, and all 3 are base reds on feat-cmux-next (see below).
  • vp check (format, lint, tsc) is clean. check-type-scale is ok.

Base reds (not this PR)

  • gallery coverage (2 tests) fails on base feat-cmux-next: chatFolders.ts, ChatUnavailableState and DropOverlay have no entry. None are files in this PR.
  • tooltip-rules.test.ts fails on base: 10 violations, all in files this PR does not touch.

Review

Three Codex xhigh rounds and three Claude rounds. Every finding is fixed with a test, and the receipts are in the comments.

Changelog

  • Changed: A failed tool call in the agent chat says what happened (blocked, denied, timed out, Exit N) and why in one line.

Size

About 1,070 changed lines: about 650 of code and 420 of tests, plus 396 generated catalog lines: three strings × 21 locales, with non-English entries marked needs_review. The tests are most of the overage past the 800-line guide. Splitting the classifier from its rows would leave either half without a visible change, so this is kept as one PR.

No capture: webview-only. The gallery shots above are the visual.

Agent session: cosmic-gray

🤖 Generated with Claude Code

teamleaderleo and others added 2 commits October 22, 2026 01:34
…n one line

Leo: "we need to have prettier errors". A failed call's row now says blocked, denied,
timed out, Exit N or failed in the error tone, and one quiet line under it says why in
the tool's own words (a hook's reason, a shell's error line, an MCP server's message)
without the JSON or tag envelope. Claude Code's "Exit code N" line becomes the row's
Exit N and the Shell block's exit line, so it is said once. Three looks are the
tool-error-look experiment (quiet line, on the row, compact card); the quiet line ships.

Leo-ask: ask-427cc7d8f5

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a16030-bf28-7072-b1f6-b83bbd0beacc&host=cosmic-gray
@teamleaderleo teamleaderleo added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 22, 2026
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Owned by cosmic-gray/transcript-parity on cosmic-gray · session

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 86a6ab7: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr24339 -f ref=tp/prettier-errors -f platform=macos -f backend_mode=local

Unclear: web / react-apps-check failed: no known signature; failed step: Check webview test shards. This PR merged before its CI finished: fix forward on main.

CI failed on 86a6ab7bad (run 39897564708 attempt 1): 1 code, 1 unknown.

Job Verdict Why
web / react-apps-check (2) code a web test failed
web / react-apps-check unknown no known signature; failed step: Check webview test shards

Failures:

  • seen on other PRs incomingMessage.test.ts a message addressed sender first is that agent's when it names this chat or a listed sender: failed (job)
  • seen on other PRs accent-glass-invariants.test.ts no orange or system accent and no opaque glass on 1818 gallery stages: failed (job)
  • seen on other PRs agent-pane-adopted-tool-calls.test.ts the adopted turn's Worked for opens to its commands and edited files: failed (job)
  • seen on other PRs composer-edges.test.ts composer card: appearance.borders = none draws no edge: failed (job)
  • seen on other PRs composer-edges.test.ts composer card: one hairline edge by default: failed (job)
  • seen on other PRs composer-foot-pill-inset.test.ts the row's padding is counted from the inset, so only the pill grows, not the text's place: failed (job)
  • seen on other PRs composer-foot-pill-inset.test.ts the context ring keeps its place past the model's text: failed (job)
  • seen on other PRs composer-location-strip.test.ts the chat composer: no hairline across the seam under the strip: failed (job)
Matched log lines
web / react-apps-check (2): (fail) a message addressed sender first is that agent's when it names this chat or a listed sender [0.43ms]

Not re-run automatically: web / react-apps-check (2), web / react-apps-check are not machine failures.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

red: cmux-next checks (god files, concurrency, crash safety, l10n); NEW: red check: Checks job structure (+1 more) is not red on the base c3bdbb8; do not merge until green; see lane-rules
red: web / Web status; base red: the same failure on the base c3bdbb8; does not block
red: web / react-apps-check; base red: the same failure on the base c3bdbb8; does not block
red: web / react-apps-check (2); NEW: Expected length: 1 Received length: 3 at <anonymous> (/home/runner/_work/cmux/cmux/webviews/test/composer-edges.test.ts:137:18) (+4 more) is not red on the base c3bdbb8; do not merge until green; see lane-rules

Tour of 86a6ab7bad (run): no media. The tour did not run: only webview code changed (15 path(s)), which the gallery advisory run checks in Linux Chromium on the pull request (its artifact holds any captures); add the media-tour-mac label, then re-add media-tour, to tour it on a Mac.

Gallery before/after for 86a6ab7: the capture produced no images (run).

@blacksmith-sh

blacksmith-sh Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Found 41 test failures on Blacksmith runners:

Failures

Test View Logs
---/--- View Logs
---/--- View Logs
---/--- View Logs
---/--- View Logs
---/--- View Logs
---/--- View Logs
---/--- View Logs
---/--- View Logs
/tmp/tmpc4d9yqeb/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmpwe0jswgn/summary-fail.json (live):/summary-fail.json (live): View Logs

...and 31 more test failures. View all on Blacksmith: run 39897564646, run 39897564708, run 39897564683

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Bead cx-rs285 created for this PR by the cmux coordinator.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6.1-sol xhigh): BLOCK at 4e7ac1b

  1. P2 — Ordinary command output becomes a permission denial. toolFailure.ts:129: Exit code 1\nFAIL permissions.test.ts\nExpected: "user declined"\nAssertionError: expected 403, received 500 returns kind: "denied" with no cause. The command ran and failed; nobody declined it. The unanchored substring check overrides the established exit code and discards the diagnostic. Fix: recognize denial only from a known refusal envelope or structured status; keep ordinary executed-command output on the exit/error path. Add a regression containing quoted denial words in failed test output.

  2. P2 — The cause exposes context that the existing output renderer removes. toolFailure.ts:141: a failed fetch with <system-reminder>ambient UI state</system-reminder>\nError: server unavailable displays the internal context as its cause. Opening the call correctly shows only Error: server unavailable, through cleanText. An MCP content[].text containing the same payload has the same defect. This also makes the cause contradict the searchable output. Fix: reuse the non-shell output's context cleaning and JSON unwrapping before selecting the cause; cover failed variants of the existing MCP/context fixtures.

  3. P2 — Non-Latin causes disappear. toolFailure.ts:61, also line 69: JavaScript \w excludes Chinese, Japanese, Cyrillic, etc. {"error":"访问被拒绝"} produces no cause, and PreToolUse:Bash hook error: Blocked: 请先运行测试 loses the hook's reason. Fix: use a Unicode letter/number predicate, such as /[\p{L}\p{N}]/u, in both line filters, and test these messages.

  4. P2 — Find's index and rendered matches diverge after exit-code inference/stripping. CommandRow.tsx:91, also lines 109–110: for command false, raw output Exit code 1\nbuild failed, and no explicit exit code, Find indexes one occurrence of 1. The opened row now exposes two searchable occurrences: the new Exit 1 badge and ShellBlock's Exit code 1 footer. With status: "failed", exitCode: 0, and output Exit code 0\nresult transport failed, Find indexes Exit code 0 but renders no match at all. chatFind.ts still indexes the raw output. Fix: derive the search index from the same transformed output and exit footer that render, and mark synthetic status badges data-find-skip. Cover both cases with count/highlight assertions.

  5. P3, non-blocking — Fenced results retain the exit line and gain a duplicate footer. toolFailure.ts:117: parsing unwraps fences and ANSI, but output removal matches the untouched raw string. For ```\nExit code 1\nbuild failed\n```, the inferred code is 1 while failure.output stays undefined, so ShellBlock receives the original fenced header plus a new exit footer. Fix: consistently unwrap the full output before removing the recognized header, preserving the full body rather than the clipped classification sample.

Verification: inspected the supplied diff and surrounding pinned source, including existing output/context/Find tests. Focused parser and React static-render probes reproduced the examples above, with host integrations stubbed; these ran on cmux16 before the huge-blue-only instruction arrived. No further reproductions ran after that instruction. The repository test suite and browser/layout tests were not run. The PR snapshot matched the requested head.

- A denial is read only from a result with no exit code, anchored at its start;
  a timeout only from the first or last line; errno codes only in capitals.
- The cause skips host-injected context (cleanText), unwraps fences and ANSI
  before the exit line, reads small JSON whole, and keeps non-Latin words.
- Find counts a shell's output as drawn (shellOutput); the red word and the
  Shell block's exit line are data-find-skip.
- A single-call group and a folded run say the failure; the card uses CopyButton.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a16030-bf28-7072-b1f6-b83bbd0beacc&host=cosmic-gray
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (claude subagent, correctness first): SHIP at 4e7ac1b

This is an earlier verdict at the first head, posted for the record. All its findings are fixed in 5351190, and that head gets a fresh review.

  1. Medium: the unanchored DENIED pattern read ordinary failed output as "denied". Fixed: it is now anchored and read only when there is no exit code.
  2. Medium: TIMED_OUT with /m matched any line. Fixed: it now checks only the first and last line.
  3. Low/medium: ^E[A-Z]{3,} under /i matched words like "Expected" and "Elapsed". Fixed: errno matching is now case-sensitive.
  4. Low: a call with status failed and exit 0 blamed its last plain line. Fixed: such a call gets a cause only from a line that names an error.
  5. Low: JSON was parsed after clipping. Fixed: small JSON is parsed whole.
  6. Low: a fenced result kept its exit line. Fixed: the fence is unwrapped before the exit line is stripped.
  7. Low: the Find index and the render disagreed. Fixed: shellOutput, plus data-find-skip on the word and on the Shell block's exit line.
  8. Low: every row read the experiment arm. Fixed: rows return early when nothing failed.
  9. Low: the card's Copy button stayed on "Copied". Fixed: the card uses CopyButton.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6.1-sol xhigh): BLOCK at 5351190

The five original examples are addressed, including the Unicode and fenced-output fixes. Three correctness gaps remain in the denial, context-cleaning and Find fixes:

  1. P2 — Structured command output can still override a known exit with “denied” or “blocked”. toolFailure.ts:138: a failed shell call with exitCode: 22 and output {"status":"declined","error":"payment declined"}—for example, an HTTP error body from curl --fail-with-body—still returns kind: "denied" and drops the cause. The new exitCode === undefined guard applies only to the textual DENIED branch. The HOOK branch likewise labels executed commands “blocked” when they print a hook-error fixture at the start of their output. Fix: apply the execution/exit-code guard to structured refusal and hook-envelope detection too; add ordinary-output cases alongside genuine harness refusals.

  2. P2 — Nested MCP JSON still exposes internal context. toolFailure.ts:157, also lines 94–95: consider an MCP result whose text block contains JSON.stringify({error: "<system-reminder>ambient UI state</system-reminder>\nError: server unavailable"}). jsonCause returns that JSON string verbatim. Calling cleanText on the surrounding JSON cannot remove the context inside its error field, so the new cause still includes ambient UI state. The opened response's prettyOutput recursively parses JSON strings and removes it. This nested response shape is already exercised by the existing MCP display tests. Fix: share bounded recursive JSON unwrapping/context cleaning with the MCP display path before extracting the cause; extend the new failed-context test with this nested shape.

  3. P2 — Find normalization still differs from the lone-call renderer for completed calls with nonzero exits. chatFind.ts:36, paired with ToolRow.tsx:86: for a lone execute call with command false, status: "completed", exitCode: 1, and output Exit code 1\nbuild failed, shellOutput uses isFailed and removes the header from Find's index. ToolRow tests only status === "failed", leaves failure undefined, and draws the original header. Find consequently reports no Exit code 1 match although that text remains in the output. Fix: use the same isFailed predicate and output transformation in ToolRow and the index; cover this status/exit-code combination in both lone and grouped Find tests.

Verification: source inspection of the pinned head, fix.diff, pr2.diff, and surrounding tests. No reproductions, Bun commands or tests ran on cmux16 during this re-review. The available workspace reach helper refused the huge-blue route with “this host (cmux16) is not a registry origin”; no installed fleet route was present, so remote tests were not run. Findings above are source-traced, not test-run claims.

teamleaderleo and others added 3 commits October 22, 2026 02:29
- claude-agent-acp's "User refused permission" reads as denied.
- A lone call is failed by its status or a nonzero exit, as a command row reads it,
  so its Find index and its drawing agree.
- Copy error copies the result as drawn (no colour codes, fence or injected context).
- A long cause never splits an emoji; a zero-count summary is never the cause.
- The gallery stage is tall enough to show every call.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a16030-bf28-7072-b1f6-b83bbd0beacc&host=cosmic-gray
…; nested MCP JSON errors lose injected context

Codex re-review of #24339: a structured refusal or a hook's block is read only from
a result with no exit code, and an MCP text block that is itself a JSON error is
unwrapped (bounded) and cleaned before it becomes the cause. Find tests cover a
completed call with a nonzero exit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a16030-bf28-7072-b1f6-b83bbd0beacc&host=cosmic-gray
From the third review at ca49990 (all low):
- a hook's bracketed command stays out of its reason;
- Copy error keeps Claude Code's "Exit code N" line;
- a call with its own exit code keeps an output line that names another;
- lone carriage returns stay, as in a call that worked;
- the tail window starts on a whole line;
- context before an MCP text block's JSON is cleaned before parsing;
- a failed background call shows no cause line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a16030-bf28-7072-b1f6-b83bbd0beacc&host=cosmic-gray
teamleaderleo added a commit that referenced this pull request Oct 22, 2026
teamleaderleo added a commit that referenced this pull request Oct 22, 2026
teamleaderleo added a commit that referenced this pull request Oct 22, 2026
From the Claude re-review at 99b4875 (both low):
- a tail slice with no newline is kept, so an over-long last line is still read;
- a line a carriage return redrew reads as its last drawn part.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a16030-bf28-7072-b1f6-b83bbd0beacc&host=cosmic-gray
teamleaderleo added a commit that referenced this pull request Oct 22, 2026
teamleaderleo added a commit that referenced this pull request Oct 22, 2026
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (claude opus-5.5 subagent, correctness first): SHIP at e21c63a

This is an independent read-only review of the whole PR, done in three rounds (at ca49990, 99b4875 and e21c63a). Every finding from the earlier rounds is fixed with a test:

  • a hook's bracketed command;
  • Copy error keeping the Exit line;
  • an Exit line that does not match the call's own code;
  • lone CRs;
  • a whole-line tail window;
  • context before nested MCP JSON;
  • a failed background call;
  • an over-long last line;
  • CR-redrawn lines.

Also checked: React hook order, rows that did not fail (unchanged, and no experiment read), regex cost (anchored or bounded to the 4K+4K window, JSON at most 64K), and ToolRow's switch to isFailed, which matches CommandRow and ToolGroupRow and is not a regression.

One nit, not blocking: a tail slice with no newline can start with a lone low surrogate, which would show as one replacement character in an extreme case.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6.1-sol xhigh): BLOCK at e21c63a

  • P2 — webviews/src/agent-session/acpmux/conversation/ToolRow.tsx:93: failed now includes any nonzero exitCode, but background is defined as !failed && .... A lone background shell with background.state: "failed" and a nonzero exit therefore renders as a foreground failure: it loses BackgroundMark and ToolFailureCause is shown at line 156. CommandRow explicitly suppresses the cause for background calls, so this path is inconsistent and regresses the background-state presentation.

  • P2 — webviews/src/agent-session/acpmux/conversation/toolFailure.ts:210: shellOutput returns raw tool.output whenever no matching Claude Exit code N header was removed. ShellBlock always runs that output through useOutput, which strips ANSI, while chatFind.ts indexes this raw fallback. ANSI bytes can create invisible matches (or split a visible query), so Find counts/highlights disagree with the rendered shell output for Codex and other colored shell failures (and for successful shells as well).

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Orphaned: no activity for 12 h, and no lane holds a lease on it. The orphan sweep could not merge feat-cmux-next in: conflicts in webviews/src/agent-session/acpmux/conversation/McpCallRows.tsx, webviews/src/agent-session/acpmux/conversation/ToolRow.tsx, webviews/src/agent-session/acpmux/conversation/ToolRun.tsx, webviews/src/experiments/registry.ts. Listed for a lane to resolve.

…view)

- Merge base: base already counts a folded run's failures, so ToolRun is base's.
- ToolRow: a shell that went on in the background fails only by its status, so it keeps its
  background mark rather than reading its exit code as a failure (as CommandRow does).
- shellOutput: Find counts a shell's output without colour codes, as ShellBlock draws it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a16030-bf28-7072-b1f6-b83bbd0beacc&host=cosmic-gray
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (claude opus-5.5 subagent, correctness first): SHIP at a6a1d0e

This is a read-only review of the base update and the two Codex P2 fixes. The update changes none of the PR's own code:

  • CommandRow picks up base's shortCommand(..., ROW_WIDTH);
  • ToolRun now equals base, which already counts a run's failures with isFailed;
  • registry.ts keeps both sides.

Both fixes are correct:

  1. ToolRow.tsx:89: a lone background shell that ended nonzero keeps its BackgroundMark, with no red word and no cause line. A failed status still reads as failed, and a test covers this.
  2. toolFailure.ts:212: shellOutput strips ANSI as ShellBlock's useOutput does. A JSDOM find test covers this for both lone and grouped rows.

One nit, not blocking: for a background Codex shell whose output starts with exactly "Exit code N" matching its own exit code, Find would skip that line while the row draws it.

- Merge base (#24305's running label on a command row; ours already says Exit N).
- rowFailed: ToolRow, CommandRow and Find's shellOutput share one rule, so a shell that ended
  in the background is drawn and counted alike (Claude review nit at a6a1d0e).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a16030-bf28-7072-b1f6-b83bbd0beacc&host=cosmic-gray
@cursor

cursor Bot commented Oct 23, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (claude opus-5.5 subagent, correctness first): SHIP at 86a6ab7

This is a read-only review of the base update (c3bdbb8) and the shared rowFailed rule.

Two consistency nits, not blocking:

  1. ToolGroupRow and ToolRun (base's code) still count with isFailed, so a group holding a background shell that completed nonzero says "1 failed" while the row inside shows only its mark.
  2. A background shell whose status is "failed" draws its mark on a command row, but the red word and cause on a lone row.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (fable-5.1 xhigh, correctness first): SHIP at 86a6ab7

Read-only review of the whole files at the head (ToolRow, CommandRow, ToolGroupRow, ToolRun, McpCallRows, ShellBlock, BackgroundMark, chatFind, toolFailure, ToolFailureCause, toolGroups, toolOutput, mcpCalls, model, TurnRows, conversation.css, both tests) against feat-cmux-next. Nothing was run; the 49 passing tool-failure tests, clean vp check --no-fmt and clean vp fmt --check are as reported.

Both P2s from the e21c63a verdict are fixed, and the base merges changed nothing beyond the PR's own lines.

P2 1, background shells. rowFailed (toolFailure.ts:217) is tool.background ? status === "failed" : isFailed(tool), and it is the one gate: ToolRow.tsx:87, CommandRow.tsx:42, and toolFailure itself (toolFailure.ts:221), so Find's shellOutput reads the same answer. Traced the case the P2 named (status completed, exitCode 1, background.state failed): no failure object, ToolRow.tsx:93 keeps the BackgroundMark, the Shell block gets the raw output and tool.exitCode, and Find indexes the same raw output, so painted and counted agree. Covered by tool-failure.test.tsx:322-332 (lone row keeps data-state="failed", no cv-tool__failed, no cause) and tool-failure-find.test.tsx:117-128 (lone and grouped, JSDOM).

P2 2, colour codes. shellOutput (toolFailure.ts:277-280) now strips ANSI on both paths: a failed call's failure.output (already stripped by drawn, toolFailure.ts:207) and a call that did not fail (tool.output). ShellBlock draws through useOutput (toolOutput.tsx:44), which strips the same way. Covered by tool-failure-find.test.tsx:109-115.

Merges. Head ToolRow vs base: the only differences are the PR's (rowFailed/failure at 86-89, the failed word at 112-118, the cause at 155, the Shell block's output and exit code at 160-166). Base's "Running {command}" label (82) is kept and BackgroundHint is absent on both sides. Head CommandRow vs base: base's exited local is gone and failureWord takes its place (92-95); the "Running" label (54-56) and the row's menuPartRef (64) are base's. ToolRun is identical to base. The CSS block (conversation.css:1971-2031) landed once.

Also checked: failure is defined exactly when failed (toolFailure returns an object on every path past the gate), so ToolRow.tsx:112 and CommandRow.tsx:92 gating on failure is safe; a ToolGroupRow of one call is only an edit with a diff (toolGroups.ts:68), where isFailed and rowFailed agree, so the word and the count never disagree.

Findings, none blocking:

  • P3 ToolRow.tsx:87-93 vs CommandRow.tsx:85,108: a background shell whose call status is failed draws differently alone and in a group. CommandRow keeps the BackgroundMark and hides the cause (!tool.background); ToolRow drops the mark (base did too) and adds the failure word plus the cause line. The mark asymmetry is base's; the cause line is new on one side only. Either guard ToolRow's ToolFailureCause the same way or drop CommandRow's guard.
  • P3 BackgroundMark.tsx:27-29 (and "Stopped" at 34-36): no data-find-skip, so Find paints "failed" on a background row but never counts it (the index holds command and output only). Base behaviour, but the PR added the skip to CommandRow's own failed word (CommandRow.tsx:93) and this sibling keeps the same invariant the new find test guards.
  • P3 chatFind.ts:36: a non-shell output is still indexed with its colour codes while ToolOutput (ToolRow.tsx:188, useOutput) and McpCallDetails strip them; the fix covers shells only. Base behaviour; stripAnsi on that branch would close it.
  • P3 CommandRow.tsx:64: the row's menuPartRef still offers raw tool.output (with Claude Code's "Exit code N" line) as Copy Output, while the opened Shell block's own ref (ShellBlock.tsx:49) offers the drawn text. Base passed raw too; not a regression.
  • Nit toolFailure.ts:237: the head window body.slice(0, HEAD_CHARS) can cut mid-line (and mid-surrogate). It only shows when no tail line names an error and that cut head line becomes the cause.

@teamleaderleo
teamleaderleo merged commit d35831e into feat-cmux-next Oct 23, 2026
142 of 146 checks passed
@teamleaderleo
teamleaderleo deleted the tp/prettier-errors branch October 23, 2026 04:24
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant