From d303e18d09f50db9c3347b8a6acf54e28e13ca02 Mon Sep 17 00:00:00 2001 From: 0xGr1mm Date: Sat, 8 Aug 2026 00:58:06 +0300 Subject: [PATCH] fix(computer_use): ignore placeholder pid/window_id ids capture() read any non-None pid/window_id as a request for exact-window targeting. Several providers emit every declared schema property on every tool call, zero-filling unused optional integers, so those calls arrive as pid=0, window_id=0. The exact-target branch was then entered, the caller's app= was discarded, _positive_int(0) returned None for both ids, and the capture failed with a message pointing at pid/window_id. For that class of model capture(app=...) and frontmost capture never worked at all. Normalize non-positive ids to None before the branch decision so dispatch falls through to app/frontmost discovery. Malformed non-numeric ids are deliberately not treated as placeholders: they still reach the existing validation error instead of being silently ignored. Fixes #81333 --- .../test_computer_use_placeholder_ids.py | 107 ++++++++++++++++++ tools/computer_use/cua_backend.py | 25 ++++ 2 files changed, 132 insertions(+) create mode 100644 tests/tools/test_computer_use_placeholder_ids.py diff --git a/tests/tools/test_computer_use_placeholder_ids.py b/tests/tools/test_computer_use_placeholder_ids.py new file mode 100644 index 0000000000..320089c290 --- /dev/null +++ b/tests/tools/test_computer_use_placeholder_ids.py @@ -0,0 +1,107 @@ +"""A zero-filled ``pid``/``window_id`` must not be read as exact targeting. + +Several providers emit every declared schema property on every tool call, +filling unused optional integers with ``0``. ``capture()`` entered its +exact-target branch for any non-``None`` id, so those calls dropped the +caller's ``app=``, coerced both ids to ``None`` via ``_positive_int``, and +failed with a message pointing at pid/window_id rather than the real problem. +For that class of model ``capture(app=...)`` never worked at all. +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from typing import Any, Dict, List +from unittest.mock import MagicMock + +sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + +def _backend_with_windows(windows: List[Dict[str, Any]]): + from tools.computer_use.cua_backend import CuaDriverBackend + + backend = CuaDriverBackend() + backend._session = MagicMock() + backend._session.call_tool.return_value = { + "data": "", + "images": [], + "structuredContent": {"windows": windows}, + "isError": False, + } + return backend + + +_WINDOWS = [ + {"app_name": "Fuwari", "pid": 100, "window_id": 1, + "is_on_screen": True, "title": "menu bar", "z_index": 0}, + {"app_name": "Comet", "pid": 200, "window_id": 2, + "is_on_screen": True, "title": "Comet Browser", "z_index": 1}, +] + + +def test_app_scoped_capture_survives_placeholder_ids(): + """The bug: app= was discarded and the capture failed outright.""" + backend = _backend_with_windows(_WINDOWS) + + cap = backend.capture(mode="som", app="Comet", pid=0, window_id=0) + + assert cap.app == "Comet", ( + f"app= was dropped for a zero-filled call; got app={cap.app!r} " + f"detail={cap.window_title!r}" + ) + + +def test_frontmost_capture_survives_placeholder_ids(): + """Frontmost capture is the same failure with no app= to drop.""" + backend = _backend_with_windows(_WINDOWS) + + cap = backend.capture(mode="som", pid=0, window_id=0) + + # Frontmost is the highest z_index window. + assert cap.app == "Comet", f"frontmost capture failed; detail={cap.window_title!r}" + + +def test_real_exact_target_still_wins_over_app(): + """Guard: a genuine pid/window pair keeps its exact-targeting behaviour.""" + backend = _backend_with_windows(_WINDOWS) + + # Ids that match no enumerated window: only the exact branch can produce + # this label, so it proves discovery was bypassed rather than consulted. + cap = backend.capture(mode="som", app="Ghost", pid=999, window_id=99) + + assert cap.app == "Ghost", f"exact targeting was bypassed; detail={cap.window_title!r}" + + +def test_partial_target_still_reports_the_missing_half(): + """Guard: one real id and one placeholder is a caller error, not discovery.""" + backend = _backend_with_windows(_WINDOWS) + + cap = backend.capture(mode="som", app="Comet", pid=200, window_id=0) + + assert "requires both pid and window_id" in cap.window_title + + +def test_malformed_ids_are_not_treated_as_placeholders(): + """Guard: garbage must still reach the validation error, not be ignored.""" + backend = _backend_with_windows(_WINDOWS) + + cap = backend.capture(mode="som", app="Comet", pid="abc", window_id="def") + + assert "positive integer" in cap.window_title + + +def test_placeholder_predicate(): + from tools.computer_use.cua_backend import _is_placeholder_id + + assert _is_placeholder_id(0) is True + assert _is_placeholder_id("0") is True + assert _is_placeholder_id(-1) is True + assert _is_placeholder_id(200) is False + assert _is_placeholder_id("200") is False + # Garbage is not a placeholder: it must keep reaching the existing error. + assert _is_placeholder_id("abc") is False + assert _is_placeholder_id(None) is False + assert _is_placeholder_id(1.5) is False + # bools are ints in Python; False must not read as a placeholder id. + assert _is_placeholder_id(False) is False + assert _is_placeholder_id(True) is False diff --git a/tools/computer_use/cua_backend.py b/tools/computer_use/cua_backend.py index 0f189d5c73..4dec67ea14 100644 --- a/tools/computer_use/cua_backend.py +++ b/tools/computer_use/cua_backend.py @@ -1979,6 +1979,24 @@ def _positive_int(value: Any) -> Optional[int]: return parsed if parsed > 0 else None +def _is_placeholder_id(value: Any) -> bool: + """True when *value* is a schema-filler id rather than a real target. + + Several providers emit every declared schema property on every tool call, + filling unused optional integers with ``0``. A non-positive id cannot name + a window, so treating it as a targeting request drops the caller's ``app=`` + and fails the capture. Malformed non-numeric values are deliberately NOT + placeholders: those still reach the existing validation error rather than + being silently ignored. + """ + if isinstance(value, bool) or not isinstance(value, (int, str)): + return False + try: + return int(value) <= 0 + except ValueError: + return False + + def _ingest_windows(raw_windows: List[Dict[str, Any]]) -> List[Dict[str, Any]]: """Normalise cua-driver ``list_windows`` entries, dropping unusable ones. @@ -2414,6 +2432,13 @@ class CuaDriverBackend(ComputerUseBackend): # PR's effective minimum (trycua/cua#1961 + #1908) is well past # that, so the fallback is gone — the wrapper now treats the # structured shape as the only contract. + # Drop schema-filler ids before they can be read as a targeting + # request, so `capture(app=...)` and frontmost capture still work for + # models that emit every optional property zero-filled. + if _is_placeholder_id(pid): + pid = None + if _is_placeholder_id(window_id): + window_id = None # An exact pid/window pair is both the stable capture_after target and # the escape hatch when app/window discovery is unavailable on X11. if pid is not None or window_id is not None: