feat(ui): render images inline in tool results#374
Conversation
Messages queued in connection.pendingSends during WS downtime (iOS background) were flushed to the wrong session after reconnect. Add clearPendingSends() and call it from newSession() and switchSession(). Co-Authored-By: Claude Opus 4.6 <[email protected]>
…dant comment Co-Authored-By: Claude Opus 4.6 <[email protected]>
The server process inherits SSH_AUTH_SOCK at startup, but macOS invalidates the socket path after sleep/wake. Resolve via launchctl getenv at session spawn time so git push/fetch always works. Co-Authored-By: Claude Opus 4.6 <[email protected]>
When Claude reads an image file (JPEG, PNG, GIF, WebP) or reads a generated plot, the SDK returns image content blocks that were previously discarded. This adds end-to-end support for extracting and rendering those images inline in the ToolPill component. Data flow: SDK tool_result → extractToolResultImages() → WS event → protocol parser → messages reducer → ToolPill <img> render. Also handles subagent tool results with images. Co-Authored-By: Claude Opus 4.6 <[email protected]>
dimakis
left a comment
There was a problem hiding this comment.
Centaur Review
Found 5 issue(s) (2 warning).
packages/protocol/src/content-blocks.ts
Solid end-to-end implementation of image rendering in tool results. Main concerns: no tests for the new extractToolResultImages function, unbounded image data size through WebSocket, and an unrelated SSH fix bundled in.
- 🟡 missing_tests (L50):
extractToolResultImages()has no unit tests. It should be tested with: (1) string content (returns []), (2) undefined content (returns []), (3) array with image blocks, (4) array with non-image blocks, (5) image blocks missing source fields. The protocol package has no test files at all for content-blocks.[fixable] - 🔵 unsafe_assumptions (L56):
media_typefrom SDK content blocks is passed through without validation to adata:URI. While base64 data URIs are not exploitable for XSS in<img>tags (browser restricts script execution), validating against animage/*pattern would be a cheap defensive measure against unexpected MIME types.[fixable]
server/query-loop.ts
Solid end-to-end implementation of image rendering in tool results. Main concerns: no tests for the new extractToolResultImages function, unbounded image data size through WebSocket, and an unrelated SSH fix bundled in.
- 🟡 unsafe_assumptions (L1261): Image base64 data is forwarded to the client without any size limit. Text results are truncated at
TOOL_RESULT_MAX_CHARS, butresultImagespasses through unbounded. A tool returning a large image (e.g. a multi-MB screenshot) could produce an oversized WebSocket frame. Consider capping image data size or count.[fixable]
packages/client/src/slices/messages.ts
Solid end-to-end implementation of image rendering in tool results. Main concerns: no tests for the new extractToolResultImages function, unbounded image data size through WebSocket, and an unrelated SSH fix bundled in.
- 🔵 missing_tests (L329): The
TOOL_RESULTreducer action now acceptsimagesand threads them intotoolResultImagesviapatchToolResult, but there are no reducer tests covering this path. The existing store test only verifiesclearPendingSendsbehavior, not image state propagation. A reducer test should verify images survivepatchToolResult→finishCurrent→ rendered state.[fixable]
server/chat.ts
Solid end-to-end implementation of image rendering in tool results. Main concerns: no tests for the new extractToolResultImages function, unbounded image data size through WebSocket, and an unrelated SSH fix bundled in.
- 🔵 style (L409): The SSH agent socket resolution is unrelated to image rendering. It should be in a separate commit/PR to keep the changeset focused (the PR title is "render images inline in tool results").
| return text; | ||
| } | ||
|
|
||
| export function extractToolResultImages( |
There was a problem hiding this comment.
🟡 missing_tests: extractToolResultImages() has no unit tests. It should be tested with: (1) string content (returns []), (2) undefined content (returns []), (3) array with image blocks, (4) array with non-image blocks, (5) image blocks missing source fields. The protocol package has no test files at all for content-blocks. [fixable]
| if (typeof content === 'string' || !Array.isArray(content)) return []; | ||
| const images: ToolResultImage[] = []; | ||
| for (const c of content) { | ||
| if (c.type === 'image' && c.source?.type === 'base64' && c.source.data && c.source.media_type) { |
There was a problem hiding this comment.
🔵 unsafe_assumptions: media_type from SDK content blocks is passed through without validation to a data: URI. While base64 data URIs are not exploitable for XSS in <img> tags (browser restricts script execution), validating against an image/* pattern would be a cheap defensive measure against unexpected MIME types. [fixable]
| for (const block of content) { | ||
| if (block.type === 'tool_result') { | ||
| const resultText = extractToolResultText(block.content); | ||
| const resultImages = extractToolResultImages(block.content); |
There was a problem hiding this comment.
🟡 unsafe_assumptions: Image base64 data is forwarded to the client without any size limit. Text results are truncated at TOOL_RESULT_MAX_CHARS, but resultImages passes through unbounded. A tool returning a large image (e.g. a multi-MB screenshot) could produce an oversized WebSocket frame. Consider capping image data size or count. [fixable]
| @@ -322,6 +329,7 @@ export function messagesReducer(state: MessagesState, action: MessagesAction): M | |||
| action.toolId, | |||
There was a problem hiding this comment.
🔵 missing_tests: The TOOL_RESULT reducer action now accepts images and threads them into toolResultImages via patchToolResult, but there are no reducer tests covering this path. The existing store test only verifies clearPendingSends behavior, not image state propagation. A reducer test should verify images survive patchToolResult → finishCurrent → rendered state. [fixable]
| const venvPaths = getRepoConfig().resolvedVenvPaths; | ||
| env.PATH = [...venvPaths, existingPath].join(':'); | ||
|
|
||
| // Resolve the current SSH agent socket on macOS. The server process may have |
There was a problem hiding this comment.
🔵 style: The SSH agent socket resolution is unrelated to image rendering. It should be in a separate commit/PR to keep the changeset focused (the PR title is "render images inline in tool results").
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Centaur ReviewFound 5 issue(s) (3 warning).
|
|
Superseded by #376 (reference-based image rendering). Closing. |
Summary
extractToolResultImages()extractstype: 'image'content blocks from SDK tool results that were previously discarded<img>in ToolPillChanges
packages/protocol/src/types.tsToolResultImagetype +toolResultImagesfield on block typespackages/protocol/src/content-blocks.tsextractToolResultImages()functionserver/query-loop.tstool_resultWS eventspackages/client/src/slices/messages.tspatchToolResult()stores images, finish helpers propagatepackages/client/src/protocol-parser.tsfrontend/src/components/ToolPill.tsx<img>in tool resultsfrontend/src/styles/global.css.tool-pill-result-imgstylingTest plan
Read tool on photo.jpg) — image renders inline in expanded tool pill🤖 Generated with Claude Code