diff --git a/packages/flow/openadapt_flow/backends/win_agent/server.py b/packages/flow/openadapt_flow/backends/win_agent/server.py index d73a9db278..610783bae5 100644 --- a/packages/flow/openadapt_flow/backends/win_agent/server.py +++ b/packages/flow/openadapt_flow/backends/win_agent/server.py @@ -1722,7 +1722,8 @@ def _window_operation( process that its owner chain does not tie to the captured window, meets the region anywhere above the captured window; and an owned window meets the region (its visible frame, not the resize border). The topmost such - window is the answer. Whether the covered part hides anything that + window is the answer, with its visible frame as ``region`` + ``[x, y, w, h]``. A 1x1 region asks about one point. Whether the covered part hides anything that matters is for the caller, which still sees the uncovered part. A captured window that is gone or changed process, a cloak state that cannot be read for one of the app's windows, and anything else answer @@ -1812,6 +1813,9 @@ def _window_operation( "hwnd": candidate, "owner_hwnd": owner, "pid": pid, + # Its visible frame (DWM bounds, no shadow), in the region's + # pixels, so the caller can check what it covers. + "region": [rect[0], rect[1], rect[2] - rect[0], rect[3] - rect[1]], } if not reached: return {"status": "ok", "window": None} # nothing proves the order diff --git a/packages/flow/openadapt_flow/backends/windows_backend.py b/packages/flow/openadapt_flow/backends/windows_backend.py index ff5699ae14..232ab60dfe 100644 --- a/packages/flow/openadapt_flow/backends/windows_backend.py +++ b/packages/flow/openadapt_flow/backends/windows_backend.py @@ -859,11 +859,13 @@ def owned_window_over( ``region`` is ``(x, y, w, h)`` in click pixels; ``pinned_step_id`` is the last step whose identity check verified (its window recorded by :meth:`record_verified_window`). Returns ``{"title", "hwnd", - "owner_hwnd", "pid"}`` only when the first visible window above the + "owner_hwnd", "pid", "region"}`` only when the first visible window above the recorded one, of the same process and with an owner chain reaching it (an owned dialog, a message box), meets the region, and no window of another process, or unowned, meets the region above it. The topmost - owned window that meets it is returned. Returns None with no record for that step, when the + owned window that meets it is returned; ``region`` is its visible + frame (DWM bounds, no shadow) as ``[x, y, w, h]`` in the same click + pixels. A 1x1 ``region`` asks about one point. Returns None with no record for that step, when the recorded window is gone or now another process's, for any other process's window or an unowned one, and on any failure. Never raises. """ @@ -884,15 +886,22 @@ def owned_window_over( "hwnd", "owner_hwnd", "pid", + "region", }: return None title = window["title"] ints = [window[k] for k in ("hwnd", "owner_hwnd", "pid")] + frame = window["region"] if ( (title is not None and (not isinstance(title, str) or len(title) > 512)) or any( isinstance(v, bool) or not isinstance(v, int) or v <= 0 for v in ints ) + or not isinstance(frame, list) + or len(frame) != 4 + or any(isinstance(v, bool) or not isinstance(v, int) for v in frame) + or frame[2] <= 0 + or frame[3] <= 0 or window["pid"] != pinned[1] or window["hwnd"] == pinned[0] ): diff --git a/packages/flow/tests/test_win_agent_server.py b/packages/flow/tests/test_win_agent_server.py index 3fd5dbcf79..295dfd1349 100644 --- a/packages/flow/tests/test_win_agent_server.py +++ b/packages/flow/tests/test_win_agent_server.py @@ -1330,9 +1330,41 @@ def test_an_owned_dialog_over_the_region_is_reported(monkeypatch) -> None: "hwnd": 0x200, "owner_hwnd": _FakeDesktop.PINNED, "pid": 10, + "region": [200, 200, 300, 200], } +def test_the_reported_region_is_the_owned_windows_visible_frame(monkeypatch) -> None: + # The whole frame, not its overlap with the query, so the caller can + # check that it spans the band from the cut point to the right edge. + desktop = _FakeDesktop() + desktop.add(0x200, owner=_FakeDesktop.PINNED, rect=(295, 265, 400, 300)) + assert _owned_over(monkeypatch, desktop)["region"] == [295, 265, 105, 35] + + +def test_a_one_pixel_region_reports_the_topmost_owned_window_there( + monkeypatch, +) -> None: + # agnostic-repair asks at the dismiss-click point and requires the same + # hwnd as the cover. + desktop = _FakeDesktop() + desktop.add(0x200, owner=_FakeDesktop.PINNED, rect=(200, 200, 500, 400)) + desktop.add(0x300, owner=_FakeDesktop.PINNED, rect=(450, 350, 600, 450)) + assert _owned_over(monkeypatch, desktop, region=(300, 300, 1, 1))["hwnd"] == 0x200 + assert _owned_over(monkeypatch, desktop, region=(460, 360, 1, 1))["hwnd"] == 0x300 + # Its right and bottom edges are exclusive: (500, 300) is off 0x200. + assert _owned_over(monkeypatch, desktop, region=(500, 300, 1, 1)) is None + + +def test_a_one_pixel_region_under_a_foreign_window_is_none(monkeypatch) -> None: + desktop = _FakeDesktop() + desktop.add(0x200, owner=_FakeDesktop.PINNED, rect=(200, 200, 500, 400)) + desktop.add(0x900, pid=99, rect=(280, 280, 320, 320)) + assert _owned_over(monkeypatch, desktop, region=(300, 300, 1, 1)) is None + # A foreign window elsewhere does not decide a point it does not reach. + assert _owned_over(monkeypatch, desktop, region=(400, 350, 1, 1))["hwnd"] == 0x200 + + def test_a_topmost_owned_popup_does_not_hide_a_foreign_window_over_the_target( monkeypatch, ) -> None: diff --git a/packages/flow/tests/test_windows_backend.py b/packages/flow/tests/test_windows_backend.py index 7be735c21b..245938f820 100644 --- a/packages/flow/tests/test_windows_backend.py +++ b/packages/flow/tests/test_windows_backend.py @@ -830,7 +830,13 @@ def do_POST(self) -> None: # noqa: N802 return server, calls -_OWNED = {"title": "Session notice", "hwnd": 0x200, "owner_hwnd": 0x100, "pid": 10} +_OWNED = { + "title": "Session notice", + "hwnd": 0x200, + "owner_hwnd": 0x100, + "pid": 10, + "region": [200, 200, 300, 200], +} def test_owned_window_over_uses_the_verified_steps_window() -> None: @@ -866,6 +872,13 @@ def test_owned_window_over_uses_the_verified_steps_window() -> None: {**_OWNED, "hwnd": 0x100}, # the captured window itself {**_OWNED, "extra": 1}, {**_OWNED, "hwnd": True}, + # The covering frame is part of the answer; without it, doubt. + {k: v for k, v in _OWNED.items() if k != "region"}, + {**_OWNED, "region": None}, + {**_OWNED, "region": [200, 200, 300]}, + {**_OWNED, "region": [200, 200, 0, 200]}, + {**_OWNED, "region": [200, 200, True, 200]}, + {**_OWNED, "region": [200, 200, 30.5, 200]}, ], ) def test_owned_window_over_is_none_outside_the_contract(answer) -> None: