fix(server): return capture_ui_snapshot images as ImageContent - #112
Conversation
capture_ui_snapshot registered no onSuccess transform, so the whole bundle — including the base64 frames under screenshots.images — was serialised into a single TextContent. One 1512x882 desktop frame is ~145 KB of base64, which overflows the response budget of MCP clients that cap tool output, making the documented "unified evidence" entry point unusable over MCP (the CLI path is unaffected: it prints the CoreResult envelope and never reaches this layer). Lift the images out of the bundle into ImageContent artifacts, the same contract get_screenshots already uses, and emit the remaining bundle JSON as text with screenshots.images emptied. imageSummaries still carries the per-image ids and hashes, so the JSON stays self-describing. The fileUrls branch is untouched: with --save-images there are no inline images and the bundle remains a single TextContent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthrough
ChangesUI snapshot artifacts
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/server_capability_core/lib/src/tools/inspection_tools.dart (1)
194-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse explicit local types in the changed paths.
packages/server_capability_core/lib/src/tools/inspection_tools.dart#L194-L196: declare concreteMap<String, Object?>andList<String>types for the decoded bundle values.packages/server_capability_core/test/tools/inspection_tools_test.dart#L598-L631: declare concrete types for the runner, registration result, decoded bundle, and image collections.As per coding guidelines, “Explicit type declarations must be used.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server_capability_core/lib/src/tools/inspection_tools.dart` around lines 194 - 196, The decoded values in inspection_tools.dart at lines 194-196 must use explicit local types: declare bundle and screenshots as Map<String, Object?> and images as List<String>. In packages/server_capability_core/test/tools/inspection_tools_test.dart lines 598-631, add concrete type declarations for the runner, registration result, decoded bundle, and image collections, preserving the existing behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/server_capability_core/lib/src/tools/inspection_tools.dart`:
- Around line 194-196: The decoded values in inspection_tools.dart at lines
194-196 must use explicit local types: declare bundle and screenshots as
Map<String, Object?> and images as List<String>. In
packages/server_capability_core/test/tools/inspection_tools_test.dart lines
598-631, add concrete type declarations for the runner, registration result,
decoded bundle, and image collections, preserving the existing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f8152a11-3cd1-4a64-8a1e-b291573a27c5
📒 Files selected for processing (2)
packages/server_capability_core/lib/src/tools/inspection_tools.dartpackages/server_capability_core/test/tools/inspection_tools_test.dart
|
hi @dipsy, thank you very much for the PR! |
|
@dipsy already contributed before to code, maintenance, bug |
Problem
capture_ui_snapshotis registered without anonSuccesstransform (inspection_tools.dart), sorunCommandfalls through to the default path andagentResultToMcpResultserialises the entire bundle into oneTextContent. WithsaveImagesToFilesoff (the default),screenshots.imagesholds inline base64, so a single 1512×882 desktop frame ships ~145 KB of base64 as text.Clients that cap tool output reject the result outright, which makes the tool the docs promote as step 1 of the debugging workflow ("unified evidence") unusable over MCP unless the server is started with
--save-images. Observed with Claude Code:fmt_capture_ui_snapshotfails with "result (145,873 characters) exceeds maximum allowed tokens", whilefmt_get_screenshotsagainst the same app and the same VM extension returns fine.The CLI is unaffected —
fmtk execprints theCoreResultenvelope and never reaches the capability tool layer — so the documented CLI recipes keep their current output.Change
Give
capture_ui_snapshotthe artifact contractget_screenshotsalready uses:ImageContentblocks (AgentArtifact.text(image, mimeType: 'image/png'));screenshots.imagesemptied —imageSummariesstill carries the per-image ids/hashes, so the JSON stays self-describing;fileUrlsbranch is untouched: with--save-imagesthere are no inline images and the bundle stays a singleTextContent.Verification
dart testinpackages/server_capability_core— 241 tests pass, including two new cases (images lifted intoimage/pngartifacts with the JSON stripped;fileUrlsbundle still a single text block).tools/call fmt_capture_ui_snapshotnow returns 2 content blocks —text15,524 chars +image(image/png) — instead of one 145 KB text block.Summary by CodeRabbit
New Features
Bug Fixes