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
This commit is contained in:
0xGr1mm
2026-08-08 00:58:06 +03:00
committed by Teknium
parent 61f2738205
commit d303e18d09
2 changed files with 132 additions and 0 deletions
@@ -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
+25
View File
@@ -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: