From f6db7cb16adc596a49d49c39636d36502b43fa53 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 10:50:23 -0400 Subject: [PATCH 1/3] focus_on: check who has the focus after a key that changes nothing visible A Tab onto an icon button, or an ArrowDown that highlights an option, gave its step no postcondition, and a strict policy refused it as vacuous. The recorder now keeps the focused element's identity around each key, and the compiler adds a focus_on postcondition when that element has a specific identity no other element shares at teach time: a data-testid, an id, or a role with a non-empty accessible name. At replay the focused element must have that identity and be the only one with it, and, for an option highlight, an option must be active (aria-activedescendant or aria-selected). Anything weaker asserts nothing, never passes, and still counts as vacuous. Co-Authored-By: Claude Opus 5.5 --- .../backends/playwright_backend.py | 93 +++++++ .../flow/openadapt_flow/compiler/compile.py | 76 ++++++ packages/flow/openadapt_flow/ir.py | 53 +++- packages/flow/openadapt_flow/policy.py | 19 +- packages/flow/openadapt_flow/recorder.py | 26 ++ .../flow/openadapt_flow/runtime/replayer.py | 55 ++++- .../flow/tests/test_focus_on_postcondition.py | 228 ++++++++++++++++++ 7 files changed, 546 insertions(+), 4 deletions(-) create mode 100644 packages/flow/tests/test_focus_on_postcondition.py diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index 094d97597..281f38456 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -396,6 +396,44 @@ class _FramePoint: ' [role="radio"], [role="switch"]' ) +#: The focused element's own facts, in its document (``focus_identity``). +_FOCUS_IDENTITY_JS = r"""(el) => { + const doc = el.ownerDocument; + if (el === doc.body || el === doc.documentElement) return {none: true}; + const count = (sel) => { + try { return doc.querySelectorAll(sel).length; } catch (e) { return 0; } + }; + const testid = el.getAttribute('data-testid') || null; + const id = el.id || null; + let active = false; + const activeId = el.getAttribute('aria-activedescendant'); + if (activeId && doc.getElementById(activeId)) active = true; + if (!active) { + for (const attr of ['aria-controls', 'aria-owns']) { + const ref = el.getAttribute(attr); + const box = ref ? doc.getElementById(ref) : null; + if (box && box.querySelector('[role="option"][aria-selected="true"]')) { + active = true; + break; + } + } + } + let top = false; + try { top = doc.defaultView === doc.defaultView.top; } catch (e) { top = false; } + return { + none: false, + tag: el.localName, + testid: testid, + testid_count: testid ? count('[data-testid="' + CSS.escape(testid) + '"]') : 0, + id: id, + id_count: id ? count('#' + CSS.escape(id)) : 0, + active_option: active, + top: top, + }; +}""" +#: The first line of an ARIA snapshot: ``- role "name"``. +_ARIA_LINE = re.compile(r'^\s*-\s+([a-z]+)(?:\s+"((?:[^"\\]|\\.)*)")?') + _BIND_COORDINATE_TARGET_JS = ( r"""(args) => { const hit = document.elementFromPoint(args.x, args.y); @@ -1870,6 +1908,61 @@ def focused_element_matches(self, selector: str) -> Optional[bool]: ) return result if isinstance(result, bool) else None + def focus_identity(self) -> Optional[dict[str, Any]]: + """Who has the focus: its testid, id, role and name, and how many share them. + + For the ``focus_on`` postcondition. ``testid_count`` / ``id_count`` + count the elements with the same ``data-testid`` / ``id`` in the + focused element's document; ``role_name_count`` counts the elements + with the same role and accessible name on the page (Playwright's own + accessibility tree, as replay resolves a role and a name). Role and + name are read only in the top document. ``active_option`` is whether + an option of the focused control is active: its + ``aria-activedescendant`` names an element, or the listbox it + controls has an ``aria-selected`` option. ``{"none": True}`` when + nothing has the focus; None when it can't be read. + """ + + info = self._focused_element_call(_FOCUS_IDENTITY_JS) + if not isinstance(info, dict): + return None + if info.get("none"): + return {"none": True} + role: Optional[str] = None + name: Optional[str] = None + role_name_count = 0 + if info.get("top"): + try: + snapshot = self.page.locator(":focus").aria_snapshot(timeout=1000) + except Exception: # noqa: BLE001 - no snapshot: no role and name + snapshot = "" + first = snapshot.splitlines()[0] if snapshot else "" + match = _ARIA_LINE.match(first) + if match: + role = match.group(1) + name = (match.group(2) or "").replace('\\"', '"').strip() or None + if role and name: + try: + role_name_count = self.page.get_by_role( + role, # type: ignore[arg-type] + name=name, + exact=True, + ).count() + except Exception: # noqa: BLE001 - uncountable is not unique + role_name_count = 0 + return { + "none": False, + "tag": info.get("tag"), + "testid": info.get("testid"), + "testid_count": int(info.get("testid_count") or 0), + "id": info.get("id"), + "id_count": int(info.get("id_count") or 0), + "role": role, + "name": name, + "role_name_count": role_name_count, + "active_option": info.get("active_option") is True, + } + def element_texts( self, selector: str, frame_path: tuple[str, ...] = () ) -> Optional[list[str]]: diff --git a/packages/flow/openadapt_flow/compiler/compile.py b/packages/flow/openadapt_flow/compiler/compile.py index 248839a3b..d8a736bd7 100644 --- a/packages/flow/openadapt_flow/compiler/compile.py +++ b/packages/flow/openadapt_flow/compiler/compile.py @@ -1125,9 +1125,79 @@ def _structural_postconditions(event: dict) -> list[Postcondition]: pcs.append(Postcondition(kind=PostconditionKind.URL_CHANGED)) elif title_before and title_after and title_before != title_after: pcs.append(Postcondition(kind=PostconditionKind.TITLE_CHANGED)) + if not pcs and event.get("kind") in ("key", "hotkey"): + focus = _focus_postcondition( + event.get("focus_before"), event.get("focus_after") + ) + if focus is not None: + pcs.append(focus) return pcs +def _focus_postcondition(before: Any, after: Any) -> Optional[Postcondition]: + """A ``focus_on`` check for a key that moved the focus, or None. + + Only when the element focused after the key has a specific identity that + no other element on the page shared at teach time: a ``data-testid``, + then an ``id``, then a role with a non-empty accessible name. A tag, or + a role with no name, is not specific, and the step keeps no check. The + key must have moved the focus or made an option active; a key that + changed neither proves nothing about it. + """ + + if not isinstance(after, dict) or after.get("none") is not False: + return None + before = before if isinstance(before, dict) else {} + + def ident(o: dict) -> tuple: + return ( + o.get("tag"), + o.get("testid"), + o.get("id"), + o.get("role"), + o.get("name"), + ) + + moved = ident(before) != ident(after) + activated = after.get("active_option") is True and not ( + not moved and before.get("active_option") is True + ) + if not (moved or activated): + return None + common: dict[str, Any] = {"focus_active_option": activated} + testid, ident_id = after.get("testid"), after.get("id") + role, name = after.get("role"), after.get("name") + if isinstance(testid, str) and testid.strip() and after.get("testid_count") == 1: + return Postcondition( + kind=PostconditionKind.FOCUS_ON, + focus_by="testid", + focus_value=testid, + **common, + ) + if isinstance(ident_id, str) and ident_id.strip() and after.get("id_count") == 1: + return Postcondition( + kind=PostconditionKind.FOCUS_ON, + focus_by="id", + focus_value=ident_id, + **common, + ) + if ( + isinstance(role, str) + and role.strip() + and isinstance(name, str) + and name.strip() + and after.get("role_name_count") == 1 + ): + return Postcondition( + kind=PostconditionKind.FOCUS_ON, + focus_by="role_name", + focus_role=role, + focus_name=name, + **common, + ) + return None + + def lint_param_leakage(workflow: Workflow, param_values: tuple[str, ...]) -> list[str]: """Scan a compiled workflow for demo parameter values baked in as literals outside the designated parameter slots. @@ -1164,6 +1234,12 @@ def lint_param_leakage(workflow: Workflow, param_values: tuple[str, ...]) -> lis violations: list[str] = [] for step in workflow.steps: for pc in step.expect: + for focus in (pc.focus_name, pc.focus_value): + if focus and _contains_excluded(focus, values): + violations.append( + f"{step.id}: focus_on postcondition {focus!r} embeds a " + "demo parameter value" + ) if pc.text and _contains_excluded(pc.text, values): violations.append( f"{step.id}: {pc.kind.value} postcondition " diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index 034a30688..158d13789 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -528,6 +528,16 @@ class PostconditionKind(str, Enum): # shown, so a list that has not loaded yet never passes. Unreadable rows # fail the check. NO_DUPLICATE_CANDIDATE = "no_duplicate_candidate" + # A key press that moves the focus and changes nothing visible (a Tab + # onto an icon button, an ArrowDown that highlights an option): after the + # step the focused element has a specific identity, ``focus_by`` (a + # ``testid``, an ``id``, or a ``role_name``: a role and a non-empty + # accessible name), and no other element on the page has it. With + # ``focus_active_option`` an option of the focused control is active + # (``aria-activedescendant``, or ``aria-selected`` in the listbox it + # controls). Unobservable focus, or no specific identity, fails the check + # (:func:`focus_identity_is_specific`). + FOCUS_ON = "focus_on" class Postcondition(BaseModel): @@ -555,6 +565,15 @@ class Postcondition(BaseModel): candidate_rows: Optional[str] = Field(default=None, min_length=1, max_length=256) #: NO_DUPLICATE_CANDIDATE: the frame chain the list is in (outermost first). candidate_frame_path: Optional[tuple[str, ...]] = Field(default=None, max_length=8) + #: FOCUS_ON: how the focused element is identified. + focus_by: Optional[Literal["testid", "id", "role_name"]] = None + #: FOCUS_ON: the ``data-testid`` or ``id`` (``focus_by`` testid / id). + focus_value: Optional[str] = Field(default=None, min_length=1, max_length=256) + #: FOCUS_ON: the ARIA role and accessible name (``focus_by`` role_name). + focus_role: Optional[str] = Field(default=None, min_length=1, max_length=64) + focus_name: Optional[str] = Field(default=None, min_length=1, max_length=256) + #: FOCUS_ON: an option of the focused control must be active. + focus_active_option: bool = False #: NO_DUPLICATE_CANDIDATE: parameters whose value in any row makes it a #: real candidate. candidate_params: Optional[tuple[str, ...]] = Field( @@ -571,12 +590,44 @@ def _serialize_compatible(self, handler: Any) -> dict[str, Any]: data.pop("field_selector", None) if self.param is None: data.pop("param", None) - for name in ("candidate_rows", "candidate_frame_path", "candidate_params"): + for name in ( + "candidate_rows", + "candidate_frame_path", + "candidate_params", + "focus_by", + "focus_value", + "focus_role", + "focus_name", + ): if getattr(self, name) is None: data.pop(name, None) + if not self.focus_active_option: + data.pop("focus_active_option", None) return data +def focus_identity_is_specific(pc: "Postcondition") -> bool: + """Whether a FOCUS_ON postcondition names one element, not a kind of element. + + A ``testid`` or an ``id`` with a value, or a role with a non-empty + accessible name. A tag alone, or a role with no name, names no element: + such a postcondition asserts nothing and never passes. + """ + + if pc.kind is not PostconditionKind.FOCUS_ON: + return False + if pc.focus_by in ("testid", "id"): + return bool(pc.focus_value and pc.focus_value.strip()) + if pc.focus_by == "role_name": + return bool( + pc.focus_role + and pc.focus_role.strip() + and pc.focus_name + and pc.focus_name.strip() + ) + return False + + IdentitySignalKeyValue = Literal[ "subject_name", "record_id", diff --git a/packages/flow/openadapt_flow/policy.py b/packages/flow/openadapt_flow/policy.py index eb772b288..8b54d7f39 100644 --- a/packages/flow/openadapt_flow/policy.py +++ b/packages/flow/openadapt_flow/policy.py @@ -28,7 +28,14 @@ from pydantic import BaseModel, ConfigDict, Field -from openadapt_flow.ir import ActionKind, Step, Workflow +from openadapt_flow.ir import ( + ActionKind, + Postcondition, + PostconditionKind, + Step, + Workflow, + focus_identity_is_specific, +) from openadapt_flow.risk import classify_step_risk, is_local_edit, step_text from openadapt_flow.traversal import iter_workflow_steps from openadapt_flow.verification import declared_effect_is_independent_read @@ -139,7 +146,15 @@ def is_vacuous(step: Step) -> bool: return False if step.action in (ActionKind.TYPE, ActionKind.SELECT_OPTION): return False - return expects_effect(step) and not step.expect + return expects_effect(step) and not any(_asserts(pc) for pc in step.expect) + + +def _asserts(pc: Postcondition) -> bool: + """Whether a postcondition can fail: a FOCUS_ON must name one element.""" + + if pc.kind is PostconditionKind.FOCUS_ON: + return focus_identity_is_specific(pc) + return True def has_screen_postcondition(step: Step) -> bool: diff --git a/packages/flow/openadapt_flow/recorder.py b/packages/flow/openadapt_flow/recorder.py index 9cf74995c..880515feb 100644 --- a/packages/flow/openadapt_flow/recorder.py +++ b/packages/flow/openadapt_flow/recorder.py @@ -661,6 +661,10 @@ def _record(self, event: dict[str, Any], act: Callable[[], None]) -> None: """Capture before frame, act, wait settle, capture after, log event.""" before = self._backend.screenshot() structural_before = self._structural_state() + # Who has the focus before and after a key press, for a key that moves + # the focus and changes nothing visible (the ``focus_on`` + # postcondition). Key events only. + focus_before = self._focus_identity(event) # Structured identity of the clicked target (DOM / a11y text), when # the backend exposes it: captured on the BEFORE frame, before the # action, so the compiler can store it on the anchor and the replayer @@ -757,6 +761,14 @@ def _record(self, event: dict[str, Any], act: Callable[[], None]) -> None: }, } after = self._wait_settled() + if focus_before is not None: + focus_after = self._focus_identity(event) + if focus_after is not None: + event = { + **event, + "focus_before": focus_before, + "focus_after": focus_after, + } opened = self._claim_new_pages(self._i) if opened: event = {**event, "opens": opened} @@ -1046,6 +1058,20 @@ def _identity_census_at(self, x: int, y: int) -> Optional[dict[str, Any]]: return None return census if isinstance(census, dict) else None + def _focus_identity(self, event: dict[str, Any]) -> Optional[dict[str, Any]]: + """The focused element's identity around a key event, if readable.""" + + if event.get("kind") not in ("key", "hotkey"): + return None + reader = getattr(self._backend, "focus_identity", None) + if not callable(reader): + return None + try: + value = reader() + except Exception: + return None + return value if isinstance(value, dict) else None + def _structural_state(self) -> dict[str, Any]: """Structural observations the backend can provide right now. diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index 5aae38dde..bc5dae91e 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -5920,7 +5920,11 @@ def _revalidate_retained_postconditions( condition for condition in conditions if condition.kind - not in {PostconditionKind.LOCAL_EDIT, PostconditionKind.FIELD_VALUE} + not in { + PostconditionKind.LOCAL_EDIT, + PostconditionKind.FIELD_VALUE, + PostconditionKind.FOCUS_ON, + } ] if not conditions: return @@ -18131,6 +18135,45 @@ def _local_edit_held(self, pc: Any, start_state: dict[str, Any]) -> bool: return False return True + def _focus_on_held(self, pc: Any) -> bool: + """The focused element has the recorded identity, and only it does. + + Fail closed: a postcondition with no specific identity, a backend + that can't read the focus, nothing focused, a different element, or + a second element with the same identity all fail. With + ``focus_active_option`` an option of the focused control must also + be active. + """ + + from openadapt_flow.ir import focus_identity_is_specific + + if not focus_identity_is_specific(pc): + return False + reader = getattr(self.backend, "focus_identity", None) + if not callable(reader): + return False + try: + seen = reader() + except Exception: # noqa: BLE001 - unobservable is a failure + return False + if not isinstance(seen, dict) or seen.get("none") is not False: + return False + if pc.focus_by == "testid": + held = ( + seen.get("testid") == pc.focus_value and seen.get("testid_count") == 1 + ) + elif pc.focus_by == "id": + held = seen.get("id") == pc.focus_value and seen.get("id_count") == 1 + else: + held = ( + seen.get("role") == pc.focus_role + and seen.get("name") == pc.focus_name + and seen.get("role_name_count") == 1 + ) + if held and pc.focus_active_option: + held = seen.get("active_option") is True + return bool(held) + @staticmethod def _malformed_date_params(workflow: Workflow, params: dict[str, Any]) -> list[str]: """Why each supplied ``date`` parameter is not a canonical ISO date. @@ -18246,6 +18289,14 @@ def _describe_postcondition(pc: Any) -> str: kind = pc.kind.value if hasattr(pc.kind, "value") else pc.kind if kind == "local_edit": return "local_edit (URL, title, page count and focus unchanged)" + if kind == "focus_on": + who = ( + f"{pc.focus_role} {pc.focus_name!r}" + if pc.focus_by == "role_name" + else f"{pc.focus_by} {pc.focus_value!r}" + ) + option = " with an option active" if pc.focus_active_option else "" + return f"focus_on (the focus is on {who}{option})" if kind == "field_value": return f"field_value (the focused field shows {{{pc.param}}})" if kind == "no_duplicate_candidate": @@ -18299,6 +18350,8 @@ def _postcondition_passes( kind = pc.kind.value if hasattr(pc.kind, "value") else pc.kind if kind == "local_edit": return self._local_edit_held(pc, start_state or {}) + if kind == "focus_on": + return self._focus_on_held(pc) if kind == "field_value": return self._field_value_shown(pc) if kind == "no_duplicate_candidate": diff --git a/packages/flow/tests/test_focus_on_postcondition.py b/packages/flow/tests/test_focus_on_postcondition.py new file mode 100644 index 000000000..5c484ac79 --- /dev/null +++ b/packages/flow/tests/test_focus_on_postcondition.py @@ -0,0 +1,228 @@ +"""The ``focus_on`` postcondition: a key that moves the focus is checked. + +A Tab onto a small icon button, or an ArrowDown that highlights an option, +changes nothing the compiler can see, so the step used to carry no check and +a strict policy refused it as vacuous. ``focus_on`` checks who has the focus +after the key. It counts only when the element has a specific identity (a +testid, an id, or a role with a non-empty accessible name) that no other +element on the page shares; anything weaker is still vacuous. At replay the +focused element must have that identity, and be the only one with it. +""" + +from __future__ import annotations + +import json +from types import SimpleNamespace + +import pytest + +from openadapt_flow.compiler.compile import ( + _focus_postcondition, + _structural_postconditions, +) +from openadapt_flow.ir import ( + ActionKind, + Postcondition, + PostconditionKind, + Step, + focus_identity_is_specific, +) +from openadapt_flow.policy import is_vacuous +from openadapt_flow.runtime.replayer import Replayer + + +def _focus(**over): + seen = { + "none": False, + "tag": "button", + "testid": None, + "testid_count": 0, + "id": None, + "id_count": 0, + "role": "button", + "name": "Add Telecom", + "role_name_count": 1, + "active_option": False, + } + seen.update(over) + return seen + + +BEFORE = _focus(tag="input", role="textbox", name="Value") + + +# -- what the compiler mines ------------------------------------------------- + + +def test_a_unique_testid_wins(): + pc = _focus_postcondition(BEFORE, _focus(testid="add", testid_count=1)) + assert pc.focus_by == "testid" and pc.focus_value == "add" + + +def test_a_unique_role_and_name(): + pc = _focus_postcondition(BEFORE, _focus()) + assert (pc.focus_by, pc.focus_role, pc.focus_name) == ( + "role_name", + "button", + "Add Telecom", + ) + assert focus_identity_is_specific(pc) + + +@pytest.mark.parametrize( + "after", + [ + _focus(role="button", name=None), + _focus(role=None, name=None), + _focus(role_name_count=2), + _focus(testid="remove", testid_count=3, role_name_count=2), + ], + ids=["role without a name", "tag only", "shared role and name", "shared testid"], +) +def test_no_specific_identity_means_no_check(after): + assert _focus_postcondition(BEFORE, after) is None + + +def test_a_key_that_moved_nothing_proves_nothing(): + assert _focus_postcondition(_focus(), _focus()) is None + assert _focus_postcondition(_focus(), {"none": True}) is None + + +def test_an_option_made_active_on_the_same_control(): + combo = _focus(tag="input", role="combobox", name="Gender") + pc = _focus_postcondition(combo, {**combo, "active_option": True}) + assert pc.focus_by == "role_name" and pc.focus_active_option is True + + +def test_only_key_steps_without_visual_checks_get_it(): + event = { + "kind": "key", + "key": "Tab", + "focus_before": BEFORE, + "focus_after": _focus(), + } + assert [p.kind for p in _structural_postconditions(event)] == [ + PostconditionKind.FOCUS_ON + ] + assert _structural_postconditions({**event, "kind": "click"}) == [] + + +# -- the policy --------------------------------------------------------------- + + +def _key_step(expect): + return Step( + id="k", intent="press Tab", action=ActionKind.KEY, key="Tab", expect=expect + ) + + +def test_a_specific_focus_check_is_not_vacuous(): + pc = _focus_postcondition(BEFORE, _focus()) + assert is_vacuous(_key_step([])) is True + assert is_vacuous(_key_step([pc])) is False + + +def test_a_focus_check_without_a_specific_identity_stays_vacuous(): + for pc in ( + Postcondition( + kind=PostconditionKind.FOCUS_ON, focus_by="role_name", focus_role="button" + ), + Postcondition(kind=PostconditionKind.FOCUS_ON), + Postcondition(kind=PostconditionKind.FOCUS_ON, focus_by="testid"), + ): + assert not focus_identity_is_specific(pc) + assert is_vacuous(_key_step([pc])) is True + + +def test_existing_postconditions_serialize_as_before(): + dumped = Postcondition( + kind=PostconditionKind.TEXT_PRESENT, text="Saved" + ).model_dump() + assert not any(k.startswith("focus_") for k in dumped) + + +# -- the check at replay, in a real browser -------------------------------------- + +PAGE = ( + "data:text/html," + "" + "" + "" + "
Female
" +) + + +@pytest.fixture +def backend(): + pytest.importorskip("playwright") + from openadapt_flow.backends.playwright_backend import PlaywrightBackend + + try: + backend, close = PlaywrightBackend.launch(PAGE) + except Exception as exc: # noqa: BLE001 - no browser on this machine + pytest.skip(f"chromium unavailable: {exc}") + yield backend + close() + + +def _held(backend, pc): + return Replayer._focus_on_held(SimpleNamespace(backend=backend), pc) + + +ADD_TELECOM = Postcondition( + kind=PostconditionKind.FOCUS_ON, + focus_by="role_name", + focus_role="button", + focus_name="Add Telecom", +) + + +def test_it_passes_on_the_recorded_focus(backend): + backend.page.get_by_role("button", name="Add Telecom").focus() + assert _held(backend, ADD_TELECOM) is True + + +def test_it_halts_on_another_button_with_the_same_role(backend): + backend.page.get_by_role("button", name="Add Address").focus() + assert _held(backend, ADD_TELECOM) is False + + +def test_it_halts_when_the_identity_is_not_unique(backend): + backend.page.get_by_role("button", name="Remove").first.focus() + remove = ADD_TELECOM.model_copy(update={"focus_name": "Remove"}) + assert _held(backend, remove) is False + + +def test_it_fails_without_a_specific_identity(backend): + backend.page.get_by_role("button", name="Add Telecom").focus() + vague = Postcondition( + kind=PostconditionKind.FOCUS_ON, focus_by="role_name", focus_role="button" + ) + assert _held(backend, vague) is False + + +def test_an_active_option_is_checked(backend): + combo = Postcondition( + kind=PostconditionKind.FOCUS_ON, + focus_by="role_name", + focus_role="combobox", + focus_name="Gender", + focus_active_option=True, + ) + page = backend.page + page.get_by_role("combobox", name="Gender").focus() + assert _held(backend, combo) is False + page.evaluate("document.getElementById('o1').setAttribute('aria-selected', 'true')") + assert _held(backend, combo) is True + + +def test_the_recorder_keeps_the_focus_around_a_key(backend, tmp_path): + from openadapt_flow.recorder import Recorder + + backend.page.get_by_role("button", name="Add Telecom").focus() + recorder = Recorder(backend, tmp_path / "rec", settle_timeout_s=2.0) + recorder.press("Tab") + recorder.finish() + event = json.loads((tmp_path / "rec" / "events.jsonl").read_text().splitlines()[-1]) + assert event["focus_before"]["name"] == "Add Telecom" + assert event["focus_after"]["name"] == "Add Address" From 2447f94f1b7a9670ec9bb263d9b5538fa7a69d4b Mon Sep 17 00:00:00 2001 From: abrichr Date: Wed, 28 Oct 2026 14:19:48 -0400 Subject: [PATCH 2/3] focus_on: record the frame, refuse shadow roots and generated ids, scope active options Review fixes. The focused element's frame chain is recorded and must match at replay, so a button with the same testid in an iframe no longer passes. Focus inside a shadow root (or on a custom element that may hide a closed one) is unreadable: no check is mined and any check fails there. A framework-generated id (React useId, Radix, Headless UI, MUI, Ember and similar) is skipped for role and name. An option is active only as a role=option inside the listbox the control names by aria-controls; aria-owns is no longer read. Each hole has a real-Chromium test. Co-Authored-By: Claude Opus 5.5 --- .../backends/playwright_backend.py | 43 +++++-- .../flow/openadapt_flow/compiler/compile.py | 30 ++++- packages/flow/openadapt_flow/ir.py | 4 + .../flow/openadapt_flow/runtime/replayer.py | 2 + .../flow/tests/test_focus_on_postcondition.py | 113 ++++++++++++++++++ 5 files changed, 178 insertions(+), 14 deletions(-) diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index 91f5b6dd9..09201ed57 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -616,22 +616,28 @@ class _FramePoint: _FOCUS_IDENTITY_JS = r"""(el) => { const doc = el.ownerDocument; if (el === doc.body || el === doc.documentElement) return {none: true}; + // activeElement names a shadow host when the focus is inside its shadow + // root. An open root is not followed, and a custom element may hide a + // closed one: either way the focused control can't be told apart. + if (el.shadowRoot || el.localName.includes('-')) return {unreadable: true}; const count = (sel) => { try { return doc.querySelectorAll(sel).length; } catch (e) { return 0; } }; const testid = el.getAttribute('data-testid') || null; const id = el.id || null; + // An option is active only inside the listbox the control names by + // aria-controls: aria-activedescendant points at one of its role=option + // elements, or one of them is aria-selected. let active = false; - const activeId = el.getAttribute('aria-activedescendant'); - if (activeId && doc.getElementById(activeId)) active = true; - if (!active) { - for (const attr of ['aria-controls', 'aria-owns']) { - const ref = el.getAttribute(attr); - const box = ref ? doc.getElementById(ref) : null; - if (box && box.querySelector('[role="option"][aria-selected="true"]')) { - active = true; - break; - } + const boxId = el.getAttribute('aria-controls'); + const box = boxId ? doc.getElementById(boxId) : null; + if (box && box.getAttribute('role') === 'listbox') { + const activeId = el.getAttribute('aria-activedescendant'); + const option = activeId ? doc.getElementById(activeId) : null; + if (option) { + active = option.getAttribute('role') === 'option' && box.contains(option); + } else { + active = !!box.querySelector('[role="option"][aria-selected="true"]'); } } let top = false; @@ -2220,8 +2226,9 @@ def focus_identity(self) -> Optional[dict[str, Any]]: nothing has the focus; None when it can't be read. """ - info = self._focused_element_call(_FOCUS_IDENTITY_JS) - if not isinstance(info, dict): + frame_path: list[str] = [] + info = self._focused_element_call(_FOCUS_IDENTITY_JS, frame_path_out=frame_path) + if not isinstance(info, dict) or info.get("unreadable"): return None if info.get("none"): return {"none": True} @@ -2258,6 +2265,7 @@ def focus_identity(self) -> Optional[dict[str, Any]]: "name": name, "role_name_count": role_name_count, "active_option": info.get("active_option") is True, + "frame_path": list(frame_path), } def element_texts( @@ -2331,11 +2339,18 @@ def _register_focused_field(self) -> None: ) def _focused_element_call( - self, js: str, *, arg: Any = None, with_arg: bool = False + self, + js: str, + *, + arg: Any = None, + with_arg: bool = False, + frame_path_out: Optional[list[str]] = None, ) -> Any: """Evaluate ``js`` on the exactly bound focused element (see above). Returns the raw result, or None on any frame mismatch or failure. + ``frame_path_out``, when given, receives the frame chain the focused + element is in (outermost first; empty in the top document). """ scope: Any = self.page @@ -2356,6 +2371,8 @@ def _focused_element_call( scope, tuple(frame_path), box ): return None + if frame_path_out is not None: + frame_path_out[:] = frame_path if with_arg: return focused.evaluate(js, arg) return focused.evaluate(js) diff --git a/packages/flow/openadapt_flow/compiler/compile.py b/packages/flow/openadapt_flow/compiler/compile.py index 23cd9512c..0786350bb 100644 --- a/packages/flow/openadapt_flow/compiler/compile.py +++ b/packages/flow/openadapt_flow/compiler/compile.py @@ -1134,6 +1134,22 @@ def _structural_postconditions(event: dict) -> list[Postcondition]: return pcs +#: Ids a UI framework generates per render: React's useId (``:r2:``, +#: ``«r2»``), Radix, Headless UI, MUI, Ember, react-select, downshift, Reach, +#: Mantine, and bare UUIDs or long numeric suffixes. +_GENERATED_ID = re.compile( + r"^(?::r[0-9a-z]+:|«r[0-9a-z]+»|:[0-9a-z]+:)$" + r"|^(?:radix-|headlessui-|mui-|ember|react-select-|downshift-|reach-|mantine-)" + r"|^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$" + r"|\d{4,}$", + re.IGNORECASE, +) + + +def _generated_id(value: str) -> bool: + return bool(_GENERATED_ID.search(value)) + + def _focus_postcondition(before: Any, after: Any) -> Optional[Postcondition]: """A ``focus_on`` check for a key that moved the focus, or None. @@ -1164,8 +1180,20 @@ def ident(o: dict) -> tuple: ) if not (moved or activated): return None - common: dict[str, Any] = {"focus_active_option": activated} + frame_path = after.get("frame_path") + if not isinstance(frame_path, list) or not all( + isinstance(f, str) for f in frame_path + ): + return None + common: dict[str, Any] = { + "focus_active_option": activated, + "focus_frame_path": tuple(frame_path), + } testid, ident_id = after.get("testid"), after.get("id") + if isinstance(ident_id, str) and _generated_id(ident_id): + # A framework-generated id (React useId, Radix, MUI ...) shifts from + # run to run, so it names no element; fall back to role and name. + ident_id = None role, name = after.get("role"), after.get("name") if isinstance(testid, str) and testid.strip() and after.get("testid_count") == 1: return Postcondition( diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index 65632e5cf..f4f27403f 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -599,6 +599,9 @@ class Postcondition(BaseModel): focus_name: Optional[str] = Field(default=None, min_length=1, max_length=256) #: FOCUS_ON: an option of the focused control must be active. focus_active_option: bool = False + #: FOCUS_ON: the frame chain the focused element is in (outermost first; + #: empty in the top document). A testid or id is unique within its frame. + focus_frame_path: Optional[tuple[str, ...]] = Field(default=None, max_length=8) #: NO_DUPLICATE_CANDIDATE: parameters whose value in any row makes it a #: real candidate. candidate_params: Optional[tuple[str, ...]] = Field( @@ -623,6 +626,7 @@ def _serialize_compatible(self, handler: Any) -> dict[str, Any]: "focus_value", "focus_role", "focus_name", + "focus_frame_path", ): if getattr(self, name) is None: data.pop(name, None) diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index 288826469..b8da04a25 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -18238,6 +18238,8 @@ def _focus_on_held(self, pc: Any) -> bool: return False if not isinstance(seen, dict) or seen.get("none") is not False: return False + if tuple(seen.get("frame_path") or ()) != tuple(pc.focus_frame_path or ()): + return False if pc.focus_by == "testid": held = ( seen.get("testid") == pc.focus_value and seen.get("testid_count") == 1 diff --git a/packages/flow/tests/test_focus_on_postcondition.py b/packages/flow/tests/test_focus_on_postcondition.py index 5c484ac79..76a365eb1 100644 --- a/packages/flow/tests/test_focus_on_postcondition.py +++ b/packages/flow/tests/test_focus_on_postcondition.py @@ -43,6 +43,7 @@ def _focus(**over): "name": "Add Telecom", "role_name_count": 1, "active_option": False, + "frame_path": [], } seen.update(over) return seen @@ -226,3 +227,115 @@ def test_the_recorder_keeps_the_focus_around_a_key(backend, tmp_path): event = json.loads((tmp_path / "rec" / "events.jsonl").read_text().splitlines()[-1]) assert event["focus_before"]["name"] == "Add Telecom" assert event["focus_after"]["name"] == "Add Address" + + +# -- the review's holes, each in a real browser ------------------------------------------ + + +def _launch(html): + pytest.importorskip("playwright") + from openadapt_flow.backends.playwright_backend import PlaywrightBackend + + try: + return PlaywrightBackend.launch("data:text/html," + html) + except Exception as exc: # noqa: BLE001 - no browser on this machine + pytest.skip(f"chromium unavailable: {exc}") + + +def _mined(backend, before): + return _focus_postcondition(before, backend.focus_identity()) + + +def test_an_iframe_button_with_the_same_testid_fails(): + backend, close = _launch( + "" + "" + ) + try: + page = backend.page + page.wait_for_timeout(300) + page.locator("button[data-testid=save]").focus() + pc = _mined(backend, BEFORE) + assert pc.focus_by == "testid" and pc.focus_frame_path == () + assert _held(backend, pc) is True + page.frame_locator("iframe").locator("button").focus() + assert backend.focus_identity()["frame_path"] != [] + assert _held(backend, pc) is False + finally: + close() + + +def test_focus_inside_a_shadow_root_is_unreadable(): + backend, close = _launch( + "
" + ) + try: + page = backend.page + page.locator("#host button", has_text="Two").focus() + assert backend.focus_identity() is None + # Nothing to mine, and any check fails there. + assert _focus_postcondition(BEFORE, backend.focus_identity()) is None + assert _held(backend, ADD_TELECOM) is False + finally: + close() + + +def test_a_generated_id_falls_back_to_role_and_name(): + backend, close = _launch( + "" + ) + try: + page = backend.page + page.get_by_role("button", name="Save").focus() + pc = _mined(backend, BEFORE) + assert (pc.focus_by, pc.focus_name) == ("role_name", "Save") + # The ids shift on the next render: the check still follows the name. + page.evaluate( + "document.querySelectorAll('button').forEach((b, i) => b.id = ':r' + (i + 7) + ':')" + ) + assert _held(backend, pc) is True + page.get_by_role("button", name="Cancel").focus() + assert _held(backend, pc) is False + finally: + close() + + +@pytest.mark.parametrize( + "markup, active", + [ + ("
A
", True), + ("
A
", False), + ("
A
", False), + ("
A
", False), + ], + ids=[ + "option in the listbox", + "option outside it", + "not an option", + "not a listbox", + ], +) +def test_an_active_option_must_be_in_the_controlled_listbox(markup, active): + backend, close = _launch( + "" + markup + ) + try: + backend.page.get_by_role("combobox", name="Gender").focus() + assert backend.focus_identity()["active_option"] is active + finally: + close() + + +def test_aria_owns_alone_is_not_an_active_option(): + backend, close = _launch( + "" + "
A
" + ) + try: + backend.page.get_by_role("combobox", name="Gender").focus() + assert backend.focus_identity()["active_option"] is False + finally: + close() From 9c111ff4075c90420bf8447c0947f4e3c19e5ec4 Mon Sep 17 00:00:00 2001 From: abrichr Date: Wed, 28 Oct 2026 14:31:17 -0400 Subject: [PATCH 3/3] focus_on: bind an id or testid to the element's role and name; narrower shadow rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 of the review. A testid or id check now also records the element's role and accessible name when it had them, and replay requires both, so an id that moves to another element can't pass whatever framework generated it. The generated-id patterns stay as a hint and match anywhere in the id (React's :r…:, «r…» and 19.2's _r_…_; react-aria, Angular Material and CDK, rc-select). Focus is unreadable only on an element with a shadow root or a registered custom element, so unregistered framework components keep their checks. Co-Authored-By: Claude Opus 5.5 --- .../backends/playwright_backend.py | 10 ++- .../flow/openadapt_flow/compiler/compile.py | 19 +++++- packages/flow/openadapt_flow/ir.py | 4 +- .../flow/openadapt_flow/runtime/replayer.py | 13 ++-- .../flow/tests/test_focus_on_postcondition.py | 66 +++++++++++++++++++ 5 files changed, 102 insertions(+), 10 deletions(-) diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index 09201ed57..f4fc6fb9c 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -617,9 +617,13 @@ class _FramePoint: const doc = el.ownerDocument; if (el === doc.body || el === doc.documentElement) return {none: true}; // activeElement names a shadow host when the focus is inside its shadow - // root. An open root is not followed, and a custom element may hide a - // closed one: either way the focused control can't be told apart. - if (el.shadowRoot || el.localName.includes('-')) return {unreadable: true}; + // root. An open root is not followed, and a registered custom element may + // hide a closed one: either way the focused control can't be told apart. + // An unregistered element with a dash (an Angular or other framework + // component without a shadow root) is an ordinary element. + if (el.shadowRoot) return {unreadable: true}; + if (el.localName.includes('-') && window.customElements + && doc.defaultView.customElements.get(el.localName)) return {unreadable: true}; const count = (sel) => { try { return doc.querySelectorAll(sel).length; } catch (e) { return 0; } }; diff --git a/packages/flow/openadapt_flow/compiler/compile.py b/packages/flow/openadapt_flow/compiler/compile.py index 0786350bb..9788f05ea 100644 --- a/packages/flow/openadapt_flow/compiler/compile.py +++ b/packages/flow/openadapt_flow/compiler/compile.py @@ -1138,8 +1138,11 @@ def _structural_postconditions(event: dict) -> list[Postcondition]: #: ``«r2»``), Radix, Headless UI, MUI, Ember, react-select, downshift, Reach, #: Mantine, and bare UUIDs or long numeric suffixes. _GENERATED_ID = re.compile( - r"^(?::r[0-9a-z]+:|«r[0-9a-z]+»|:[0-9a-z]+:)$" - r"|^(?:radix-|headlessui-|mui-|ember|react-select-|downshift-|reach-|mantine-)" + # React useId, anywhere in the id: ``:r2:``, ``«r2»``, and React 19.2+'s + # ``_r_2_`` / ``_R_2_`` (react-aria's ``react-aria-:r1:`` contains one). + r":r[0-9a-z]+:|«r[0-9a-z]+»|_r_[0-9a-z]+_" + r"|^(?:radix-|headlessui-|mui-|ember|react-select-|downshift-|reach-|mantine-" + r"|mat-|cdk-|rc_select_)" r"|^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$" r"|\d{4,}$", re.IGNORECASE, @@ -1195,11 +1198,22 @@ def ident(o: dict) -> tuple: # run to run, so it names no element; fall back to role and name. ident_id = None role, name = after.get("role"), after.get("name") + named = ( + isinstance(role, str) + and role.strip() + and isinstance(name, str) + and name.strip() + ) + # An id or testid check also binds the element's role and name when it had + # them, so an id that moved to another element can't pass: the generated-id + # patterns are only a hint, and no list keeps up with every framework. + pair: dict[str, Any] = {"focus_role": role, "focus_name": name} if named else {} if isinstance(testid, str) and testid.strip() and after.get("testid_count") == 1: return Postcondition( kind=PostconditionKind.FOCUS_ON, focus_by="testid", focus_value=testid, + **pair, **common, ) if isinstance(ident_id, str) and ident_id.strip() and after.get("id_count") == 1: @@ -1207,6 +1221,7 @@ def ident(o: dict) -> tuple: kind=PostconditionKind.FOCUS_ON, focus_by="id", focus_value=ident_id, + **pair, **common, ) if ( diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index f4f27403f..acecc5108 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -594,7 +594,9 @@ class Postcondition(BaseModel): focus_by: Optional[Literal["testid", "id", "role_name"]] = None #: FOCUS_ON: the ``data-testid`` or ``id`` (``focus_by`` testid / id). focus_value: Optional[str] = Field(default=None, min_length=1, max_length=256) - #: FOCUS_ON: the ARIA role and accessible name (``focus_by`` role_name). + #: FOCUS_ON: the ARIA role and accessible name (``focus_by`` role_name; + #: with a testid or id too, when the element had them, and then both must + #: hold at replay). focus_role: Optional[str] = Field(default=None, min_length=1, max_length=64) focus_name: Optional[str] = Field(default=None, min_length=1, max_length=256) #: FOCUS_ON: an option of the focused control must be active. diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index b8da04a25..45c4fe246 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -18240,12 +18240,17 @@ def _focus_on_held(self, pc: Any) -> bool: return False if tuple(seen.get("frame_path") or ()) != tuple(pc.focus_frame_path or ()): return False - if pc.focus_by == "testid": + if pc.focus_by in ("testid", "id"): held = ( - seen.get("testid") == pc.focus_value and seen.get("testid_count") == 1 + seen.get(pc.focus_by) == pc.focus_value + and seen.get(f"{pc.focus_by}_count") == 1 ) - elif pc.focus_by == "id": - held = seen.get("id") == pc.focus_value and seen.get("id_count") == 1 + if held and pc.focus_role and pc.focus_name: + # Recorded with a role and name too: both must still hold. + held = ( + seen.get("role") == pc.focus_role + and seen.get("name") == pc.focus_name + ) else: held = ( seen.get("role") == pc.focus_role diff --git a/packages/flow/tests/test_focus_on_postcondition.py b/packages/flow/tests/test_focus_on_postcondition.py index 76a365eb1..1635b1ffb 100644 --- a/packages/flow/tests/test_focus_on_postcondition.py +++ b/packages/flow/tests/test_focus_on_postcondition.py @@ -339,3 +339,69 @@ def test_aria_owns_alone_is_not_an_active_option(): assert backend.focus_identity()["active_option"] is False finally: close() + + +# -- round 2: ids that move, and framework components ------------------------------------ + + +def test_react_19_ids_that_shift_fall_back_to_role_and_name(): + backend, close = _launch( + "" + ) + try: + page = backend.page + page.get_by_role("button", name="Save").focus() + pc = _mined(backend, BEFORE) + assert (pc.focus_by, pc.focus_name) == ("role_name", "Save") + page.evaluate( + "document.querySelectorAll('button').forEach((b, i) => b.id = '_r_' + (i + 5) + '_')" + ) + assert _held(backend, pc) is True + page.get_by_role("button", name="Cancel").focus() + assert _held(backend, pc) is False + finally: + close() + + +def test_a_react_aria_id_is_not_identity(): + pc = _focus_postcondition(BEFORE, _focus(id="react-aria-:r1:", id_count=1)) + assert pc.focus_by == "role_name" + + +def test_an_id_that_moves_to_another_button_fails_on_its_name(): + backend, close = _launch( + "" + ) + try: + page = backend.page + page.get_by_role("button", name="Save").focus() + pc = _mined(backend, BEFORE) + assert (pc.focus_by, pc.focus_value, pc.focus_name) == ("id", "btn-7", "Save") + # The ids swap: the wrong button now carries the recorded id. + page.evaluate( + "const [a, b] = document.querySelectorAll('button'); a.id = 'btn-8'; b.id = 'btn-7';" + ) + page.get_by_role("button", name="Cancel").focus() + assert _held(backend, pc) is False + finally: + close() + + +def test_an_unregistered_component_keeps_its_check_and_a_registered_one_does_not(): + backend, close = _launch( + "" + "" + "" + ) + try: + page = backend.page + page.locator("mat-select").focus() + seen = backend.focus_identity() + assert seen is not None and (seen["role"], seen["name"]) == ( + "combobox", + "Gender", + ) + page.locator("x-picker").focus() + assert backend.focus_identity() is None + finally: + close()