Skip to content

fix(computer-use): fail unverified CUA actions closed - #59730

Closed
bbopen wants to merge 1 commit into
NousResearch:mainfrom
bbopen:codex/fix-cua-action-verification
Closed

fix(computer-use): fail unverified CUA actions closed#59730
bbopen wants to merge 1 commit into
NousResearch:mainfrom
bbopen:codex/fix-cua-action-verification

Conversation

@bbopen

@bbopen bbopen commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fail CUA-backed computer_use actions closed when cua-driver reports structuredContent.verified: false, and preserve the structured action metadata in ActionResult.meta even when the human-readable data field is a string.

The same change also sends the active window_id on keyboard actions (type_text, press_key, hotkey) when capture() has already resolved a target window. That keeps keyboard input aligned with the same window-local targeting contract already used by capture, set_value, and element actions.

This is related to #59387, which preserves structured action metadata and adds window_id for coordinate pointer actions. It does not duplicate that PR completely: #59387 still returns ok=True for verified:false and does not pass window_id for bare active-window type_text/key actions.

Related Issue

Fixes #59731

Related: #59387

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Security fix
  • Documentation update
  • Tests (adding or improving test coverage)
  • Refactor (no behavior change)
  • New skill (bundled or hub)

Changes Made

  • tools/computer_use/cua_backend.py
    • merge structuredContent into action metadata for all CUA actions
    • set ActionResult.ok=False when structuredContent.verified is False
    • include active window_id on type_text, press_key, and hotkey calls when available
  • tests/tools/test_computer_use.py
    • add regression coverage for verified:false fail-closed behavior
    • add coverage that type_text, press_key, and hotkey carry window_id
    • add coverage that unverified key actions fail closed

How to Test

Reproduced on latest origin/main at 7426c09bee with an isolated fake CUA session: type_text and hotkey returned result_ok: true, empty metadata, and no window_id, even though the simulated driver payload contained structuredContent.verified: false.

Patched commit cdd9b29eaf returns result_ok: false, preserves verified:false/effect/path in meta, and sends window_id: 456 for the same active-window keyboard calls.

Validation run locally:

scripts/run_tests.sh tests/tools/test_computer_use.py tests/tools/test_computer_use_null_pid_windows.py tests/computer_use/test_doctor.py tests/computer_use/test_cua_spawn_env_sanitization.py -q
# 202 tests passed, 0 failed

python -m ruff check tools/computer_use/cua_backend.py tests/tools/test_computer_use.py tests/tools/test_computer_use_null_pid_windows.py tests/computer_use/test_doctor.py tests/computer_use/test_cua_spawn_env_sanitization.py
# All checks passed!

git diff --check
# passed

python -m py_compile tools/computer_use/cua_backend.py tests/tools/test_computer_use.py
# passed

Non-destructive live smoke on macOS with cua-driver 0.7.0 also passed: backend start, list_apps, and capture(mode="som") completed successfully.

Real action smoke on macOS with a disposable Tk text window:

  • Hermes resolved the probe window via real cua-driver list_windows (app_name: "python", concrete pid and window_id).
  • backend.type_text(...) sent the active window_id and the Tk process observed the typed text land in the widget.
  • The real driver still returned structuredContent.verified: false (path: "key_events", effect: "unverifiable"), so the patched branch returned ActionResult.ok == False while preserving that metadata.
  • A direct foreground delivery probe also landed text, but cua-driver still returned verified:false (path: "key_events_fg"). This is a driver verification limitation; the wrapper should not report it as confirmed success.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched existing issues and PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: macOS, with isolated synthetic CUA repro and non-destructive live CUA smoke

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) - N/A
  • I've updated cli-config.yaml.example if I added/changed config keys - N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows - N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide
  • I've updated tool descriptions/schemas if I changed tool behavior - N/A

Screenshots / Logs

Baseline origin/main synthetic repro:

{
  "type_text": {"result_ok": true, "result_meta": {}, "call_args": {"pid": 123, "session": "hermes-repro-fake", "text": "hello"}},
  "hotkey": {"result_ok": true, "result_meta": {}, "call_args": {"pid": 123, "session": "hermes-repro-fake", "keys": ["cmd", "a"]}}
}

Patched synthetic repro:

{
  "type_text": {"result_ok": false, "result_meta": {"verified": false, "effect": "unverifiable", "path": "key_events"}, "call_args": {"pid": 123, "session": "hermes-repro-fake", "text": "hello", "window_id": 456}},
  "hotkey": {"result_ok": false, "result_meta": {"verified": false, "effect": "unverifiable", "path": "key_events"}, "call_args": {"pid": 123, "session": "hermes-repro-fake", "keys": ["cmd", "a"], "window_id": 456}}
}

@bbopen
bbopen force-pushed the codex/fix-cua-action-verification branch from 352e239 to cdd9b29 Compare July 6, 2026 15:55
@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets tool/mcp MCP client and OAuth P2 Medium — degraded but workaround exists labels Jul 6, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused CUA evidence. The false-success portion remains a real current-main bug: tools/computer_use/cua_backend.py:2327-2335 derives success solely from isError and discards structuredContent for string responses.

Problems

  • The keyboard forwarding hunk is superseded and would weaken current behavior if applied as written. Current main requires a window ID and fails closed at tools/computer_use/cua_backend.py:1895-1921; the PR only forwards it when present, retaining a PID-only dispatch path.
  • The keyboard tests are now redundant with tests/tools/test_computer_use.py:2542-2614, landed in 2b6897f982d (#63725).

Suggested changes

  • Salvage the focused _action() metadata/fail-closed logic and add its regression tests.
  • Keep the stronger current-main keyboard routing implementation unchanged.

Automated hermes-sweeper review.

@@ -1679,7 +1679,10 @@ def type_text(self, text: str) -> ActionResult:
if pid is None:
return ActionResult(ok=False, action="type_text",
message="No active window — call capture() first.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not salvage this keyboard hunk as-is. Current main now fails closed when _active_window_id is absent; this conditional forwarding would retain PID-only keyboard dispatch in that state. Keep the current-main guard and limit salvage to _action() verification handling.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 15, 2026
@bbopen

bbopen commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Closing as this was superceded by #67123

@bbopen bbopen closed this Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

computer-use reports unverified CUA keyboard actions as success

3 participants