Skip to content

feat: bash tool UI component#890

Merged
graphite-app[bot] merged 1 commit intomainfrom
04-27-feat_bash_tool_ui_component
Apr 30, 2026
Merged

feat: bash tool UI component#890
graphite-app[bot] merged 1 commit intomainfrom
04-27-feat_bash_tool_ui_component

Conversation

@MaheshtheDev
Copy link
Copy Markdown
Member

No description provided.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 28, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
supermemory-mcp 654386c Apr 30 2026, 02:30 AM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 28, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
supermemory-app 654386c Commit Preview URL Apr 30 2026, 02:31 AM

@MaheshtheDev MaheshtheDev marked this pull request as ready for review April 28, 2026 04:06
Copy link
Copy Markdown
Member Author


How to use the Graphite Merge Queue

Add the label Main to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@claude
Copy link
Copy Markdown

claude Bot commented Apr 28, 2026

Claude finished @MaheshtheDev's task in 2m 0s —— View job


Re-review Complete

  • Get latest PR diff
  • Check if previously identified bugs were fixed
  • Review current implementation
  • Post review summary

Summary: The PR has addressed 2 of the 4 issues from the previous reviews:

Issue Status
Error text silently dropped ✅ Fixed
Memory tool names regressed ✅ Fixed (kept with comment)
exitCode=undefined shows amber ❌ Still present
Dead TOOL_META["bash"] entry ❌ Still present

Posted inline comments for the 2 remaining issues with suggested fixes.

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview: Adds a specialized BashToolDisplay component for rendering bash/terminal tool calls with expandable output, exit code coloring, and loading states.

Issues found: None — this looks good to ship.

The implementation is clean and handles all states correctly:

  • Loading states show spinner and "running..." indicator
  • Exit code 0 shows green, non-zero shows amber warning color
  • Error states handled appropriately
  • Output expansion is gated on actual content existing
  • React's default text escaping protects against XSS in stdout/stderr output

Minor note: PlusIcon and BookOpenIcon are now unused imports after removing the old tool entries, but this won't cause runtime issues.

Score: 9/10

Copy link
Copy Markdown
Contributor

@vorflux vorflux Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — found 2 issues.


Review with Vorflux

Comment thread apps/web/components/chat/message/agent-message.tsx
Comment thread apps/web/components/chat/message/agent-message.tsx
@vorflux
Copy link
Copy Markdown
Contributor

vorflux Bot commented Apr 28, 2026

Testing Results

Ran browser-based UI tests against all 5 render states of BashToolDisplay. 2 bugs found.


Bug 1 (Medium) — exitCode=undefined falsely shows amber warning color

The color logic output?.exitCode === 0 evaluates to false when exitCode is absent (exitCode?: number). Any tool completion that doesn't include an explicit exit code will show amber/warning styling instead of neutral or green.

Before (amber — wrong):

After fix (green — correct):

Suggested fix:

const exitSuccess = isDone
  ? output?.exitCode !== undefined ? output.exitCode === 0 : true
  : false
// Use `exitSuccess` in place of `output?.exitCode === 0` in both color ternaries

Bug 2 (Low) — TOOL_META["bash"] is dead code with a wrong label

The bash entry added to TOOL_META ({ label: "Memory", icon: TerminalIcon }) is never reached — ToolCallDisplay returns <BashToolDisplay/> for toolName === "bash" before TOOL_META is ever read. The label "Memory" is also semantically wrong for a bash/terminal tool. This entry can be safely removed.


All states verified (12/12 pass after fixes)

State Result
input-streaming / input-available — blue spinner + "running..."
output-available, exitCode=0 — green + expandable stdout
output-available, exitCode=1 — amber + expandable stderr
output-available, no output — green + "done"
error / output-error — red + "error" label
output-available, exitCode=undefinedwas amber, should be green ✅ (after fix)
Dead TOOL_META["bash"] entry removed ✅ (after fix)
Long command truncation

Attached Images

07-bug-exitcode-undefined-amber.png

09-after-fix-bottom.png

13-final-fixed-green.png

@MaheshtheDev MaheshtheDev force-pushed the 04-27-feat_bash_tool_ui_component branch from 716ad4f to e6df01f Compare April 29, 2026 01:51
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview: Adds a specialized BashToolDisplay component for bash/terminal tool calls with expandable output, exit code coloring, and loading states.

Previous issues addressed:

  • ✅ Error text is now rendered in the expandable section (fixes the "error details silently dropped" issue)
  • ✅ Legacy memory tool names (searchMemories, addMemory, fetchMemory) are kept with a clarifying comment

Issues found: 2 remaining from vorflux's browser testing:

  1. Medium: exitCode=undefined shows amber warning color — commands that complete successfully but don't include an explicit exit code will appear as warnings
  2. Low: TOOL_META["bash"] entry is dead code (never reached due to early return) with incorrect "Memory" label

Score: 8/10

Good progress on the previous feedback. The two remaining issues are minor but worth addressing before merge.

Comment thread apps/web/components/chat/message/agent-message.tsx
Comment thread apps/web/components/chat/message/agent-message.tsx
@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented Apr 30, 2026

Merge activity

@graphite-app graphite-app Bot force-pushed the 04-27-feat_bash_tool_ui_component branch from e6df01f to 654386c Compare April 30, 2026 02:27
@graphite-app graphite-app Bot merged commit 654386c into main Apr 30, 2026
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants