feat(cua-driver): add screenshot_out_file param; CLI no longer dumps base64 by default - #1414
Merged
Merged
Conversation
…base64 by default get_window_state now accepts screenshot_out_file (optional absolute path). When set, screenshot bytes are written to that file, the MCP image content block is omitted, and screenshot_file_path is returned in structuredContent instead — callers get the AX tree without paying the ~31 KB base64-in-context token cost. CLI change: removed mergeImageContentIntoJSON — stdout no longer splices screenshot_png_b64 into the JSON by default. Use --image-out <path> or the new screenshot_out_file param to get the image on disk. This fixes context saturation in agents that call get_window_state via CLI subprocess (OpenCode with local Ollama models being the motivating case). Docs: integrations.mdx OpenCode section now warns about the CLI fallback problem and shows the modalities config fix for local vision models. SKILL.md updated to remove the legacy jq .screenshot_png_b64 path and document screenshot_out_file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe changes introduce file-based screenshot handling to CUA Driver, allowing screenshots to be written to disk instead of embedded as base64 in responses. This includes updating documentation, adding optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
… consistency Aligns the CLI flag name with the tool param name (screenshot_out_file) so both interfaces use the same terminology. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/cua-driver/guide/getting-started/integrations.mdx`:
- Around line 95-97: Update the warning in the Callout: change the message to
reflect the new CLI behavior after CallCommand was changed — remove the claim
that raw base64 screenshot floods stdout and instead warn that image blocks are
dropped by the shell fallback unless callers explicitly use the CLI image output
options; reference get_window_state and advise using --image-out or the
screenshot_out_file option (and prefer running cua-driver as an MCP server) so
image data is preserved.
In `@libs/cua-driver/Skills/cua-driver/SKILL.md`:
- Around line 439-440: The documented response shape incorrectly lists
has_screenshot: bool; remove that entry from the SKILL.md doc for
get_window_state and instead document only the fields actually present on
AppStateSnapshot (screenshot_file_path and the screenshot_* metadata). Update
any nearby examples or JSON schemas in the same file to eliminate references to
has_screenshot so consumers only rely on screenshot_file_path and the
screenshot_* fields emitted by get_window_state.
- Around line 444-450: The fenced code block showing the example CLI usage for
"cua-driver get_window_state" is missing a language, causing markdownlint
warnings; update the fence to include a language (e.g., add "bash" after the
opening ```), so the block becomes a bash code fence and the two example lines
"cua-driver get_window_state
'{\"pid\":N,\"window_id\":W,\"screenshot_out_file\":\"/tmp/shot.jpg\"}'" and
"cua-driver get_window_state '{\"pid\":N,\"window_id\":W}' --image-out
/tmp/shot.jpg" are correctly highlighted.
In `@libs/cua-driver/Sources/CuaDriverCore/AppState/AppState.swift`:
- Around line 57-59: The doc for the public property screenshotFilePath (in
AppState.swift) is incorrect: GetWindowStateTool currently captures JPEG bytes
(captureWindow(format: .jpeg)) and advertises image/jpeg, so update either the
capture to write PNGs or, more simply, change the comment for screenshotFilePath
to reflect the actual image format produced (JPEG) or state that the file
extension/format matches what GetWindowStateTool writes (currently
JPEG/image/jpeg); reference screenshotFilePath, GetWindowStateTool and
captureWindow(format: .jpeg) when making the change so the public contract and
implementation stay consistent.
In `@libs/cua-driver/Sources/CuaDriverServer/Tools/GetWindowStateTool.swift`:
- Around line 277-283: The code currently writes screenshot bytes with try? and
unconditionally sets resolvedScreenshotFilePath, so I need you to perform the
write with error handling and only set resolvedScreenshotFilePath when the write
actually succeeds; locate the block that checks screenshotOutFile and
snapshot.screenshotPngBase64 (and the similar block around lines 287–290),
replace the silent try? bytes.write(to:) with a do/catch or a conditional write
that verifies success, and in the catch/failure path do not set
resolvedScreenshotFilePath (and instead allow the inline image handling to
remain); reference the variables/methods screenshotOutFile,
snapshot.screenshotPngBase64, Data(base64Encoded:), bytes.write(to:), and
resolvedScreenshotFilePath when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ac9182a-0096-4130-9a82-8a3e1a7fc179
📒 Files selected for processing (5)
docs/content/docs/cua-driver/guide/getting-started/integrations.mdxlibs/cua-driver/Skills/cua-driver/SKILL.mdlibs/cua-driver/Sources/CuaDriverCLI/CallCommand.swiftlibs/cua-driver/Sources/CuaDriverCore/AppState/AppState.swiftlibs/cua-driver/Sources/CuaDriverServer/Tools/GetWindowStateTool.swift
Comment on lines
+95
to
+97
| <Callout type="warn"> | ||
| **Always configure cua-driver as an MCP server — never rely on the CLI fallback.** If MCP is not wired up, OpenCode will call `cua-driver` as a shell subprocess and the full `get_window_state` response (including a ~31 KB base64 screenshot) lands as raw text in the model context, consuming thousands of tokens per call. | ||
| </Callout> |
Contributor
There was a problem hiding this comment.
Update this warning to match the new CLI behavior.
This still describes the old failure mode. After the CallCommand change, the default CLI path no longer splices screenshot base64 into stdout, so the problem with shell fallback is now "image blocks get lost unless you handle --image-out/screenshot_out_file," not "raw base64 floods the context."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/content/docs/cua-driver/guide/getting-started/integrations.mdx` around
lines 95 - 97, Update the warning in the Callout: change the message to reflect
the new CLI behavior after CallCommand was changed — remove the claim that raw
base64 screenshot floods stdout and instead warn that image blocks are dropped
by the shell fallback unless callers explicitly use the CLI image output
options; reference get_window_state and advise using --image-out or the
screenshot_out_file option (and prefer running cua-driver as an MCP server) so
image data is preserved.
Comment on lines
+439
to
+440
| - `has_screenshot: bool` — `true` when a screenshot was captured this turn | ||
| (either inline via MCP image block or written to `screenshot_file_path`). |
Contributor
There was a problem hiding this comment.
Remove has_screenshot from the documented response shape.
get_window_state does not emit a has_screenshot boolean here. AppStateSnapshot only carries screenshot_file_path and the screenshot_* metadata, so documenting has_screenshot will send consumers down a branch that never exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Skills/cua-driver/SKILL.md` around lines 439 - 440, The
documented response shape incorrectly lists has_screenshot: bool; remove that
entry from the SKILL.md doc for get_window_state and instead document only the
fields actually present on AppStateSnapshot (screenshot_file_path and the
screenshot_* metadata). Update any nearby examples or JSON schemas in the same
file to eliminate references to has_screenshot so consumers only rely on
screenshot_file_path and the screenshot_* fields emitted by get_window_state.
Comment on lines
444
to
450
| ``` | ||
| # canonical, works in every capture mode — writes the image bytes | ||
| # wherever you point, stdout stays readable (tree in som, summary | ||
| # in vision). stderr warns (exit 0) if the response had no image. | ||
| cua-driver get_window_state '{"pid":N,"window_id":W}' --image-out /tmp/shot.png | ||
|
|
||
| # som-only legacy path: pull the spliced base64 out of structuredContent. | ||
| # Prefer --image-out above — it's one flag vs a probe + pipe. | ||
| if [ "$(cua-driver get_window_state '{"pid":N,"window_id":W}' | jq -r '.has_screenshot')" = "true" ]; then | ||
| cua-driver get_window_state '{"pid":N,"window_id":W}' | jq -r '.screenshot_png_b64' | base64 -d > shot.png | ||
| fi | ||
| # write to file — stdout stays readable (AX tree / summary only, no base64) | ||
| cua-driver get_window_state '{"pid":N,"window_id":W,"screenshot_out_file":"/tmp/shot.jpg"}' | ||
|
|
||
| # CLI --image-out flag is equivalent and works for all capture modes | ||
| cua-driver get_window_state '{"pid":N,"window_id":W}' --image-out /tmp/shot.jpg | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language to this fenced example.
This block is missing a fence language, which is why markdownlint is flagging it.
Suggested fix
-```
+```bash
# write to file — stdout stays readable (AX tree / summary only, no base64)
cua-driver get_window_state '{"pid":N,"window_id":W,"screenshot_out_file":"/tmp/shot.jpg"}'
# CLI --image-out flag is equivalent and works for all capture modes
cua-driver get_window_state '{"pid":N,"window_id":W}' --image-out /tmp/shot.jpg</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 444-444: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Skills/cua-driver/SKILL.md` around lines 444 - 450, The
fenced code block showing the example CLI usage for "cua-driver
get_window_state" is missing a language, causing markdownlint warnings; update
the fence to include a language (e.g., add "bash" after the opening ```), so the
block becomes a bash code fence and the two example lines "cua-driver
get_window_state
'{\"pid\":N,\"window_id\":W,\"screenshot_out_file\":\"/tmp/shot.jpg\"}'" and
"cua-driver get_window_state '{\"pid\":N,\"window_id\":W}' --image-out
/tmp/shot.jpg" are correctly highlighted.
Comment on lines
+57
to
+59
| /// File-system path to the saved screenshot PNG, when the caller | ||
| /// requested a file-backed screenshot. Absent when no path was written. | ||
| public let screenshotFilePath: String? |
Contributor
There was a problem hiding this comment.
Document the actual screenshot format.
The new comment says screenshot_file_path points to a saved PNG, but GetWindowStateTool currently captures JPEG bytes (captureWindow(format: .jpeg)) and advertises image/jpeg. That makes the public contract misleading for consumers choosing an extension or decoder based on this field's docs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverCore/AppState/AppState.swift` around lines
57 - 59, The doc for the public property screenshotFilePath (in AppState.swift)
is incorrect: GetWindowStateTool currently captures JPEG bytes
(captureWindow(format: .jpeg)) and advertises image/jpeg, so update either the
capture to write PNGs or, more simply, change the comment for screenshotFilePath
to reflect the actual image format produced (JPEG) or state that the file
extension/format matches what GetWindowStateTool writes (currently
JPEG/image/jpeg); reference screenshotFilePath, GetWindowStateTool and
captureWindow(format: .jpeg) when making the change so the public contract and
implementation stay consistent.
Comment on lines
+277
to
+283
| if let outPath = screenshotOutFile, let b64 = snapshot.screenshotPngBase64 { | ||
| let expandedPath = (outPath as NSString).expandingTildeInPath | ||
| if let bytes = Data(base64Encoded: b64) { | ||
| let url = URL(fileURLWithPath: expandedPath) | ||
| try? bytes.write(to: url) | ||
| resolvedScreenshotFilePath = expandedPath | ||
| } |
Contributor
There was a problem hiding this comment.
Don't suppress the inline image unless the file write actually succeeded.
try? bytes.write(to:) drops I/O errors, but resolvedScreenshotFilePath is still set immediately afterward. On a permission or missing-directory failure, the response will advertise screenshot_file_path and also skip the inline image block, so the caller gets neither a real file nor image content.
Suggested fix
if let outPath = screenshotOutFile, let b64 = snapshot.screenshotPngBase64 {
let expandedPath = (outPath as NSString).expandingTildeInPath
if let bytes = Data(base64Encoded: b64) {
let url = URL(fileURLWithPath: expandedPath)
- try? bytes.write(to: url)
- resolvedScreenshotFilePath = expandedPath
+ do {
+ try bytes.write(to: url)
+ resolvedScreenshotFilePath = expandedPath
+ } catch {
+ textContent += "\n\n⚠️ Failed to write screenshot_out_file to \(expandedPath): \(error.localizedDescription)"
+ }
}
}Also applies to: 287-290
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverServer/Tools/GetWindowStateTool.swift`
around lines 277 - 283, The code currently writes screenshot bytes with try? and
unconditionally sets resolvedScreenshotFilePath, so I need you to perform the
write with error handling and only set resolvedScreenshotFilePath when the write
actually succeeds; locate the block that checks screenshotOutFile and
snapshot.screenshotPngBase64 (and the similar block around lines 287–290),
replace the silent try? bytes.write(to:) with a do/catch or a conditional write
that verifies success, and in the catch/failure path do not set
resolvedScreenshotFilePath (and instead allow the inline image handling to
remain); reference the variables/methods screenshotOutFile,
snapshot.screenshotPngBase64, Data(base64Encoded:), bytes.write(to:), and
resolvedScreenshotFilePath when making the change.
…_file - GetWindowStateTool: only set resolvedScreenshotFilePath when write actually succeeds; fall through to inline image block on I/O error - AppState: fix doc comment — saved file is JPEG not PNG - SKILL.md: remove stale has_screenshot entry; add bash fence language - integrations.mdx: update callout to reflect actual CLI behavior after mergeImageContentIntoJSON removal Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
screenshot_out_fileoptional param toget_window_state: when set, screenshot bytes are written to that path, the MCP image content block is omitted, andscreenshot_file_pathis returned instructuredContentinstead of inline base64.get_window_statestdout no longer includesscreenshot_png_b64by default. ThemergeImageContentIntoJSONsplice has been removed. Use--image-out <path>(existing CLI flag) or the newscreenshot_out_fileparam to get the image on disk.AppStateSnapshotgains ascreenshotFilePath: String?field (encoded asscreenshot_file_path).integrations.mdxOpenCode section now warns about the CLI-subprocess base64 saturation problem and includes themodalitiesconfig fix for local Ollama vision models.SKILL.mdremoves the legacyjq .screenshot_png_b64path and documentsscreenshot_out_file.Motivation
Agents calling
get_window_statevia CLI subprocess (e.g. OpenCode with a local Ollama model) were receiving ~31 KB of raw base64 in stdout on every call, saturating the context window in 1–2 turns. Thescreenshot_out_fileparam and the CLI default change together fix this: the model gets the AX tree + a file path, and reads the image from disk when it needs to reason over the screenshot.Test plan
swift buildclean ✅cua-driver get_window_state '{"pid":N,"window_id":W}'— stdout has noscreenshot_png_b64fieldcua-driver get_window_state '{"pid":N,"window_id":W,"screenshot_out_file":"/tmp/shot.jpg"}'— file written, response hasscreenshot_file_path, no base64 in stdoutcua-driver get_window_state '{"pid":N,"window_id":W}' --image-out /tmp/shot.jpg— file written via existing flag, no base64 in stdoutscreenshot_out_fileis not set🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
New Features