mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 04:30:59 +00:00
fix: surface Bash tool image output instead of dropping it (#617)
## What's broken
The Bash branch of `toolUpdateFromToolResult` (`src/tools.ts`) guards
the array-content path on `"text" in result[0]`. When the local Bash
tool produces image content — `[{ type: "image", source: { type:
"base64", ... } }]`, e.g. when a command pipes a base64 data URI — the
guard rejects the array and `output` stays empty. The image is silently
dropped from the ACP `tool_call_update` and the client renders nothing.
## Fix
Distinguish text-only arrays (existing terminal-stream behavior) from
arrays containing non-text blocks. For the latter, bypass the terminal
`_meta` channel (binary payloads can't be streamed through it) and
delegate to `toAcpContentUpdate` so images surface as ACP content
blocks.
The change is contained to the Bash case in `toolUpdateFromToolResult`;
no other tool dispatch is affected.
## Tests
Three regression tests added in `src/tests/tools.test.ts` under a new
`describe("with image array tool_result …")`:
- pure image, `supportsTerminalOutput: false` — image surfaces as ACP
content
- pure image, `supportsTerminalOutput: true` — `_meta` is correctly
bypassed and image still surfaces
- mixed text + image — both blocks surface in order
All existing tests pass. Total: 240 → 243.
## Validation
- `npm run check` ✅
- `npm run build` ✅
- `npm run test:run` ✅ (243 passed, 13 integration skipped)
Co-authored-by: dave <dave@mint.lan>
Co-authored-by: Ben Brandt <benjamin.j.brandt@gmail.com>
This commit is contained in:
@@ -702,6 +702,77 @@ describe("Bash terminal output", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("with image array tool_result (local Bash image output path)", () => {
|
||||
// The local Bash tool emits image content as
|
||||
// `[{ type: "image", source: { type: "base64", ... } }]` when a
|
||||
// command produces an image (e.g. piping a base64 data URI).
|
||||
const makeImageBashResult = (
|
||||
data: string,
|
||||
media_type: "image/png" | "image/jpeg" | "image/gif" | "image/webp" = "image/png",
|
||||
): ToolResultBlockParam => ({
|
||||
type: "tool_result",
|
||||
tool_use_id: "toolu_bash",
|
||||
content: [
|
||||
{
|
||||
type: "image",
|
||||
source: { type: "base64", media_type, data },
|
||||
} as ImageBlockParam,
|
||||
],
|
||||
});
|
||||
|
||||
it("should surface image content as ACP image content (terminal disabled)", () => {
|
||||
const toolResult = makeImageBashResult("iVBORw0KGgo=");
|
||||
const update = toolUpdateFromToolResult(toolResult, bashToolUse, false);
|
||||
|
||||
expect(update.content).toEqual([
|
||||
{
|
||||
type: "content",
|
||||
content: { type: "image", data: "iVBORw0KGgo=", mimeType: "image/png" },
|
||||
},
|
||||
]);
|
||||
expect(update._meta).toBeUndefined();
|
||||
});
|
||||
|
||||
it("should bypass terminal _meta even when supportsTerminalOutput is true", () => {
|
||||
// Binary content cannot be streamed through the terminal-output
|
||||
// _meta channel, so the fix returns ACP content directly and skips
|
||||
// the terminal info/output/exit triple.
|
||||
const toolResult = makeImageBashResult("iVBORw0KGgo=", "image/jpeg");
|
||||
const update = toolUpdateFromToolResult(toolResult, bashToolUse, true);
|
||||
|
||||
expect(update.content).toEqual([
|
||||
{
|
||||
type: "content",
|
||||
content: { type: "image", data: "iVBORw0KGgo=", mimeType: "image/jpeg" },
|
||||
},
|
||||
]);
|
||||
expect(update._meta).toBeUndefined();
|
||||
});
|
||||
|
||||
it("should still surface multi-block content with text + image mixed", () => {
|
||||
const toolResult: ToolResultBlockParam = {
|
||||
type: "tool_result",
|
||||
tool_use_id: "toolu_bash",
|
||||
content: [
|
||||
{ type: "text", text: "generated:" },
|
||||
{
|
||||
type: "image",
|
||||
source: { type: "base64", media_type: "image/png", data: "AAAA" },
|
||||
} as ImageBlockParam,
|
||||
],
|
||||
};
|
||||
const update = toolUpdateFromToolResult(toolResult, bashToolUse, false);
|
||||
|
||||
expect(update.content).toEqual([
|
||||
{ type: "content", content: { type: "text", text: "generated:" } },
|
||||
{
|
||||
type: "content",
|
||||
content: { type: "image", data: "AAAA", mimeType: "image/png" },
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("toAcpNotifications with clientCapabilities", () => {
|
||||
|
||||
25
src/tools.ts
25
src/tools.ts
@@ -530,7 +530,9 @@ export function toolUpdateFromToolResult(
|
||||
// Extract output and exit code from either format:
|
||||
// 1. BetaBashCodeExecutionResultBlock: { type: "bash_code_execution_result", stdout, stderr, return_code }
|
||||
// 2. Plain string content from a regular tool_result
|
||||
// 3. Array content (e.g. [{ type: "text", text: "..." }])
|
||||
// 3. Array content (e.g. [{ type: "text", text: "..." }] for stdout,
|
||||
// or [{ type: "image", source: {...} }] when the local Bash tool
|
||||
// produces an image, e.g. piping a base64 data URI)
|
||||
let output = "";
|
||||
let exitCode = isError ? 1 : 0;
|
||||
|
||||
@@ -545,13 +547,20 @@ export function toolUpdateFromToolResult(
|
||||
exitCode = bashResult.return_code;
|
||||
} else if (typeof result === "string") {
|
||||
output = result;
|
||||
} else if (
|
||||
Array.isArray(result) &&
|
||||
result.length > 0 &&
|
||||
"text" in result[0] &&
|
||||
typeof result[0].text === "string"
|
||||
) {
|
||||
output = result.map((c: any) => c.text).join("\n");
|
||||
} else if (Array.isArray(result) && result.length > 0) {
|
||||
const textOnly = result.every(
|
||||
(c: any) => c && typeof c === "object" && typeof c.text === "string",
|
||||
);
|
||||
if (textOnly) {
|
||||
output = result.map((c: any) => c.text).join("\n");
|
||||
} else {
|
||||
// Image (or mixed non-text) content. Binary payloads can't be
|
||||
// streamed through the terminal-output _meta channel, so bypass
|
||||
// it and surface the blocks as ACP content. This handles the
|
||||
// local Bash tool's image output, which previously failed the
|
||||
// text-only guard and was silently dropped.
|
||||
return toAcpContentUpdate(result, isError);
|
||||
}
|
||||
}
|
||||
|
||||
if (supportsTerminalOutput) {
|
||||
|
||||
Reference in New Issue
Block a user