feat(mcp): fbash structured colored renderer#33
Merged
Conversation
Replace the minimal fbash MCP renderer with a Monokai-themed structured
layout that matches the other fsuite renderers (renderFmapResult,
renderFreadResult, etc.). The new renderer surfaces per-stream coloring,
the command classification, duration, exit status, background job
callouts, truncation warnings, and routing hints in visually distinct
zones so agents can parse fbash output at a glance instead of reading
a raw text blob.
Rendered layout (closes fcase #359):
1. Header — bold gold "fbash" + dim class + cyan duration + exit badge
(green exit=0 / red exit=N; header is hidden for bg starts).
2. Stdout — 2-space indent, preserved whitespace, truncated at 30
visible lines with a dim "... N more lines" trailer.
3. Stderr — bold red "stderr:" label + muted-warm body, only shown
when non-empty, truncated at 10 visible lines.
4. Status — (silent success) / (no output) markers for the empty
stdout + stderr edge cases, keyed off exit_code.
5. Footer — truncation reason, warnings[] (orange warn glyph), and
routing_suggestion / next_hint (cyan arrow, bold tool
name). Only one hint is emitted, preferring routing_suggestion
when present.
6. Background — when metadata.background_job_id is set and
exit_code is null, emits a "[bg job <id> started · <cmd>
· poll: <next_hint>]" callout and skips the stdout/stderr
blocks (the job hasn't produced final output yet).
The renderer returns null on parse failure or when d.tool !== "fbash",
letting cli() fall back to the raw text path — matches the existing
renderer contract in mcp/index.js.
Also update the pre-existing "fbash pretty output does not color
exit=null as an error" test to assert on the new "[bg job <id>
started...]" callout format instead of the old literal
"background_job_id" string, while preserving the original intent
(exit=null must never render in red and the job id must still be
surfaced for the caller).
All 35 MCP tests pass.
Exercise renderFbashResult through the real MCP stdio client across
all six test cases from fcase #359:
1. printf hello — simple success: gold header, exit=0, stdout block
2. false — silent failure: red exit=1, (no output) marker
3. sh -c OUT/ERR — mixed streams: all three zones rendered distinctly
4. sleep --bg — background job: [bg job ...] callout, no streams
5. ls /nope — stderr-only error: red exit, stderr block, hint
6. seq 1 500 — long stdout: truncated with "... N more lines"
Also pins invariants that the renderer contract relies on:
- structuredContent is never returned (Claude Code "early return
blender" discards content[text] when structuredContent is set)
- the fbash header is always bold + 256-color gold (fg 255,215,0)
- exit=null never appears in plain or colored text
Contributor
|
Caution Review failedPull request was closed or merged during review Summary by CodeRabbitRelease Notes
WalkthroughThe PR refactors the Changes
Sequence DiagramsequenceDiagram
participant Test as Test Harness
participant Server as MCP Server<br/>(index.js)
participant Fbash as fbash Tool<br/>(exec)
participant Renderer as renderFbashResult
Test->>Server: Spawn: node ./index.js
Test->>Server: MCP call: fbash tool<br/>(command, args)
alt Background Job Start
Fbash-->>Server: {background_job_id, exit_code: null}
Server->>Renderer: Parse result
Renderer-->>Server: [bg job fbash_<id>_<id>]<br/>(early return)
else Normal Execution
Fbash-->>Server: {stdout, stderr, exit_code}
Server->>Renderer: Parse result
Renderer->>Renderer: Build header<br/>(fbash, duration_ms, exit=N)
Renderer->>Renderer: Render stdout<br/>(cap 30 lines)
Renderer->>Renderer: Render stderr<br/>(cap 10 lines, muted)
Renderer->>Renderer: Add truncation<br/>indicators if needed
Renderer-->>Server: out.join("\n") + "\n"
end
Server-->>Test: MCP response:<br/>rendered text
Test->>Test: Assert ANSI<br/>sequences & labels
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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: af5ab2d357
ℹ️ 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".
| const isBackgroundStart = bgId && d.exit_code == null && bgStatus !== "completed"; | ||
| if (isBackgroundStart) { | ||
| const pollHint = d.next_hint ? ` ${DIM}· poll:${UNDIM} ${INFO}${d.next_hint}${RESET}` : ""; | ||
| out.push(`${GRAY}[bg job ${bgId} started${d.command ? ` · ${d.command}` : ""}${pollHint}${GRAY}]${RESET}`); |
There was a problem hiding this comment.
The background callout interpolates d.command verbatim, so multi-line commands or ANSI/control sequences break the renderer layout and can inject spoofed lines into the MCP text output. For example, a background command containing \n renders as extra lines inside the [bg job ...] block, which makes the status text ambiguous and can mislead downstream agent parsing. Please sanitize/escape control characters (or omit the raw command) before formatting this line.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes fcase #359 (
fbash-colored-structured-mcp-output).Replaces the previous minimal
renderFbashResultinmcp/index.jswith a Monokai-themed structured layout that matches the rest of the fsuite MCP renderers (renderFmapResult,renderFreadResult,renderFcontentResult, ...). Before this PR,fbashsurfaced only a bareexit=Nline plus raw stdout/stderr, ignoring the command class, duration, background job metadata, warnings, routing suggestions, and next hints thatfbash -o jsonalready emits. Agents consumingfbashoutput had to either parse text blobs or fall through to the raw JSON envelope.Rendered layout
fbash(256-color fg 220 /fg(255,215,0)) + dimclass=<class>+ cyan<Nms>duration + exit badge (greenexit=0/ redexit=N). Header is emitted for every call except background starts, where the exit badge is hidden becauseexit_codeisnull(still running).... N more linestrailer. Empty stdout is rendered as nothing (no phantomSTDOUT:header).stderr:label + muted-warm body (fg(170,160,140)— distinct from stdout without screaming), only shown when non-empty, truncated at 10 lines.(silent success)onexit=0with empty streams,(no output)on non-zero exit with empty streams.warnings[]with orange warn glyph,routing_suggestion.tool/next_hintwith cyan arrow + bold tool name. Only one hint is emitted, preferringrouting_suggestionwhen both are present.metadata.background_job_idis set andexit_codeisnull, emits[bg job <id> started · <cmd> · poll: <next_hint>]in dim gray and skips the stdout/stderr blocks entirely (the job has not produced final output yet).The renderer returns
nullon parse failure or whend.tool !== "fbash", lettingcli()fall back to raw text — the graceful-fallback contract of the other renderers.Design research
/modelcontextprotocol/typescript-sdkvia Context7): confirmed the tool-result contract is{content: [{type: "text", text}], structuredContent?}.structuredContentis intentionally omitted because Claude Code's ''early return blender'' discardscontent[text]when both are set (existing comment atmcp/index.js:1029). All existing rendered fsuite tools already follow this text-only pattern; this PR matches.content[text]is sound.fbashJSON envelope (./fbash -o json): all 14 fields now consumed —tool,command,command_class,exit_code,duration_ms,stdout,stderr,truncated,truncation_reason,warnings[],routing_suggestion.{tool,reason},next_hint,metadata.background_job_id,metadata.background_status.Before / after
Before
After
Rendered test-case snapshots (ANSI stripped)
1.
printf hello— simple success2.
false— silent failure3.
sh -c "echo OUT; echo ERR >&2; exit 2"— mixed streams4.
sleep 0.2 --background— background job5.
ls /nonexistent-path-xxxxx— stderr-only error (see before/after above)6.
seq 1 500— truncationTest plan
mcp/fbash-render.test.mjs— 7 end-to-end tests viaStdioClientTransportcovering all 6 brief cases + the ''header is always bold gold'' invariant.structured-parity.test.mjs— the ''fbash pretty output does not color exit=null as an error'' test previously asserted on the literal stringbackground_job_id; rewritten to match the new[bg job <id> started …]callout while preserving the original intent (exit=null must never appear in plain or colored text).cd mcp && node --test— 35 / 35 pass, zero regressions.result.structuredContentisundefined(contract with Claude Code's early-return blender).nullsocli()falls through to the raw text path.Commits
feat(mcp): add structured colored renderFbashResult— renderer rewrite + updatedstructured-parityassertiontest(mcp): add fbash renderer end-to-end coverage— new dedicated test suiteCloses fcase #359.