chore(cua-driver): add CLI-first agent helper - #1426
Conversation
|
@kamellperry is attempting to deploy a commit to the Cua Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR updates cua-driver documentation to clarify canonical CLI workflows using ChangesCUA Driver CLI Guidance and Helper Script
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/cua-driver/scripts/agent-cli-helper.sh`:
- Around line 81-83: The cleanup handler unconditionally sets capture_mode to
"ax", clobbering any pre-existing mode; modify the script to read and save the
current mode into a variable before running "$CUA_DRIVER_BIN config set
capture_mode som", and have cleanup() restore that saved value (e.g.,
"$CUA_DRIVER_BIN config set capture_mode $PREV_CAPTURE_MODE" or skip restore if
empty), ensuring the trap EXIT uses this cleanup; also handle failures robustly
by defaulting to a safe value and silencing errors with "|| true" as needed.
- Around line 92-98: The script's screenshot branch uses the non-portable
`timeout` command (in the if that runs `timeout 20 "$CUA_DRIVER_BIN" call
screenshot ...`) which is missing on stock macOS; add a small detection at
script startup to set a TIMEOUT_CMD variable to the available binary (prefer
gtimeout if present, then timeout, else empty) and then replace the hardcoded
`timeout` invocation in the screenshot if-condition with
"${TIMEOUT_CMD:+$TIMEOUT_CMD 20 }" or equivalent so the script falls back
gracefully to calling "$CUA_DRIVER_BIN" without timeout when neither `gtimeout`
nor `timeout` exist; update the if-condition that invokes `"$CUA_DRIVER_BIN"
call screenshot` to use this TIMEOUT_CMD variable so the primary cua-driver path
isn't silently skipped on macOS.
- Line 84: The get_window_state invocation embeds the user-supplied `${out}`
path into a JSON string without escaping, so use the existing json_escape helper
to produce a safe JSON string; update the call that runs "$CUA_DRIVER_BIN" call
get_window_state
"{\"pid\":${pid},\"window_id\":${wid},\"screenshot_out_file\":\"${out}\"}" to
pass the escaped path (via json_escape) instead of raw ${out}—mirror how the
state command uses json_escape at the earlier invocation to ensure quotes and
backslashes are properly escaped.
🪄 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: 37fc0b90-ae24-415c-8681-7e560538af35
📒 Files selected for processing (2)
libs/cua-driver/Skills/cua-driver/SKILL.mdlibs/cua-driver/scripts/agent-cli-helper.sh
| "$CUA_DRIVER_BIN" config set capture_mode som >/dev/null | ||
| cleanup() { "$CUA_DRIVER_BIN" config set capture_mode ax >/dev/null 2>&1 || true; } | ||
| trap cleanup EXIT |
There was a problem hiding this comment.
state-shot cleanup unconditionally clobbers any pre-existing capture mode to ax.
If the caller was in som or vision mode before invoking state-shot, the trap cleanup EXIT silently resets their config to ax. The fix is to save the current mode before switching and restore it on cleanup.
🐛 Proposed fix: save and restore the previous capture mode
+ prev_mode=$("$CUA_DRIVER_BIN" config get capture_mode 2>/dev/null || echo "ax")
"$CUA_DRIVER_BIN" config set capture_mode som >/dev/null
- cleanup() { "$CUA_DRIVER_BIN" config set capture_mode ax >/dev/null 2>&1 || true; }
+ cleanup() { "$CUA_DRIVER_BIN" config set capture_mode "$prev_mode" >/dev/null 2>&1 || true; }
trap cleanup EXIT📝 Committable suggestion
‼️ 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.
| "$CUA_DRIVER_BIN" config set capture_mode som >/dev/null | |
| cleanup() { "$CUA_DRIVER_BIN" config set capture_mode ax >/dev/null 2>&1 || true; } | |
| trap cleanup EXIT | |
| prev_mode=$("$CUA_DRIVER_BIN" config get capture_mode 2>/dev/null || echo "ax") | |
| "$CUA_DRIVER_BIN" config set capture_mode som >/dev/null | |
| cleanup() { "$CUA_DRIVER_BIN" config set capture_mode "$prev_mode" >/dev/null 2>&1 || true; } | |
| trap cleanup EXIT |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/scripts/agent-cli-helper.sh` around lines 81 - 83, The
cleanup handler unconditionally sets capture_mode to "ax", clobbering any
pre-existing mode; modify the script to read and save the current mode into a
variable before running "$CUA_DRIVER_BIN config set capture_mode som", and have
cleanup() restore that saved value (e.g., "$CUA_DRIVER_BIN config set
capture_mode $PREV_CAPTURE_MODE" or skip restore if empty), ensuring the trap
EXIT uses this cleanup; also handle failures robustly by defaulting to a safe
value and silencing errors with "|| true" as needed.
| "$CUA_DRIVER_BIN" config set capture_mode som >/dev/null | ||
| cleanup() { "$CUA_DRIVER_BIN" config set capture_mode ax >/dev/null 2>&1 || true; } | ||
| trap cleanup EXIT | ||
| "$CUA_DRIVER_BIN" call get_window_state "{\"pid\":${pid},\"window_id\":${wid},\"screenshot_out_file\":\"${out}\"}" |
There was a problem hiding this comment.
User-supplied output path is not JSON-escaped, so paths with " or \ produce malformed JSON.
Line 84 embeds ${out} directly into the JSON string literal. The existing json_escape helper (line 36) already handles this correctly and is used for the state command's query arg on line 70.
🐛 Proposed fix: use json_escape for the path
+ out_json=$(json_escape "$out")
- "$CUA_DRIVER_BIN" call get_window_state "{\"pid\":${pid},\"window_id\":${wid},\"screenshot_out_file\":\"${out}\"}"
+ "$CUA_DRIVER_BIN" call get_window_state "{\"pid\":${pid},\"window_id\":${wid},\"screenshot_out_file\":${out_json}}"📝 Committable suggestion
‼️ 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.
| "$CUA_DRIVER_BIN" call get_window_state "{\"pid\":${pid},\"window_id\":${wid},\"screenshot_out_file\":\"${out}\"}" | |
| out_json=$(json_escape "$out") | |
| "$CUA_DRIVER_BIN" call get_window_state "{\"pid\":${pid},\"window_id\":${wid},\"screenshot_out_file\":${out_json}}" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/scripts/agent-cli-helper.sh` at line 84, The get_window_state
invocation embeds the user-supplied `${out}` path into a JSON string without
escaping, so use the existing json_escape helper to produce a safe JSON string;
update the call that runs "$CUA_DRIVER_BIN" call get_window_state
"{\"pid\":${pid},\"window_id\":${wid},\"screenshot_out_file\":\"${out}\"}" to
pass the escaped path (via json_escape) instead of raw ${out}—mirror how the
state command uses json_escape at the earlier invocation to ensure quotes and
backslashes are properly escaped.
| if timeout 20 "$CUA_DRIVER_BIN" call screenshot "{\"window_id\":${wid},\"format\":\"png\"}" --screenshot-out-file "$out" >/tmp/cua-driver-agent-screenshot.log 2>&1 && test -s "$out"; then | ||
| printf '%s\n' "$out" | ||
| else | ||
| /usr/sbin/screencapture -x -l "$wid" "$out" | ||
| test -s "$out" | ||
| printf '%s\n' "$out" | ||
| fi |
There was a problem hiding this comment.
timeout is not available on stock macOS, silently bypassing the cua-driver screenshot path entirely.
"macOS does not include the GNU Coreutils timeout command by default." Homebrew installs it prefixed as gtimeout, not timeout.
With stderr redirected to the log file (2>&1), the bash: timeout: command not found error is silently swallowed, test -s "$out" fails (nothing was written), and the else branch runs unconditionally on every stock macOS install. The entire if arm — the primary cua-driver screenshot path — is dead code for users without GNU coreutils.
🐛 Proposed fix: resolve a timeout command at the top of the script
CUA_DRIVER_BIN="${CUA_DRIVER_BIN:-cua-driver}"
SHOT_DIR="${CUA_SCREENSHOT_DIR:-/tmp/cua-screenshots}"
+TIMEOUT_CMD=$(command -v timeout 2>/dev/null || command -v gtimeout 2>/dev/null || echo "")Then in the screenshot case:
- if timeout 20 "$CUA_DRIVER_BIN" call screenshot "{\"window_id\":${wid},\"format\":\"png\"}" --screenshot-out-file "$out" >/tmp/cua-driver-agent-screenshot.log 2>&1 && test -s "$out"; then
+ if [[ -n "$TIMEOUT_CMD" ]] && "$TIMEOUT_CMD" 20 "$CUA_DRIVER_BIN" call screenshot "{\"window_id\":${wid},\"format\":\"png\"}" --screenshot-out-file "$out" >/tmp/cua-driver-agent-screenshot.log 2>&1 && test -s "$out"; then
+ # or if no timeout available, run without it:
+ # if "$CUA_DRIVER_BIN" call screenshot ... >/tmp/cua-driver-agent-screenshot.log 2>&1 && test -s "$out"; thenOr use a portable wrapper:
+_timeout() {
+ local t="$1"; shift
+ if command -v timeout >/dev/null 2>&1; then timeout "$t" "$@"
+ elif command -v gtimeout >/dev/null 2>&1; then gtimeout "$t" "$@"
+ else "$@"
+ fi
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/scripts/agent-cli-helper.sh` around lines 92 - 98, The
script's screenshot branch uses the non-portable `timeout` command (in the if
that runs `timeout 20 "$CUA_DRIVER_BIN" call screenshot ...`) which is missing
on stock macOS; add a small detection at script startup to set a TIMEOUT_CMD
variable to the available binary (prefer gtimeout if present, then timeout, else
empty) and then replace the hardcoded `timeout` invocation in the screenshot
if-condition with "${TIMEOUT_CMD:+$TIMEOUT_CMD 20 }" or equivalent so the script
falls back gracefully to calling "$CUA_DRIVER_BIN" without timeout when neither
`gtimeout` nor `timeout` exist; update the if-condition that invokes
`"$CUA_DRIVER_BIN" call screenshot` to use this TIMEOUT_CMD variable so the
primary cua-driver path isn't silently skipped on macOS.
|
Closing this PR because we're moving the work into Omni-World-LLC/computer-use-agents as the canonical repo. |
1 participant
Summary
cua-driver call <tool>syntaxTest Plan
--image-out/ missing SCREENSHOT.md refsSummary by CodeRabbit
New Features
Documentation