Repository navigation
feat(tools): structured error wrapper + TUI cause/recovery parsing - #1051
Conversation
Claude Code pattern: errors carry the cause, the current state, and a recovery path — not just a message string. The ops tools already follow this pattern; this extends it to every tool. - tools/errors.py: tool_error() builder, ToolError exception, and wrap_tool_exceptions() that converts common exceptions to structured errors with category-based recovery hints. - TUI ErrorMessage parses structured errors (is_error/tool/cause/recovery) and displays cause and recovery separately, with esc-to-dismiss. Partially addresses #1034, #1044
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35783d74d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { isActive: onDismiss !== undefined }, | ||
| ); | ||
|
|
||
| const structured = parseStructured(display); |
There was a problem hiding this comment.
Route structured tool results through their renderer
When a tool returns the new tool_error() payload, useAgent.ts records it as a tool_result, and EventItem.tsx sends that event to ToolCallMessage; ErrorMessage is instantiated only for the separate top-level agent.error state. Consequently, this parser never receives the structured tool results created by this commit, so operators continue to see the raw payload rather than separated cause and recovery text. Parse these payloads in ToolCallMessage or route structured error events to this component.
Useful? React with 👍 / 👎.
| try: | ||
| return func(*args, **kwargs) |
There was a problem hiding this comment.
Await asynchronous tools inside the exception boundary
When func is asynchronous—as are repository tools such as bash and browser_action—this call only returns a coroutine and leaves the try block; exceptions are raised later when the caller awaits it and therefore bypass every conversion branch below. Provide an async wrapper or detect and await awaitables inside this exception boundary so failures from asynchronous tools also become structured errors.
Useful? React with 👍 / 👎.
| except Exception as exc: # noqa: BLE001 — tools must not crash the agent | ||
| return tool_error( | ||
| tool_name, | ||
| cause=str(exc), | ||
| recovery="Inspect the error above; use `/doctor` for session diagnostics", | ||
| ) No newline at end of file |
There was a problem hiding this comment.
Preserve ToolError fields before the generic exception branch
When a wrapped function follows the documented raise ToolError(...) path, this catch-all handles it as an ordinary exception, uses the wrapper's tool_name, turns its cause into the prefixed str(exc), and replaces its supplied recovery text with the generic /doctor advice. Add a dedicated except ToolError branch that converts the exception's tool, cause, and recovery fields unchanged.
Useful? React with 👍 / 👎.
| ? content.slice(0, headLen) + "\n...\n" + content.slice(-tailLen) | ||
| : content; |
There was a problem hiding this comment.
Parse the structured payload before truncating it
In prompt mode, any serialized structured error longer than 1,000 characters is sliced into invalid JSON before parseStructured runs. Such errors therefore fall back to displaying fragmented raw JSON and lose the dedicated cause/recovery presentation. Parse content first, then truncate the individual rendered fields or their formatted output.
Useful? React with 👍 / 👎.
| * When the tool result is a structured error (from tools/errors.py), | ||
| * cause and recovery are displayed separately for readability. |
There was a problem hiding this comment.
Record this user-visible change in CHAGELOG.md
This commit changes operator-visible TUI error presentation, but CHAGELOG.md is absent from the changed files. Add a proposed, PR-linked entry under Unreleased before merge as required by the repository's versioned-change policy.
AGENTS.md reference: AGENTS.md:L3-L8
Useful? React with 👍 / 👎.
Renders ErrorMessage through ink-testing-library (real Ink, no mocks): - Structured error (is_error/tool/cause/recovery JSON) renders as 'error> [Tool] cause' + 'Recovery: ...' + 'esc to dismiss' - Plain text errors keep the existing fallback display - Caught by live rendering: the structured branch was previously untestable via DOM mocks because Ink Text nesting needs real layout
TUI rendering verification (real Ink)Rendered ErrorMessage through ink-testing-library (the real Ink renderer, no mocks) on Structured error frame (actual output): Plain error fallback (actual output): Cause and recovery render separately as designed, and the esc-to-dismiss hint appears only when onDismiss is provided. Added |
Direct TUI capture (real Ink, no mocks)Captured actual Ink frames with Structured error frame on the implementation branch: Plain error fallback: Pending tool frame: This was run against the actual Ink renderer, with no Ink/React mocks. The earlier branch mismatch was caught because the non-implementation branch rendered raw JSON; the implementation branch renders the structured cause/recovery correctly. |
- ops tools report failures as tool_error() envelopes
({is_error, tool, cause, recovery}) instead of ad-hoc error JSON
- EventItem routes tool results carrying that envelope to the
structured error display instead of the raw tool view
- drop the never-wired wrap_tool_exceptions/ToolError helpers so the
module only exports what is actually consumed
- add the missing ink-testing-library devDependency the render test
needs under CI's npm ci
- remove the internal design note accidentally committed at the root
- cover the envelope builder and the TUI routing with tests
…errors # Conflicts: # CHAGELOG.md
…errors # Conflicts: # CHAGELOG.md # clients/cli/src/components/messages/ErrorMessage.tsx
Partially addresses #1034, #1044
What
tools/errors.py: structured error builder (tool_error()), exception class (ToolError), and exception-to-structured wrapper (wrap_tool_exceptions()) with category-based recovery hints.ErrorMessageparses structured errors (is_error/tool/cause/recovery) and displays cause and recovery separately, mirroring Claude Code's error display pattern.tools/ops/tools.py:42-52); this extends it to every tool.Compatibility
Python: new module, no existing behavior changed. TS: ErrorMessage gains structured parsing with fallback to plain text display. Cross-platform.
Verification
Implementation: tools/errors.py + ErrorMessage.tsx update