fix(tools): include trailing output in error briefs and render brief as plain text#2389
fix(tools): include trailing output in error briefs and render brief as plain text#2389liruifengv wants to merge 1 commit into
Conversation
…as plain text - Add ToolResultBuilder.tail() to extract last non-empty lines from output - Include tail output in Shell tool error briefs on non-zero exit - Include tail output in ACP Terminal tool error briefs on timeout/signal/error - Render Brief display blocks as plain text instead of Markdown
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d74c4fb234
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for chunk in reversed(self._buffer): | ||
| for line in reversed(chunk.splitlines()): |
There was a problem hiding this comment.
For commands that emit more than the builder limit before failing (for example a verbose test run that prints >50 KB and then writes the actual failure at the end), write() has already stopped accepting new data, so scanning _buffer here returns the tail of the retained prefix rather than the command's real trailing stderr/stdout. Because the shell failure brief now displays this value, users can still miss the actionable error or see misleading context; keep a small rolling tail independently of the capped output buffer before using it in failure briefs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR improves Shell tool failure reporting by adding trailing command output to error briefs (to surface actionable context) and by rendering briefs as plain text in the shell UI.
Changes:
- Add
ToolResultBuilder.tail()and tests to extract the last non-empty output lines for brief/error context. - Include trailing output in Shell/ACP error briefs when commands fail.
- Render
BriefDisplayBlockas plain text (rich.Text) instead of Markdown, and update changelogs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/utils/test_result_builder.py | Adds unit tests covering the new ToolResultBuilder.tail() behavior. |
| src/kimi_cli/tools/utils.py | Implements ToolResultBuilder.tail() for extracting trailing non-empty lines with truncation. |
| src/kimi_cli/tools/shell/init.py | Appends trailing output to Shell tool error brief for non-zero exit codes. |
| src/kimi_cli/acp/tools.py | Appends trailing output to ACP Terminal tool error briefs (timeout/signal/exit code). |
| src/kimi_cli/ui/shell/visualize/_blocks.py | Renders brief blocks as plain text (not Markdown) in the shell UI. |
| CHANGELOG.md | Adds an Unreleased entry describing the new brief behavior. |
| docs/en/release-notes/changelog.md | Mirrors the Unreleased changelog entry in English docs. |
| docs/zh/release-notes/changelog.md | Adds the corresponding Unreleased entry in Chinese docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def tail(self, max_lines: int = 5, max_line_len: int = 200) -> str: | ||
| """Return the last non-empty lines from the buffer, joined with newlines. | ||
|
|
||
| Useful for surfacing actionable error context (stderr) in tool result briefs. | ||
| """ |
| brief = f"Failed with exit code: {exitcode}" | ||
| tail = builder.tail() | ||
| if tail: | ||
| brief += f"\n{tail}" | ||
| return builder.error( |
| tail = builder.tail() | ||
| tail_md = f"\n{tail}" if tail else "" |
|
|
||
| ## 未发布 | ||
|
|
||
| - Shell：工具执行失败时显示完整错误信息 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| except TimeoutError: | ||
| return builder.error( |
There was a problem hiding this comment.
🟡 Shell tool omits tail output in timeout error brief, inconsistent with ACP Terminal
The Shell tool only appends builder.tail() to the brief for non-zero exit code errors (lines 120-123), but omits it for TimeoutError (lines 128-132). Meanwhile, the ACP Terminal tool (src/kimi_cli/acp/tools.py:145-150) consistently includes the tail in all error cases including timeout. Before the TimeoutError is raised, stdout_cb/stderr_cb have already written output to the builder (via _run_shell_command at src/kimi_cli/tools/shell/__init__.py:247-254), so builder.tail() would contain useful context about what the command was doing before it timed out.
(Refers to lines 128-132)
Was this helpful? React with 👍 or 👎 to provide feedback.
Shell: Show trailing output in tool error briefs when commands fail