From 732fdcb6ea6b3c24f97e2514c6921d70957ac608 Mon Sep 17 00:00:00 2001 From: abrichr Date: Wed, 4 Nov 2026 16:23:41 -0500 Subject: [PATCH 01/10] feat(flow): interruption classes cleared only when provably read-only A run clears a known interruption (session warning, banner, tour) itself only when it matches a class by text and control, the dismissal is Escape or a closed non-committing label, no write watcher sees a write (browser write fence, or the system-of-record read-back on any backend), and the step's normal identity check passes afterwards. Classes are per-app data (runtime.interruptions_path) or learned from a halt. Each dismissal is in the run report. --- CAPABILITIES.md | 1 + llms-full.txt | 1 + packages/flow/CHANGELOG.md | 4 + packages/flow/docs/INTERRUPTION_RECOVERY.md | 116 ++++++ .../backends/playwright_backend.py | 60 ++- packages/flow/openadapt_flow/deployment.py | 12 + packages/flow/openadapt_flow/interruptions.py | 357 ++++++++++++++++ packages/flow/openadapt_flow/ir.py | 44 ++ .../openadapt_flow/runtime/effect_watch.py | 130 ++++++ .../flow/openadapt_flow/runtime/replayer.py | 165 +++++--- packages/flow/tests/test_interruptions.py | 385 ++++++++++++++++++ .../flow/tests/test_interruptions_browser.py | 195 +++++++++ packages/flow/tests/test_write_fence.py | 131 ++++++ 13 files changed, 1538 insertions(+), 63 deletions(-) create mode 100644 packages/flow/docs/INTERRUPTION_RECOVERY.md create mode 100644 packages/flow/openadapt_flow/interruptions.py create mode 100644 packages/flow/openadapt_flow/runtime/effect_watch.py create mode 100644 packages/flow/tests/test_interruptions.py create mode 100644 packages/flow/tests/test_interruptions_browser.py create mode 100644 packages/flow/tests/test_write_fence.py diff --git a/CAPABILITIES.md b/CAPABILITIES.md index 6068a996cf..3949f7a4fc 100644 --- a/CAPABILITIES.md +++ b/CAPABILITIES.md @@ -105,6 +105,7 @@ monorepo and installable from source. | Checks a save on desktop, RDP and Citrix apps with no API at all, by looking the record up in the app's own screens in a separate session. | [EFFECT_KIT.md](packages/flow/docs/EFFECT_KIT.md#separate-session-desktop-read-back-desktop_readback) | | Keeps pixel-path runs working across display scaling from 100% to 175% and font smoothing on or off, and stops rather than saves when the record changed. | [benchmark/pixel_scale](packages/flow/benchmark/pixel_scale/README.md) | | Lists every change made before a stop and rolls it back with compensations a person approved once, then proves it by reading the whole database: equal to the read from before the run is `ROLLED_BACK`, anything else names the exact rows. | `undo`, `rollback`, [ROLLBACK.md](packages/flow/docs/ROLLBACK.md), [AFTER_A_STOP.md](packages/flow/docs/AFTER_A_STOP.md) | +| Clears a known interruption (a session warning, a banner, a tour) by itself, only when the dismissal is provably read-only: matched by its text and control, a closed set of non-committing labels, no write seen by the browser write fence or the system-of-record read-back, and the step's normal identity check afterwards. Anything else stops the run. Classes are per-app data. | [INTERRUPTION_RECOVERY.md](packages/flow/docs/INTERRUPTION_RECOVERY.md) | | Repairs itself when the app changes: governed repair, agent-demonstrated fixes, fleet patches, and self-repair probed up to the save. | [REPAIR_LIFECYCLE.md](packages/flow/docs/REPAIR_LIFECYCLE.md) | | Repairs a change that needs new steps, such as a new required dialog or a control moved into a menu: up to three new steps, each with its own record check, probed up to the save before one person approves. | [SELF_REPAIR.md](packages/flow/docs/SELF_REPAIR.md), [#1325](https://github.com/OpenAdaptAI/OpenAdapt/pull/1325) | | Compiles a computer-use agent's verified run, with no human demonstration needed. | [JIT.md](packages/flow/docs/JIT.md) | diff --git a/llms-full.txt b/llms-full.txt index 23cc32870e..20e90d0096 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -508,6 +508,7 @@ monorepo and installable from source. | Checks a save on desktop, RDP and Citrix apps with no API at all, by looking the record up in the app's own screens in a separate session. | [EFFECT_KIT.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/EFFECT_KIT.md#separate-session-desktop-read-back-desktop_readback) | | Keeps pixel-path runs working across display scaling from 100% to 175% and font smoothing on or off, and stops rather than saves when the record changed. | [benchmark/pixel_scale](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/benchmark/pixel_scale/README.md) | | Lists every change made before a stop and rolls it back with compensations a person approved once, then proves it by reading the whole database: equal to the read from before the run is `ROLLED_BACK`, anything else names the exact rows. | `undo`, `rollback`, [ROLLBACK.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/ROLLBACK.md), [AFTER_A_STOP.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/AFTER_A_STOP.md) | +| Clears a known interruption (a session warning, a banner, a tour) by itself, only when the dismissal is provably read-only: matched by its text and control, a closed set of non-committing labels, no write seen by the browser write fence or the system-of-record read-back, and the step's normal identity check afterwards. Anything else stops the run. Classes are per-app data. | [INTERRUPTION_RECOVERY.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/INTERRUPTION_RECOVERY.md) | | Repairs itself when the app changes: governed repair, agent-demonstrated fixes, fleet patches, and self-repair probed up to the save. | [REPAIR_LIFECYCLE.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/REPAIR_LIFECYCLE.md) | | Repairs a change that needs new steps, such as a new required dialog or a control moved into a menu: up to three new steps, each with its own record check, probed up to the save before one person approves. | [SELF_REPAIR.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/SELF_REPAIR.md), [#1325](https://github.com/OpenAdaptAI/OpenAdapt/pull/1325) | | Compiles a computer-use agent's verified run, with no human demonstration needed. | [JIT.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/JIT.md) | diff --git a/packages/flow/CHANGELOG.md b/packages/flow/CHANGELOG.md index ffcdd9ddb4..ce35264eed 100644 --- a/packages/flow/CHANGELOG.md +++ b/packages/flow/CHANGELOG.md @@ -17,6 +17,10 @@ ## Unreleased +### Features + +- Interruption classes: a run clears a known interruption, such as a session warning, a banner or a tour, by itself, but only when the dismissal is provably read-only. The class must match by its text and control, the control's label must be in a closed non-committing set, and no write watcher may see a write. On the web the watcher is the browser write fence; on any backend, including desktop and RDP, it is the system-of-record read-back. Afterwards the step's normal identity check must pass. Classes are per-app data (`runtime.interruptions_path`) or learned from a halt (`learn_interruption`). Each dismissal is in the run report. See `docs/INTERRUPTION_RECOVERY.md`. + ### Fixes - The OpenEMR contracts (`benchmark/lifecycle_proof/verify.yaml`, `benchmark/openemr_local/verify.yaml`) now declare how the New Patient form stores what was typed: street and city in capitals, names with each word capitalized. Without it the exact record check refuted 19 of 20 correct saves on the stand-in suites (#1811). A contract value's `stored` form gains `capitalize_words` (OpenEMR's "C" fields); the set stays closed and the check stays exact. diff --git a/packages/flow/docs/INTERRUPTION_RECOVERY.md b/packages/flow/docs/INTERRUPTION_RECOVERY.md new file mode 100644 index 0000000000..639c7af6b3 --- /dev/null +++ b/packages/flow/docs/INTERRUPTION_RECOVERY.md @@ -0,0 +1,116 @@ +# Interruption recovery + +Production apps interrupt a run: a session warning, an "Update available" +banner, a feature tour, a notification, a modal from background polling. +Without help, each one stops a compiled run for a person. An *interruption +class* lets the run clear a known interruption itself, but only when the +dismissal is provably read-only. Anything else stops the run as before. + +## The policy + +A run clears an interruption only when all four of these hold: + +1. **It matches a class by identity.** Every text the class lists is on + screen, and the dismiss control is found by its role, accessible name, + selector, automation ID or window. Position is never used. +2. **The dismissal does not commit anything.** The dismissal is Escape, or a + control whose label is in a closed set: close, dismiss, not now, maybe + later, skip, got it, continue session, stay signed in, and similar + (`NON_COMMITTING_LABEL` in `openadapt_flow/interruptions.py`). "OK", + "Continue", "Yes", "Accept" and any write word are outside the set, so a + class that uses one is refused when it loads. +3. **No watcher sees a write.** The dismissal runs under every write watcher + the run has (`openadapt_flow/runtime/effect_watch.py`): + - in a browser, the write fence. It is the same hold that guards a + checked save, with nothing allowed through. Every write request, new + WebSocket, worker and socket message is stopped before it leaves the + page, and each is reported. + - on any backend, the system-of-record read-back that the run's writes + are checked with. It is read before and after the dismissal; any + difference is a write. This is the watcher for desktop and RDP or + Citrix runs, which have no network to fence. + + A run with no watcher, a fence that can't go up, or a read-back that + can't be read does not dismiss. A dismissal that a watcher saw write + stops the run as a safety halt. +4. **The expected screen comes back.** After one dismissal the screen must + settle, the class's text must be gone, and the step then resolves its + target through its normal identity check. If the screen isn't the one + the step expects, the run stops there. + +A run makes at most one dismissal per class per step, and never retries one. + +## The record + +Each dismissal is recorded in the step's `interstitial_actions` in the run +report (`report.json`, which the run seal signs). The record includes the +class, the control's label, the frames before and after, the watchers that +were up (`effect_watchers`), and any write they saw (`write_attempts`). + +## Declaring classes + +Classes are data. The engine has no rule for any particular app. Put a +per-app file next to the deployment and point `runtime.interruptions_path` +at it: + +```yaml +interruptions: + - name: session expiry warning + texts: ["Your session will expire"] + dismiss: {control: {role: button, name: Stay signed in}} + - name: update banner + texts: ["Update available"] + dismiss: {control: {role: button, name: Close}} + - name: product tour + texts: ["Take a quick tour"] + dismiss: {key: Escape} +``` + +`texts` must all be present. `dismiss.control` takes `role`, `name`, +`selector`, `automation_id` and `window`. `gone` names the text whose +absence clears the interruption; it defaults to the first text. If any +entry fails the policy, the run refuses to start. + +## Learning a class from a halt + +When a person clears a new interruption during a halt, +`openadapt_flow.interruptions.learn_interruption` can learn it as a class. +It builds the class from the interruption's texts and the control the +person chose, and checks that class against rules 1 and 2. It then clicks +the control on the live screen under the run's watchers. It keeps the class +only when no watcher saw a write and the text is gone. If the dismissal +writes, the class is not learned. `save_interruption_class` adds the class +to the app's file. + +## Cases + +These cases were registered before the runs. The real-browser cases are in +`tests/test_interruptions_browser.py`, which uses headless Chromium, the +real replayer and OCR, and a local synthetic app that saves with +`POST /save`. The fake-backend cases are in `tests/test_interruptions.py`. + +| Case | Expected | Result | +|---|---|---| +| Session warning, "Stay signed in" (sends only a GET) | cleared, save lands | passes | +| "Update available" banner, "Close" | cleared, save lands | passes | +| Session warning whose "Stay signed in" POSTs /logout | safety halt; the logout is stopped in the browser, nothing saved | passes | +| Unknown dialog | halt, no click | passes | +| Class whose control reads "OK", "Continue", "Accept", "Confirm" | refused at load | passes | +| Class found only by position | refused at load | passes | +| Desktop run: Escape, read-back unchanged | cleared (`record` watcher) | passes | +| Read-back changes during the dismissal | safety halt | passes | +| Run with no watcher; fence refused | no dismissal, halt | passes | +| Dismissal leaves a different screen | halt at the step's identity check | passes | + +## Limits + +- A `GET` request with a side effect gets through the browser fence. On + desktop backends, the read-back sees only the records that the run's + verifier reads. Configure a read-back that covers the records the run + touches. +- A session keep-alive that POSTs is a write, so the run halts on it. Use + a session warning whose control only closes it, or re-authenticate. +- Consent prompts (cookie "Accept" or "Reject") are decisions, not + dismissals, so they stay outside the closed set. +- Session re-authentication, a bounded re-resolve after late page loads, + and real-app runs on OpenEMR and Dolibarr are not covered here. diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index 90e629b093..d1f6dd1439 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -8,6 +8,7 @@ from __future__ import annotations +import contextlib import hashlib import hmac import json @@ -18,7 +19,7 @@ import uuid from dataclasses import dataclass, field from datetime import date, datetime, timezone -from typing import TYPE_CHECKING, Any, Callable, Literal, Optional +from typing import TYPE_CHECKING, Any, Callable, Iterator, Literal, Optional from urllib.parse import urlsplit from openadapt_flow.backends._page_quiescence import PageQuiescence @@ -2652,7 +2653,9 @@ def form_identity_at(self, x: int, y: int, identity: FormIdentity) -> Optional[s # -- the wire check (openadapt_flow.wire_check) -------------------------- - def _arm_wire_check(self, submission: ExpectedSubmission) -> Optional[str]: + def _arm_wire_check( + self, submission: Optional[ExpectedSubmission], *, check: Any = None + ) -> Optional[str]: """Hold the save's request at the network to the checked form. Until it is disarmed, every write request from any page of this @@ -2704,7 +2707,8 @@ def _arm_wire_check(self, submission: ExpectedSubmission) -> Optional[str]: if workers: # A worker's WebSockets skip the page's guard and its routes. return "a worker runs on this page, and its WebSockets can't be held" - check = WireCheck(submission=submission) + if check is None: + check = WireCheck(submission=submission) def handler(route: Any) -> None: check.handle(route, proceed=route.fallback) @@ -2967,6 +2971,36 @@ def _disarm_wire_check(self) -> list[str]: self._form_wire = None return reasons + @contextlib.contextmanager + def write_fence(self) -> Iterator["WriteFenceReport"]: + """Run the ``with`` body while nothing on the page can write. + + The same hold as a checked save (:meth:`_arm_wire_check`), with no + submission allowed: every write request from any page of the context + is aborted before it leaves, and a WebSocket, worker or message sent + meanwhile is refused in the page. Each is reported. A fence that + can't go up sets ``refusal`` and the body must not act. Used around + the dismissal of an interruption + (:mod:`openadapt_flow.runtime.effect_watch`). + """ + + report = WriteFenceReport() + if self._form_wire is not None: + report.refusal = "a save is already held on this page" + yield report + return + refusal = self._arm_wire_check(None, check=WireCheck()) + if refusal is not None: + report.refusal = refusal + yield report + return + try: + yield report + # Let anything the action queued reach the hold while it is up. + self.wait_form_submit_guard(WRITE_FENCE_TAIL_MS) + finally: + report.attempts.extend(self._disarm_wire_check()) + def wait_form_submit_guard(self, ms: int) -> None: """Let the page and the wire check run for ``ms`` milliseconds.""" @@ -5274,6 +5308,26 @@ def _ws_guard_secrets(target: Any) -> tuple[str, str, str]: return secrets +#: How long a write fence stays up after its action, so a request the action +#: queued just before the fence comes down is still caught. +WRITE_FENCE_TAIL_MS = 500 + + +@dataclass +class WriteFenceReport: + """What a write fence saw. ``refusal`` set means it never went up.""" + + refusal: Optional[str] = None + #: Each write the fence stopped, and how. + attempts: list[str] = field(default_factory=list) + + @property + def held(self) -> bool: + """The fence was up for the whole action and nothing tried to write.""" + + return self.refusal is None and not self.attempts + + def _wire_reasons(check: WireCheck, notes: list[str]) -> list[str]: """Each refusal says what happened: an aborted request never left.""" diff --git a/packages/flow/openadapt_flow/deployment.py b/packages/flow/openadapt_flow/deployment.py index 3a256e3cb0..f78f835282 100644 --- a/packages/flow/openadapt_flow/deployment.py +++ b/packages/flow/openadapt_flow/deployment.py @@ -801,6 +801,12 @@ class RuntimeSection(BaseModel): #: ``known_ids_max_age_hours``. See :mod:`openadapt_flow.runtime.known_ids`. #: None (default) keeps the halt. known_ids_path: Optional[str] = None + #: Per-app interruption classes (YAML; :mod:`openadapt_flow.interruptions`): + #: session warnings, banners, tours and toasts the run may clear itself, + #: each only when the dismissal is proven read-only. A file with any entry + #: that fails that policy stops the run before it starts. None (default) + #: halts on every interruption. + interruptions_path: Optional[str] = None #: Identifier column when ``known_ids_path`` is a CSV/TSV export. known_ids_column: Optional[str] = None #: Confirmation record; default ``.confirmation.json``. @@ -1842,6 +1848,11 @@ def build_replayer( confirmation_path=runtime_config.known_ids_confirmation_path, max_age_s=runtime_config.known_ids_max_age_hours * 3600.0, ) + interruptions = None + if runtime_config is not None and runtime_config.interruptions_path: + from openadapt_flow.interruptions import load_interruption_classes + + interruptions = load_interruption_classes(runtime_config.interruptions_path) identity_copy = None read_clipboard = None copy_cfg = runtime_config.identity_copy_look if runtime_config is not None else None @@ -1882,6 +1893,7 @@ def build_replayer( qualification_environment_observer=qualification_environment_observer, approval_gate=approval_gate, deployment_surface=deployment_surface, + interstitials=interruptions, ) diff --git a/packages/flow/openadapt_flow/interruptions.py b/packages/flow/openadapt_flow/interruptions.py new file mode 100644 index 0000000000..473954093e --- /dev/null +++ b/packages/flow/openadapt_flow/interruptions.py @@ -0,0 +1,357 @@ +"""Interruption classes: benign interruptions a run may clear by itself. + +Production apps interrupt: "Your session expires in 5 minutes", an "Update +available" toast, a feature tour, a banner, a modal from background polling. +Each would halt a compiled run for a person. An *interruption class* lets the +run clear one itself, but only when all four of these hold +(``docs/INTERRUPTION_RECOVERY.md``): + +a. **Identity.** It matches the class by what it says and what its control + is: every one of the class's texts is on screen, and the dismiss control + resolves by its role, name, selector or window, never by position. +b. **A non-committing action.** The dismissal is Escape or a control whose + label is in a closed set (:data:`NON_COMMITTING_LABEL`): close, dismiss, + not now, continue session, and the like. "OK", "Continue", "Yes", + "Accept" and any write word are outside it. +c. **No write.** The dismissal runs under the run's write watchers + (:mod:`openadapt_flow.runtime.effect_watch`): the browser's write fence + and the system-of-record read-back. A watcher that sees a write, or a run + with none, halts. +d. **The expected screen comes back.** The class's text is gone, and the + step then goes through its normal identity check; a screen that isn't + the one the step expects halts there. + +Every dismissal is written to the step's report and to the write receipt. + +Classes are data, never engine code: a per-app YAML file (see +:func:`load_interruption_classes`) or a class a person taught from a halt +(:func:`learn_interruption`). Each becomes an :class:`~openadapt_flow.ir.Interstitial` +with ``effect_watch=True``, so the replayer's existing interstitial path +(one audited action, re-settle, clearance check, no retry) applies. +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from pathlib import Path +from typing import TYPE_CHECKING, Any, Callable, Mapping, Optional, Sequence + +if TYPE_CHECKING: # pragma: no cover + from openadapt_flow.ir import Interstitial, Predicate + +#: The labels a dismiss control may carry. Closed on purpose: a label outside +#: it ("OK", "Continue", "Yes", "Accept", "Submit") may commit something, so +#: it is never cleared automatically, whatever a watcher would have seen. +NON_COMMITTING_LABEL = re.compile( + r"^\s*(?:close(?:\s+(?:dialog|banner|notification|message|tour|this))?|" + r"dismiss(?:\s+(?:all|notification|message|banner))?|" + r"not\s+now|maybe\s+later|later|remind\s+me\s+later|no,?\s+thanks|" + r"skip(?:\s+(?:for\s+now|tour|this))?|got\s+it|hide|" + r"continue\s+(?:session|working)|stay\s+(?:signed|logged)\s+in|" + r"keep\s+me\s+(?:signed|logged)\s+in|extend\s+(?:my\s+)?session|" + r"i'?m\s+still\s+here)\s*[.!]?\s*$", + re.IGNORECASE, +) + + +def is_non_committing_label(label: Optional[str]) -> bool: + """Whether ``label`` is one of the closed non-committing dismiss labels.""" + + return bool(label and NON_COMMITTING_LABEL.match(label)) + + +def dismiss_label(interstitial: "Interstitial") -> Optional[str]: + """The dismiss control's label: its accessible name, else its OCR text.""" + + anchor = interstitial.dismiss_anchor + if anchor is None: + return None + structural = anchor.structural + if structural is not None and structural.name and structural.name.strip(): + return structural.name.strip() + if anchor.ocr_text and anchor.ocr_text.strip(): + return anchor.ocr_text.strip() + return None + + +def _has_text_identity(pred: "Predicate") -> bool: + from openadapt_flow.ir import PredicateKind + + if pred.kind is PredicateKind.TEXT_PRESENT: + return bool(pred.text and pred.text.strip()) + if pred.kind is PredicateKind.AND: + return any(_has_text_identity(op) for op in pred.operands) + if pred.kind is PredicateKind.OR: + return bool(pred.operands) and all( + _has_text_identity(op) for op in pred.operands + ) + return False + + +def interruption_class_error(interstitial: "Interstitial") -> Optional[str]: + """Why ``interstitial`` can't be an automatically cleared interruption + class, or None when it can. Checked on load and again at run time.""" + + if interstitial.dismiss_key is None and interstitial.dismiss_anchor is None: + return "an interruption class must declare its dismissal" + if not _has_text_identity(interstitial.detect): + return ( + "an interruption class must be detected by its text (TEXT_PRESENT), " + "not only by how it looks or where it is" + ) + if interstitial.dismiss_key is not None: + return None # Escape only (Interstitial's own validation) + label = dismiss_label(interstitial) + if not is_non_committing_label(label): + return ( + f"the dismiss control {label!r} is not a non-committing label " + "(close, dismiss, not now, continue session, ...); it may commit " + "something, so it is never cleared automatically" + ) + anchor = interstitial.dismiss_anchor + assert anchor is not None + structural = anchor.structural + has_identity = bool(anchor.template.strip()) or ( + structural is not None + and any( + (v or "").strip() + for v in (structural.selector, structural.automation_id, structural.name) + ) + ) + if not has_identity: + return "the dismiss control must be found by its identity, not its position" + return None + + +# -- per-app data files --------------------------------------------------------- + + +class InterruptionClassError(ValueError): + """An interruption class file or entry that can't be used.""" + + +def interruption_from_data(entry: Mapping[str, Any]) -> "Interstitial": + """One class from its data-file entry. + + ``{name, texts: [..], dismiss: {key: Escape} | {control: {role, name, + selector, automation_id, window}}, gone: }``. ``gone`` defaults to + the first text: the clearance is that text no longer on screen. + """ + + from openadapt_flow.ir import ( + Anchor, + Interstitial, + Predicate, + PredicateKind, + StructuralLocator, + ) + + name = str(entry.get("name") or "").strip() + texts = [str(t) for t in entry.get("texts") or () if str(t).strip()] + if not name or not texts: + raise InterruptionClassError("each interruption needs a name and texts") + dismiss = entry.get("dismiss") or {} + if not isinstance(dismiss, Mapping): + raise InterruptionClassError(f"{name}: dismiss must be a mapping") + key = dismiss.get("key") + control = dismiss.get("control") + anchor = None + if control is not None: + if not isinstance(control, Mapping): + raise InterruptionClassError(f"{name}: dismiss.control must be a mapping") + anchor = Anchor( + template="", + structural=StructuralLocator( + selector=control.get("selector"), + role=control.get("role"), + name=control.get("name"), + automation_id=control.get("automation_id"), + window_name=control.get("window"), + ), + region=(0, 0, 0, 0), + click_point=(0, 0), + ocr_text=control.get("name"), + ) + present = [Predicate(kind=PredicateKind.TEXT_PRESENT, text=t) for t in texts] + detect = ( + present[0] + if len(present) == 1 + else Predicate(kind=PredicateKind.AND, operands=present) + ) + gone = str(entry.get("gone") or texts[0]) + try: + return Interstitial( + name=name, + detect=detect, + dismiss_key=key, + dismiss_anchor=anchor, + risk="reversible", + consequential=False, + clearance=Predicate(kind=PredicateKind.TEXT_ABSENT, text=gone), + effect_watch=True, + ) + except Exception as exc: + raise InterruptionClassError(f"{name}: {exc}") from exc + + +def interruption_to_data(interstitial: "Interstitial") -> dict[str, Any]: + """The data-file entry for a class, the inverse of + :func:`interruption_from_data` for classes it can express.""" + + from openadapt_flow.ir import PredicateKind + + detect = interstitial.detect + operands = detect.operands if detect.kind is PredicateKind.AND else [detect] + texts = [ + op.text for op in operands if op.kind is PredicateKind.TEXT_PRESENT and op.text + ] + entry: dict[str, Any] = {"name": interstitial.name, "texts": texts} + if interstitial.dismiss_key is not None: + entry["dismiss"] = {"key": interstitial.dismiss_key} + else: + anchor = interstitial.dismiss_anchor + assert anchor is not None and anchor.structural is not None + s = anchor.structural + control = { + "role": s.role, + "name": s.name, + "selector": s.selector, + "automation_id": s.automation_id, + "window": s.window_name, + } + entry["dismiss"] = {"control": {k: v for k, v in control.items() if v}} + clearance = interstitial.clearance + if ( + clearance is not None + and clearance.text + and texts + and clearance.text != texts[0] + ): + entry["gone"] = clearance.text + return entry + + +def load_interruption_classes(path: Path | str) -> list["Interstitial"]: + """The classes in a per-app interruption file (YAML):: + + interruptions: + - name: session expiry warning + texts: ["Your session will expire"] + dismiss: {control: {role: button, name: Stay signed in}} + - name: product tour + texts: ["Take a quick tour"] + dismiss: {key: Escape} + + Raises :class:`InterruptionClassError` on any entry that fails the + read-only policy, so a bad file stops the run before it starts. + """ + + import yaml + + data = yaml.safe_load(Path(path).read_text(encoding="utf-8")) or {} + entries = data.get("interruptions") if isinstance(data, Mapping) else None + if not isinstance(entries, list): + raise InterruptionClassError(f"{path}: expected an 'interruptions' list") + classes = [interruption_from_data(entry) for entry in entries] + names = [c.name for c in classes] + if len(set(names)) != len(names): + raise InterruptionClassError(f"{path}: interruption names must be unique") + return classes + + +def save_interruption_class(path: Path | str, interstitial: "Interstitial") -> None: + """Add (or replace by name) one class in a per-app interruption file.""" + + import yaml + + path = Path(path) + data: dict[str, Any] = {"interruptions": []} + if path.exists(): + loaded = yaml.safe_load(path.read_text(encoding="utf-8")) or {} + if isinstance(loaded, Mapping): + data = dict(loaded) + entries = [ + e + for e in data.get("interruptions") or [] + if not (isinstance(e, Mapping) and e.get("name") == interstitial.name) + ] + entries.append(interruption_to_data(interstitial)) + data["interruptions"] = entries + path.write_text(yaml.safe_dump(data, sort_keys=False), encoding="utf-8") + + +# -- learning a class from a halt ----------------------------------------------- + + +class LearnRefused(RuntimeError): + """The interruption can't be learned as a class; the reason says why.""" + + +@dataclass(frozen=True) +class LearnedInterruption: + interstitial: "Interstitial" + #: What the watchers saw: the proof the dismissal wrote nothing. + watchers: tuple[str, ...] + + +def learn_interruption( + *, + name: str, + texts: Sequence[str], + control: Mapping[str, Any], + backend: Any, + effect_verifier: Any = None, + shows: Callable[[str], bool], + settle: Callable[[], Any] = lambda: None, +) -> LearnedInterruption: + """Offer to learn the interruption a person is clearing during a halt. + + ``control`` is the control the person pointed at (``role``, ``name``, + ``selector``, ...). The class is built and checked against the read-only + policy first (identity, non-committing label); then this function clicks + the control under the run's write watchers on the live screen and + learns the class only when no watcher saw a write and the interruption's + text is gone (``shows(text)`` is False). A refusal after the click + means the screen changed, and the person continues by hand. + """ + + from openadapt_flow.ir import StructuralLocator + from openadapt_flow.runtime.effect_watch import watch_effects + + interstitial = interruption_from_data( + {"name": name, "texts": list(texts), "dismiss": {"control": dict(control)}} + ) + missing = [t for t in texts if not shows(t)] + if missing: + raise LearnRefused(f"the interruption's text is not on screen: {missing}") + locate = getattr(backend, "locate_structural", None) + act = getattr(backend, "act_structural", None) + if not callable(locate) or not callable(act): + raise LearnRefused( + "this backend can't find the control by its identity, so the " + "class can't be learned here" + ) + anchor = interstitial.dismiss_anchor + assert anchor is not None and anchor.structural is not None + locator: StructuralLocator = anchor.structural + handle = locate(locator) + if handle is None: + raise LearnRefused(f"{dismiss_label(interstitial)!r} did not resolve uniquely") + with watch_effects(backend, effect_verifier) as report: + if report.refusal is not None: + raise LearnRefused(f"the dismissal can't be watched: {report.refusal}") + act(locator, handle) + settle() + if not report.held: + raise LearnRefused( + "the dismissal tried to write (" + + "; ".join(report.attempts) + + "), so it is not learned" + ) + gone = interstitial.clearance.text if interstitial.clearance else texts[0] + if gone and shows(gone): + raise LearnRefused(f"{gone!r} is still on screen after the dismissal") + return LearnedInterruption( + interstitial=interstitial, watchers=tuple(report.watchers) + ) diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index 494fa4722f..0a3f0ebad5 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -1401,6 +1401,26 @@ class Interstitial(BaseModel): description="Expected visual postcondition after dismissal. It must hold " "and the detection predicate must no longer hold before replay continues.", ) + effect_watch: Optional[bool] = Field( + default=None, + description="Dismiss only under the run's write watchers " + "(openadapt_flow.runtime.effect_watch): the browser's write fence and " + "the system-of-record read-back. A run that can't watch, or any write " + "a watcher sees, halts the run. Set on every interruption class " + "(openadapt_flow.interruptions), declared or learned; it also limits " + "the dismissal to Escape or a control whose label is a closed " + "non-committing one (close, dismiss, not now, continue session).", + ) + + @model_serializer(mode="wrap") + def _serialize_compatible(self, handler: Any) -> dict[str, Any]: + """Leave ``effect_watch`` out when unset, so every existing + interstitial (and bundle digest) serializes exactly as before.""" + + data: dict[str, Any] = handler(self) + if self.effect_watch is None: + data.pop("effect_watch", None) + return data @model_validator(mode="after") def _validate_safe_detection_and_dismissal(self) -> "Interstitial": @@ -1493,6 +1513,12 @@ def visual_postcondition(pred: Predicate) -> bool: "interstitial clearance must be a visual postcondition; " "parameter-only predicates cannot verify a UI dismissal" ) + if self.effect_watch: + from openadapt_flow.interruptions import interruption_class_error + + error = interruption_class_error(self) + if error is not None: + raise ValueError(error) return self @@ -4235,6 +4261,24 @@ class InterstitialActionResult(BaseModel): before_frame_sha256: str = Field(pattern=r"^[0-9a-f]{64}$") after_frame_sha256: Optional[str] = Field(default=None, pattern=r"^[0-9a-f]{64}$") error: Optional[str] = None + #: The write watchers that were up around the dismissal (``"network"``, + #: ``"record"``); empty when the interstitial declares no effect watch. + effect_watchers: list[str] = Field(default_factory=list) + #: Each write a watcher saw. Non-empty halts the run. + write_attempts: list[str] = Field(default_factory=list) + #: The dismiss control's label, for the receipt. + control_label: Optional[str] = None + + @model_serializer(mode="wrap") + def _serialize_compatible(self, handler: Any) -> dict[str, Any]: + """Leave the effect-watch fields out when unused, so reports of + existing interstitials serialize exactly as before.""" + + data: dict[str, Any] = handler(self) + for key in ("effect_watchers", "write_attempts", "control_label"): + if not data.get(key): + data.pop(key, None) + return data class ProgramExecutionScopeFrame(BaseModel): diff --git a/packages/flow/openadapt_flow/runtime/effect_watch.py b/packages/flow/openadapt_flow/runtime/effect_watch.py new file mode 100644 index 0000000000..9c49bc6f79 --- /dev/null +++ b/packages/flow/openadapt_flow/runtime/effect_watch.py @@ -0,0 +1,130 @@ +"""Watch an interruption's dismissal for any write, on any backend. + +An interruption (a session warning, a banner, a tour, an "Update available" +toast) may be cleared automatically only when clearing it writes nothing +(``docs/INTERRUPTION_RECOVERY.md``). A label can't prove that: "Continue" +on a session warning and "Continue" on a wizard look the same. What the +dismissal did can. :func:`watch_effects` runs the dismissal under every +watcher the run has, and the replayer halts when any of them saw a write or +none could watch: + +- **network**: a backend with ``write_fence()`` (the browser) aborts every + write request, WebSocket, worker and socket message before it leaves the + page, and reports each one; +- **record**: the run's effect verifier (the system-of-record read-back the + steps' writes are checked with) is read before and after; any difference + in its records is a write. This works on every backend, desktop and + RDP/Citrix included, because it never looks at the screen. + +A watcher that can't go up, or a read-back that can't be read, is a refusal: +the caller treats it as "no proof" and halts. +""" + +from __future__ import annotations + +import hashlib +import json +from contextlib import ExitStack, contextmanager +from dataclasses import dataclass, field +from typing import Any, Iterator, Optional + + +@dataclass +class EffectWatchReport: + """What the watchers saw around one dismissal.""" + + #: The watchers that were up: ``"network"``, ``"record"``. + watchers: list[str] = field(default_factory=list) + #: Why the watch could not prove anything. Set means "don't act". + refusal: Optional[str] = None + #: Each write a watcher saw. + attempts: list[str] = field(default_factory=list) + #: The system-of-record digest before and after, when it was read. + record_before: Optional[str] = None + record_after: Optional[str] = None + + @property + def held(self) -> bool: + """At least one watcher was up and none of them saw a write.""" + + return bool(self.watchers) and self.refusal is None and not self.attempts + + +def record_digest(verifier: Any) -> Optional[str]: + """A digest of the system-of-record records ``verifier`` reads, or None + when they could not be read.""" + + try: + state = verifier.capture_pre_state() + except Exception: # noqa: BLE001 - unreadable is no proof + return None + if not getattr(state, "reachable", False): + return None + try: + records = sorted( + json.dumps(record, sort_keys=True, default=str) for record in state.records + ) + except Exception: # noqa: BLE001 + return None + return hashlib.sha256("\n".join(records).encode()).hexdigest() + + +@contextmanager +def watch_effects( + backend: Any, effect_verifier: Any = None +) -> Iterator[EffectWatchReport]: + """Run the ``with`` body under every write watcher available. + + Check ``report.refusal`` before acting inside the body; after the body, + ``report.held`` says whether the dismissal is proven write-free. + """ + + report = EffectWatchReport() + with ExitStack() as stack: + fence_opener = getattr(backend, "write_fence", None) + fence: Any = None + if callable(fence_opener): + try: + fence = stack.enter_context(fence_opener()) + except Exception as exc: # noqa: BLE001 - no fence, no proof + report.refusal = ( + f"the write fence failed to go up ({type(exc).__name__})" + ) + else: + if fence.refusal is not None: + report.refusal = f"the write fence could not go up: {fence.refusal}" + else: + report.watchers.append("network") + reads = effect_verifier is not None and callable( + getattr(effect_verifier, "capture_pre_state", None) + ) + if reads and report.refusal is None: + report.record_before = record_digest(effect_verifier) + if report.record_before is not None: + report.watchers.append("record") + elif not report.watchers: + # With the network fenced, every write the page can make is + # already stopped; without it, the read-back is the only proof. + report.refusal = "the system of record could not be read before" + if report.refusal is None and not report.watchers: + report.refusal = ( + "this run has no way to see a write: the backend can't fence " + "the network and no system-of-record read-back is configured" + ) + try: + yield report + finally: + # The fence (if any) closes with the stack, below; its blocked + # writes are collected after. + pass + if fence is not None and "network" in report.watchers: + report.attempts.extend(f"network: {a}" for a in fence.attempts) + if "record" in report.watchers: + report.record_after = record_digest(effect_verifier) + if report.record_after is None: + report.attempts.append( + "record: the system of record could not be read after the " + "dismissal, so a write can't be ruled out" + ) + elif report.record_after != report.record_before: + report.attempts.append("record: the system of record changed") diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index ef03ef4ca8..365dc78fa2 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -49,6 +49,7 @@ import time import uuid from collections import Counter +from contextlib import ExitStack from copy import deepcopy from dataclasses import dataclass from datetime import date, datetime, timezone @@ -96,6 +97,7 @@ normalize_signal_text, parameterize_identity_text, ) +from openadapt_flow.interruptions import dismiss_label from openadapt_flow.ir import ( ActionDeliveryUncertainty, ActionKind, @@ -188,6 +190,7 @@ from openadapt_flow.runtime.durable.program_checkpoint import ( history_hash as _history_hash, ) +from openadapt_flow.runtime.effect_watch import EffectWatchReport, watch_effects from openadapt_flow.runtime.effects import ( Effect, EffectState, @@ -15116,6 +15119,7 @@ def _handle_interstitials( # unreported key/click attempt. The event carries the exact admitted # policy and expected visual outcome. event = InterstitialActionResult( + control_label=dismiss_label(it), interstitial=it.name, action=action, key=it.dismiss_key, @@ -15148,76 +15152,117 @@ def _handle_interstitials( f"durable continuation was preempted before delivery: {exc}" ) return before_png, event.error - try: - if it.dismiss_key is not None: - self._require_qualification_environment_current() - self._deliver_backend_call( - result, lambda: self.backend.press(it.dismiss_key or "Escape") + watch_stack = ExitStack() + watch: Optional[EffectWatchReport] = None + if it.effect_watch: + # An interruption class is cleared only when the dismissal is + # proven write-free: every watcher this run has is up around + # it, or nothing is sent. + watched: list[EffectWatchReport] = [] + + def _record_watch() -> None: + # Registered first, so it runs after the watchers close + # on every exit, a failed delivery included. + if watched: + event.effect_watchers = list(watched[0].watchers) + event.write_attempts = list(watched[0].attempts) + + watch_stack.callback(_record_watch) + watch = watch_stack.enter_context( + watch_effects(self.backend, self.effect_verifier) + ) + watched.append(watch) + if watch.refusal is not None: + watch_stack.close() + event.error = f"dismissal not watched: {watch.refusal}" + return before_png, ( + f"Interruption '{it.name}' can't be cleared: " + f"{watch.refusal} - refusing to act beneath it; run " + "aborted." ) - else: - assert resolution is not None - assert it.dismiss_anchor is not None - if resolution.rung == "structural": - native_act = getattr(self.backend, "act_structural", None) - structural_anchor = it.dismiss_anchor.structural - if ( - not callable(native_act) - or resolution.structural_handle is None - or structural_anchor is None - ): - raise StructuralResolutionRefused( - "structural dismissal cannot be delivered " - "natively without a stable element handle" - ) + with watch_stack: + try: + if it.dismiss_key is not None: self._require_qualification_environment_current() self._deliver_backend_call( result, - lambda: native_act( - structural_anchor, - resolution.structural_handle, - ), + lambda: self.backend.press(it.dismiss_key or "Escape"), ) else: - self._require_qualification_environment_current() - self._deliver_backend_call( - result, - lambda: self.backend.click( - int(resolution.point[0]), int(resolution.point[1]) - ), - ) - event.delivered = True - except Exception as exc: - if not isinstance( - exc, (FreshActuationRequired, StructuralResolutionRefused) - ): - target_fingerprint = None - if ( - resolution is not None - and resolution.structural_handle is not None + assert resolution is not None + assert it.dismiss_anchor is not None + if resolution.rung == "structural": + native_act = getattr(self.backend, "act_structural", None) + structural_anchor = it.dismiss_anchor.structural + if ( + not callable(native_act) + or resolution.structural_handle is None + or structural_anchor is None + ): + raise StructuralResolutionRefused( + "structural dismissal cannot be delivered " + "natively without a stable element handle" + ) + self._require_qualification_environment_current() + self._deliver_backend_call( + result, + lambda: native_act( + structural_anchor, + resolution.structural_handle, + ), + ) + else: + self._require_qualification_environment_current() + self._deliver_backend_call( + result, + lambda: self.backend.click( + int(resolution.point[0]), int(resolution.point[1]) + ), + ) + event.delivered = True + except Exception as exc: + if not isinstance( + exc, (FreshActuationRequired, StructuralResolutionRefused) ): - target_fingerprint = ( - resolution.structural_handle.target_fingerprint + target_fingerprint = None + if ( + resolution is not None + and resolution.structural_handle is not None + ): + target_fingerprint = ( + resolution.structural_handle.target_fingerprint + ) + result.delivery_uncertainty = ActionDeliveryUncertainty( + operation="interstitial_dismiss", + native=( + resolution is not None + and resolution.rung == "structural" + ), + target_fingerprint=target_fingerprint, + observed_at=datetime.now(timezone.utc).isoformat(), + cause_type=type(exc).__name__, ) - result.delivery_uncertainty = ActionDeliveryUncertainty( - operation="interstitial_dismiss", - native=( - resolution is not None and resolution.rung == "structural" - ), - target_fingerprint=target_fingerprint, - observed_at=datetime.now(timezone.utc).isoformat(), - cause_type=type(exc).__name__, + event.error = ( + "dismissal delivery failed " + f"({type(exc).__name__}); outcome is unknown" + ) + return before_png, ( + f"Interstitial '{it.name}' dismissal delivery failed " + f"({type(exc).__name__}) - refusing to act beneath it; run " + "aborted." ) - event.error = ( - "dismissal delivery failed " - f"({type(exc).__name__}); outcome is unknown" - ) - return before_png, ( - f"Interstitial '{it.name}' dismissal delivery failed " - f"({type(exc).__name__}) - refusing to act beneath it; run " - "aborted." - ) - before_png, settle_error = self._resettle_after_interstitial(before_png) + before_png, settle_error = self._resettle_after_interstitial(before_png) + if watch is not None: + if watch.attempts: + event.after_frame_sha256 = hashlib.sha256(before_png).hexdigest() + event.error = "the dismissal tried to write" + result.safety_halt = True + return before_png, ( + f"Interruption '{it.name}': its dismissal tried to " + f"write ({'; '.join(watch.attempts)}) - it is not a " + "benign interruption; refusing to continue; run aborted." + ) event.after_frame_sha256 = hashlib.sha256(before_png).hexdigest() if settle_error is not None: event.error = settle_error diff --git a/packages/flow/tests/test_interruptions.py b/packages/flow/tests/test_interruptions.py new file mode 100644 index 0000000000..79117ba46c --- /dev/null +++ b/packages/flow/tests/test_interruptions.py @@ -0,0 +1,385 @@ +"""Interruption classes: cleared only when provably read-only. + +The four conditions (``docs/INTERRUPTION_RECOVERY.md``): identity by text and +control, a non-committing action, no write seen by any watcher, and the +expected screen afterwards. Each test below breaks one and expects a halt, +or holds all four and expects the run to complete with the dismissal in its +report. Backend and vision are faked; the real-browser cases are in +``tests/test_interruptions_browser.py``. +""" + +from __future__ import annotations + +from contextlib import contextmanager + +import pytest + +from openadapt_flow.interruptions import ( + InterruptionClassError, + LearnRefused, + interruption_from_data, + is_non_committing_label, + learn_interruption, + load_interruption_classes, + save_interruption_class, +) +from openadapt_flow.ir import ( + Anchor, + Interstitial, + Predicate, + PredicateKind, + StructuralHandle, + StructuralLocator, + Workflow, +) +from openadapt_flow.runtime.effect_watch import watch_effects +from openadapt_flow.runtime.replayer import Replayer +from tests.test_replayer import FakeBackend, Match, click_step, make_png +from tests.test_state_dependency import SettleVision + +SESSION = { + "name": "session expiry warning", + "texts": ["Your session will expire"], + "dismiss": {"control": {"role": "button", "name": "Stay signed in"}}, +} + + +@pytest.fixture() +def bundle(tmp_path): + bundle_dir = tmp_path / "bundle" + (bundle_dir / "templates").mkdir(parents=True) + (bundle_dir / "templates" / "btn.png").write_bytes(make_png((50, 20))) + return bundle_dir + + +@pytest.fixture() +def run_dir(tmp_path): + return tmp_path / "run" + + +class _Fence: + def __init__(self, refusal=None, attempts=()): + self.refusal = refusal + self.attempts = list(attempts) + + +class WatchedBackend(FakeBackend): + """A structural backend with a scripted write fence.""" + + def __init__(self, *, fence=True, refusal=None, writes=()): + super().__init__() + self._fence = fence + self._refusal = refusal + self._writes = list(writes) + self.fenced: list[bool] = [] + self._up = False + if not fence: + self.write_fence = None # type: ignore[assignment] + + def locate_structural(self, locator): + return StructuralHandle( + point=(15, 15), region=(10, 10, 10, 10), target_fingerprint="a" * 64 + ) + + def act_structural(self, locator, handle, *, double=False): + self.fenced.append(self._up) + self.actions.append(("act_structural", locator.name)) + + def press(self, key): + self.fenced.append(self._up) + super().press(key) + + @contextmanager + def write_fence(self): # type: ignore[override] + report = _Fence(refusal=self._refusal) + self._up = self._refusal is None + try: + yield report + finally: + self._up = False + if self._refusal is None: + report.attempts.extend(self._writes) + + +class Verifier: + """A system-of-record read-back whose records a dismissal may change.""" + + substrate = "fake" + + def __init__(self, change=False, reachable=True): + self.reads = 0 + self.change = change + self.reachable = reachable + + def capture_pre_state(self, context=None): + from openadapt_flow.runtime.effects.effect import EffectState + + self.reads += 1 + records = [{"id": 1}] + if self.change and self.reads > 1: + records.append({"id": 2}) + return EffectState(substrate="fake", reachable=self.reachable, records=records) + + +def _vision(text="Your session will expire", target=True): + vision = SettleVision(settled=True) + vision.text_results = {text: [Match((10, 10), (0, 0, 5, 5)), None, None]} + vision.template_results = [Match((110, 105), (100, 100, 50, 20))] if target else [] + return vision + + +def _run(backend, vision, bundle, run_dir, classes, **kw): + workflow = Workflow(name="wf", steps=[click_step()], interstitials=classes) + return Replayer(backend, vision=vision, poll_interval_s=0.005, **kw).run( + workflow, bundle_dir=bundle, run_dir=run_dir + ) + + +# -- (a) and (b): what a class may be ------------------------------------------- + + +@pytest.mark.parametrize( + "label", + ["Close", "Dismiss", "Not now", "Stay signed in", "Continue session", "Got it"], +) +def test_non_committing_labels(label): + assert is_non_committing_label(label) + + +@pytest.mark.parametrize( + "label", + [ + "OK", + "Continue", + "Yes", + "Accept", + "Accept all", + "Save", + "Submit", + "Delete", + "Close account", + "Confirm", + "Sign out", + "", + ], +) +def test_committing_or_ambiguous_labels_are_outside_the_closed_set(label): + assert not is_non_committing_label(label) + + +def test_a_class_whose_control_may_commit_is_refused(): + entry = dict(SESSION, dismiss={"control": {"role": "button", "name": "OK"}}) + with pytest.raises(InterruptionClassError, match="non-committing"): + interruption_from_data(entry) + + +def test_a_class_found_only_by_position_is_refused(): + anchor = Anchor( + template="", + structural=StructuralLocator(role="button"), + region=(0, 0, 10, 10), + click_point=(5, 5), + ocr_text="Close", + ) + with pytest.raises(ValueError, match="identity|structural"): + Interstitial( + name="x", + detect=Predicate(kind=PredicateKind.TEXT_PRESENT, text="x"), + dismiss_anchor=anchor, + risk="reversible", + consequential=False, + clearance=Predicate(kind=PredicateKind.TEXT_ABSENT, text="x"), + effect_watch=True, + ) + + +def test_an_existing_interstitial_serializes_as_before(): + it = Interstitial( + name="rate us", + detect=Predicate(kind=PredicateKind.TEXT_PRESENT, text="rate us"), + dismiss_key="Escape", + risk="reversible", + consequential=False, + clearance=Predicate(kind=PredicateKind.TEXT_ABSENT, text="rate us"), + ) + assert "effect_watch" not in it.model_dump() + + +def test_class_file_round_trip_and_bad_files(tmp_path): + path = tmp_path / "interruptions.yaml" + save_interruption_class(path, interruption_from_data(SESSION)) + tour = {"name": "tour", "texts": ["Take a tour"], "dismiss": {"key": "Escape"}} + save_interruption_class(path, interruption_from_data(tour)) + classes = load_interruption_classes(path) + assert [c.name for c in classes] == ["session expiry warning", "tour"] + assert all(c.effect_watch for c in classes) + path.write_text( + "interruptions:\n - {name: a, texts: [a], dismiss: {key: Enter}}\n" + ) + with pytest.raises(InterruptionClassError): + load_interruption_classes(path) + + +# -- (c) and (d) at run time ------------------------------------------------------ + + +def test_a_watched_dismissal_clears_and_is_reported(bundle, run_dir): + backend = WatchedBackend() + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + assert report.success is True, report.results[0].error + assert backend.actions[0] == ("act_structural", "Stay signed in") + assert backend.fenced == [True] + event = report.results[0].interstitial_actions[0] + assert event.ok and event.effect_watchers == ["network"] + assert event.write_attempts == [] + assert event.control_label == "Stay signed in" + + +def test_a_dismissal_that_tries_to_write_halts(bundle, run_dir): + backend = WatchedBackend(writes=["POST /session/end"]) + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + assert report.success is False + result = report.results[0] + assert result.safety_halt is True + assert "tried to write" in (result.error or "") + assert ("click", 110, 105, False) not in backend.actions + assert result.interstitial_actions[0].write_attempts + + +def test_a_run_that_cannot_watch_never_dismisses(bundle, run_dir): + backend = WatchedBackend(fence=False) + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + assert report.success is False + assert "no way to see a write" in (report.results[0].error or "") + assert backend.actions == [] + + +def test_a_fence_that_cannot_go_up_never_dismisses(bundle, run_dir): + backend = WatchedBackend(refusal="a worker runs on this page") + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + assert report.success is False + assert backend.actions == [] + + +def test_record_read_back_watches_a_desktop_dismissal(bundle, run_dir): + backend = WatchedBackend(fence=False) + verifier = Verifier() + tour = { + "name": "tour", + "texts": ["Your session will expire"], + "dismiss": {"key": "Escape"}, + } + report = _run( + backend, + _vision(), + bundle, + run_dir, + [interruption_from_data(tour)], + effect_verifier=verifier, + ) + event = report.results[0].interstitial_actions[0] + assert event.effect_watchers == ["record"] + assert event.ok + assert backend.actions[0] == ("press", "Escape") + + +def test_record_change_during_a_dismissal_halts(bundle, run_dir): + backend = WatchedBackend(fence=False) + report = _run( + backend, + _vision(), + bundle, + run_dir, + [interruption_from_data(SESSION)], + effect_verifier=Verifier(change=True), + ) + assert report.success is False + assert report.results[0].safety_halt is True + assert "system of record changed" in (report.results[0].error or "") + + +def test_dismissal_onto_an_unexpected_screen_halts_at_identity(bundle, run_dir): + backend = WatchedBackend() + report = _run( + backend, + _vision(target=False), + bundle, + run_dir, + [interruption_from_data(SESSION)], + ) + assert report.success is False + assert report.results[0].interstitial_actions[0].ok + assert not any(a[0] == "click" for a in backend.actions) + + +def test_an_unknown_dialog_halts(bundle, run_dir): + backend = WatchedBackend() + vision = _vision(text="Something else", target=False) + report = _run(backend, vision, bundle, run_dir, [interruption_from_data(SESSION)]) + assert report.success is False + assert report.results[0].interstitial_actions == [] + assert backend.actions == [] + + +# -- learning a class from a halt -------------------------------------------------- + + +def test_learning_proves_the_dismissal_before_keeping_it(): + backend = WatchedBackend() + on_screen = {"Your session will expire": True} + + def shows(text): + return on_screen.get(text, False) + + def settle(): + on_screen["Your session will expire"] = False + + learned = learn_interruption( + name="session expiry warning", + texts=["Your session will expire"], + control={"role": "button", "name": "Stay signed in"}, + backend=backend, + shows=shows, + settle=settle, + ) + assert learned.watchers == ("network",) + assert learned.interstitial.effect_watch is True + + +def test_learning_refuses_a_dismissal_that_writes(): + backend = WatchedBackend(writes=["POST /x"]) + with pytest.raises(LearnRefused, match="tried to write"): + learn_interruption( + name="n", + texts=["t"], + control={"role": "button", "name": "Close"}, + backend=backend, + shows=lambda text: True, + ) + + +def test_learning_refuses_a_committing_control_without_clicking(): + backend = WatchedBackend() + with pytest.raises(InterruptionClassError): + learn_interruption( + name="n", + texts=["t"], + control={"role": "button", "name": "Confirm"}, + backend=backend, + shows=lambda text: True, + ) + assert backend.actions == [] + + +def test_watch_needs_a_watcher(): + with watch_effects(object(), None) as report: + assert report.refusal is not None + assert not report.held diff --git a/packages/flow/tests/test_interruptions_browser.py b/packages/flow/tests/test_interruptions_browser.py new file mode 100644 index 0000000000..0229153b74 --- /dev/null +++ b/packages/flow/tests/test_interruptions_browser.py @@ -0,0 +1,195 @@ +"""Interruption classes end to end: a real browser, the real replayer and OCR. + +A local synthetic app saves a note with ``POST /save``. Before the save, it +shows one interruption, chosen by the page address. The server counts every +request it receives, so each case can tell a cleared interruption from a +halted run and a blocked write from one that reached the server. + +Pre-registered cases (``docs/INTERRUPTION_RECOVERY.md``): + +============= =============================================== ============ +interruption what its control does expected +============= =============================================== ============ +session "Stay signed in" removes the dialog (GET ping) saved +banner "Close" removes an "Update available" banner saved +committing "Stay signed in" also POSTs /logout halt, no write +unknown an unlisted dialog halt, no click +============= =============================================== ============ +""" + +from __future__ import annotations + +import http.server +import threading +from typing import Any, Iterator + +import pytest + +pytest.importorskip("playwright.sync_api") + +from openadapt_flow.interruptions import interruption_from_data # noqa: E402 +from openadapt_flow.ir import ( # noqa: E402 + ActionKind, + Anchor, + Step, + StructuralLocator, + Workflow, +) + +PAGE = b""" +

Encounter note

+ +
+""" + + +class _Server(http.server.BaseHTTPRequestHandler): + seen: list[str] = [] + + def do_GET(self) -> None: # noqa: N802 - http.server API + type(self).seen.append(f"GET {self.path}") + self.send_response(200) + self.send_header("content-type", "text/html") + self.end_headers() + self.wfile.write(PAGE) + + def do_POST(self) -> None: # noqa: N802 - http.server API + type(self).seen.append(f"POST {self.path}") + self.send_response(204) + self.end_headers() + + def log_message(self, *_: Any) -> None: + pass + + +CLASSES = [ + interruption_from_data( + { + "name": "session expiry warning", + "texts": ["Your session will expire"], + "dismiss": {"control": {"role": "button", "name": "Stay signed in"}}, + } + ), + interruption_from_data( + { + "name": "update banner", + "texts": ["Update available"], + "dismiss": {"control": {"role": "button", "name": "Close"}}, + } + ), +] + + +@pytest.fixture() +def server() -> Iterator[str]: + _Server.seen = [] + srv = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _Server) + thread = threading.Thread(target=srv.serve_forever, daemon=True) + thread.start() + try: + yield f"http://127.0.0.1:{srv.server_address[1]}/" + finally: + srv.shutdown() + + +def _run(url: str, kind: str, tmp_path: Any) -> Any: + from openadapt_flow.backends.playwright_backend import PlaywrightBackend + from openadapt_flow.runtime.replayer import Replayer + + try: + backend, close = PlaywrightBackend.launch(url + "#" + kind, headless=True) + except Exception as exc: # pragma: no cover - no browser installed + pytest.skip(f"no Playwright browser: {exc}") + workflow = Workflow( + name="save note", + steps=[ + Step( + id="save", + intent="click 'Save note'", + action=ActionKind.CLICK, + anchor=Anchor( + template="", + structural=StructuralLocator(selector="#save"), + region=(8, 80, 120, 30), + click_point=(60, 95), + ocr_text="Save note", + ), + risk="reversible", + ) + ], + interstitials=CLASSES, + ) + bundle = tmp_path / "bundle" + bundle.mkdir() + try: + report = Replayer(backend).run( + workflow, bundle_dir=bundle, run_dir=tmp_path / "run" + ) + backend.page.wait_for_timeout(300) + finally: + close() + return report + + +@pytest.mark.parametrize("kind", ["session", "banner"]) +def test_a_benign_interruption_is_cleared_and_the_save_lands( + server: str, kind: str, tmp_path: Any +) -> None: + report = _run(server, kind, tmp_path) + assert report.success is True, report.results[0].error + event = report.results[0].interstitial_actions[0] + assert event.ok and event.effect_watchers == ["network"] + assert event.write_attempts == [] + assert "POST /save" in _Server.seen + + +def test_a_dismissal_that_commits_halts_and_nothing_is_written( + server: str, tmp_path: Any +) -> None: + report = _run(server, "committing", tmp_path) + assert report.success is False + result = report.results[0] + assert result.safety_halt is True + assert any("/logout" in a for a in result.interstitial_actions[0].write_attempts) + # The fence stopped the logout in the browser, and the save never ran. + assert not any(seen.startswith("POST") for seen in _Server.seen) + + +def test_an_unknown_dialog_halts_without_a_click(server: str, tmp_path: Any) -> None: + report = _run(server, "unknown", tmp_path) + assert report.success is False + assert report.results[0].interstitial_actions == [] + assert not any(seen.startswith("POST") for seen in _Server.seen) diff --git a/packages/flow/tests/test_write_fence.py b/packages/flow/tests/test_write_fence.py new file mode 100644 index 0000000000..46a3f0a0b5 --- /dev/null +++ b/packages/flow/tests/test_write_fence.py @@ -0,0 +1,131 @@ +"""The write fence blocks every write path and refuses when it can't see them. + +The fence is the save's wire check with no submission allowed +(``PlaywrightBackend.write_fence``); it guards interruption dismissals. + +A real browser and a local server: the server counts every non-``GET`` +request it receives, so a test can tell "blocked in the browser" from +"reached the server". +""" + +from __future__ import annotations + +import http.server +import threading +from typing import Any, Iterator + +import pytest + +pytest.importorskip("playwright.sync_api") + +PAGE = b""" + + + +
+ +
+""" + + +class _Server(http.server.BaseHTTPRequestHandler): + writes: list[str] = [] + + def do_GET(self) -> None: # noqa: N802 - http.server API + self.send_response(200) + self.send_header("content-type", "text/html") + self.end_headers() + self.wfile.write(PAGE) + + def do_POST(self) -> None: # noqa: N802 - http.server API + type(self).writes.append(self.path) + self.send_response(204) + self.end_headers() + + def log_message(self, *_: Any) -> None: + pass + + +@pytest.fixture() +def page_url() -> Iterator[str]: + _Server.writes = [] + server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _Server) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield f"http://127.0.0.1:{server.server_address[1]}/" + finally: + server.shutdown() + + +@pytest.fixture() +def backend(page_url: str) -> Iterator[Any]: + from openadapt_flow.backends.playwright_backend import PlaywrightBackend + + try: + backend, close = PlaywrightBackend.launch(page_url, headless=True) + except Exception as exc: # pragma: no cover - no browser installed + pytest.skip(f"no Playwright browser: {exc}") + try: + yield backend + finally: + close() + + +def _click(backend: Any, selector: str) -> None: + backend.page.click(selector) + + +def test_a_click_that_only_changes_the_page_holds_the_fence(backend: Any) -> None: + with backend.write_fence() as report: + assert report.refusal is None + _click(backend, "#local") + assert report.held + assert backend.page.inner_text("#out") == "ok" + + +@pytest.mark.parametrize( + ("selector", "attempt"), + [("#post", "POST"), ("#beacon", "POST"), ("#form", "POST"), ("#ws", "websocket")], +) +def test_every_write_path_is_blocked_and_reported( + backend: Any, selector: str, attempt: str +) -> None: + with backend.write_fence() as report: + assert report.refusal is None + _click(backend, selector) + assert not report.held + assert any(attempt.lower() in a.lower() for a in report.attempts), report.attempts + # Blocked in the browser: the server never saw a write. + assert _Server.writes == [] + + +def test_writes_pass_again_once_the_fence_is_down(backend: Any) -> None: + with backend.write_fence() as report: + _click(backend, "#local") + assert report.held + _click(backend, "#post") + backend.page.wait_for_timeout(300) + assert _Server.writes == ["/write"] + + +def test_a_page_loaded_before_the_guard_is_refused(page_url: str) -> None: + from playwright.sync_api import sync_playwright + + from openadapt_flow.backends.playwright_backend import PlaywrightBackend + + with sync_playwright() as pw: + try: + browser = pw.chromium.launch(headless=True) + except Exception as exc: # pragma: no cover - no browser installed + pytest.skip(f"no Playwright browser: {exc}") + page = browser.new_page() + page.goto(page_url) + backend = PlaywrightBackend(page) + with backend.write_fence() as report: + pass + # Its scripts ran before the guard and may hold the browser's own + # WebSocket: the fence can't vouch for them. + assert report.refusal is not None + assert not report.held + browser.close() From 4e99647a59ba5ce4d3552f20598be69eaf93fb13 Mon Sep 17 00:00:00 2001 From: abrichr Date: Wed, 4 Nov 2026 17:20:32 -0500 Subject: [PATCH 02/10] feat(flow): classify the whole interruption and check where a click lands An interruption class now matches on the whole dialog (full text, title, owning window), never its button word: a dialog mentioning a patient, a record, or saving/applying/changing anything is a business question and halts, as does a surface that can't read the whole dialog. Every web structural click arms a witness of where its trusted pointer events land; input that reached anything but the verified element (a popup that appeared mid-click) is delivery uncertainty, reconciled and never retried. --- packages/flow/docs/INTERRUPTION_RECOVERY.md | 54 ++++++- .../backends/playwright_backend.py | 136 +++++++++++++++++- packages/flow/openadapt_flow/interruptions.py | 122 ++++++++++++++++ packages/flow/openadapt_flow/ir.py | 13 +- .../flow/openadapt_flow/runtime/replayer.py | 28 +++- packages/flow/tests/test_interruptions.py | 73 +++++++++- .../flow/tests/test_interruptions_browser.py | 48 +++++++ 7 files changed, 464 insertions(+), 10 deletions(-) diff --git a/packages/flow/docs/INTERRUPTION_RECOVERY.md b/packages/flow/docs/INTERRUPTION_RECOVERY.md index 639c7af6b3..eec1f3e0c8 100644 --- a/packages/flow/docs/INTERRUPTION_RECOVERY.md +++ b/packages/flow/docs/INTERRUPTION_RECOVERY.md @@ -10,9 +10,25 @@ dismissal is provably read-only. Anything else stops the run as before. A run clears an interruption only when all four of these hold: -1. **It matches a class by identity.** Every text the class lists is on - screen, and the dismiss control is found by its role, accessible name, - selector, automation ID or window. Position is never used. +1. **It matches a class by identity, on the whole dialog.** Every text the + class lists is on screen, and the dismiss control is found by its role, + accessible name, selector, automation ID or window. Position is never + used. The button word never decides: the backend reads the whole + interruption (its full text, accessible title and owning window). The + run halts unless all of these hold: + - the dialog shows every class text; + - it carries the class's title and window, if the class declares them; + - it holds no business word (`BUSINESS_WORDS`: a patient, a record, a + chart, an order, or saving, applying, changing, discarding, deleting + or confirming anything); + - it says at most 240 characters beyond the class texts and the + control's label. + + "Keep my changes? [Yes]" and "Apply template to patient? [OK]" can look + like "Stay logged in? [OK]", but they never match a class. A backend + that can't read the whole dialog can't prove the class, so the run + halts. This applies to pixel surfaces, and to native desktop backends + until they implement `interruption_text`. 2. **The dismissal does not commit anything.** The dismissal is Escape, or a control whose label is in a closed set: close, dismiss, not now, maybe later, skip, got it, continue session, stay signed in, and similar @@ -40,6 +56,28 @@ A run clears an interruption only when all four of these hold: A run makes at most one dismissal per class per step, and never retries one. +## A popup that steals the click + +A toast or modal can appear after the last check and before the click +lands, so the input reaches the popup instead of the target. On the web, +every structural click is checked twice: + +- **Before the input.** Playwright's actionability trial and the + existing target and context checks refuse a covered target before any + input is sent. +- **After the input.** A witness armed just before the click records, + from the first moment the page sees each trusted pointer event, + whether it reached the verified element. If any part of the click + (press, release or click) reached something else, the outcome is + reported as delivery uncertainty (`InputLandedElsewhere`). The run then + stops and reconciles; it never retries. If the click navigated away, + the witness is gone with the old document, and the checks before the + input stand alone. + +The same check after dispatch on native desktop and RDP/Citrix backends +(the foreground window, the focused element, and a frame diff of the +target region) is not built yet. + ## The record Each dismissal is recorded in the step's `interstitial_actions` in the run @@ -95,6 +133,12 @@ real replayer and OCR, and a local synthetic app that saves with | "Update available" banner, "Close" | cleared, save lands | passes | | Session warning whose "Stay signed in" POSTs /logout | safety halt; the logout is stopped in the browser, nothing saved | passes | | Unknown dialog | halt, no click | passes | +| Session text plus "Apply template to patient?" behind "Stay signed in" | halt before any input | passes | +| A popup covers Save as the pointer arrives | refused before any input (pre-dispatch trial) | passes | +| A popup covers Save between press and release | delivery uncertainty `InputLandedElsewhere`, no retry, nothing written | passes | +| Dialog mentions "Keep my changes?" or "Unsaved record" | halt before any input | passes | +| Backend can't read the whole dialog | halt before any input | passes | +| Dialog says much more than the class texts; title mismatch | halt | passes | | Class whose control reads "OK", "Continue", "Accept", "Confirm" | refused at load | passes | | Class found only by position | refused at load | passes | | Desktop run: Escape, read-back unchanged | cleared (`record` watcher) | passes | @@ -113,4 +157,6 @@ real replayer and OCR, and a local synthetic app that saves with - Consent prompts (cookie "Accept" or "Reject") are decisions, not dismissals, so they stay outside the closed set. - Session re-authentication, a bounded re-resolve after late page loads, - and real-app runs on OpenEMR and Dolibarr are not covered here. + the native continuity check after dispatch, and real-app runs on + OpenEMR and Dolibarr (including behind a chaos proxy) are not covered + here. diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index d1f6dd1439..55a336daa9 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -19,7 +19,7 @@ import uuid from dataclasses import dataclass, field from datetime import date, datetime, timezone -from typing import TYPE_CHECKING, Any, Callable, Iterator, Literal, Optional +from typing import TYPE_CHECKING, Any, Callable, Iterator, Literal, Optional, Sequence from urllib.parse import urlsplit from openadapt_flow.backends._page_quiescence import PageQuiescence @@ -610,6 +610,70 @@ class _FramePoint: # own visible box on purpose, so the bind guard refusing it is not occlusion. _TRANSPARENT_JS = "el => Number(window.getComputedStyle(el).opacity || '1') === 0" +#: Records, from the first moment the window sees it, which element each +#: trusted pointer event of a click reached. Read back right after the click +#: (:meth:`PlaywrightBackend.act_structural`): input that reached anything but +#: the verified element (a toast or modal that appeared between the check and +#: the click) is reported, never retried. +_ARM_DISPATCH_WITNESS_JS = """(el, key) => { + const w = el.ownerDocument.defaultView; + const seen = []; + const types = ['pointerdown', 'mousedown', 'pointerup', 'mouseup', 'click']; + const f = e => { + if (!e.isTrusted) return; + const path = typeof e.composedPath === 'function' ? e.composedPath() : []; + seen.push({type: e.type, hit: path.includes(el) || el.contains(e.target)}); + }; + for (const t of types) w.addEventListener(t, f, true); + Object.defineProperty(w, key, {configurable: true, value: () => { + for (const t of types) w.removeEventListener(t, f, true); + delete w[key]; + return seen; + }}); +}""" + +#: The whole interruption a dismiss control belongs to: its container's full +#: text, accessible title and the page's title, so a class is matched on the +#: whole dialog, never on its button word (openadapt_flow.interruptions). +_INTERRUPTION_TEXT_JS = """(el, texts) => { + const DIALOG = '[role=dialog],[role=alertdialog],[role=alert],[role=status],' + + '[aria-modal=true],dialog'; + const visible = n => { + const r = n.getBoundingClientRect(); + const cs = n.ownerDocument.defaultView.getComputedStyle(n); + return r.width > 0 && r.height > 0 && cs.visibility !== 'hidden' && + cs.display !== 'none'; + }; + const floating = n => { + for (let a = n; a && a.nodeType === 1; a = a.parentElement) { + if (a.matches(DIALOG)) return a; + const pos = a.ownerDocument.defaultView.getComputedStyle(a).position; + if (pos === 'fixed' || pos === 'sticky') return a; + } + return null; + }; + let box = null; + if (el) { + box = floating(el); + } else { + const all = [...document.querySelectorAll(DIALOG)].filter(visible) + .filter(n => texts.every(t => (n.innerText || '').includes(t))); + const outer = all.filter(n => !all.some(m => m !== n && m.contains(n))); + if (outer.length === 1) box = outer[0]; + } + if (!box) return null; + let title = box.getAttribute('aria-label') || ''; + const by = box.getAttribute('aria-labelledby'); + if (!title && by) { + title = by.split(/\\s+/).map(i => { + const t = box.ownerDocument.getElementById(i); + return t ? t.innerText : ''; + }).join(' '); + } + return {text: box.innerText || '', title: title.trim(), + window: box.ownerDocument.title || ''}; +}""" + _BIND_STRUCTURAL_TARGET_JS = ( "(el, args) => { const describe = " + _DESCRIBE_TARGET_JS @@ -2971,6 +3035,61 @@ def _disarm_wire_check(self) -> list[str]: self._form_wire = None return reasons + def _check_dispatch_witness( + self, scope: Any, witness: str, fingerprint: str, *, double: bool + ) -> None: + """Raise when the click's input reached anything but its target. + + A toast or modal can appear between the last check and the click, so + the input lands on it. That is a possible unintended action: it is + reported as delivery uncertainty (the runtime reconciles, never + retries). When the page navigated away on the click, the witness is + gone with it; the pre-dispatch checks then stand alone, as before. + """ + + try: + seen = scope.evaluate( + "key => { const r = window[key]; return r ? r() : null; }", witness + ) + except Exception: # noqa: BLE001 - the document went away on the click + return + if seen is None: + return # a new document: the click navigated + if not seen or not all(bool(e.get("hit")) for e in seen): + raise ActionDeliveryUncertain( + operation="dom_double_click" if double else "dom_click", + native=False, + target_fingerprint=fingerprint, + cause_type="InputLandedElsewhere", + ) + + def interruption_text( + self, locator: Optional[StructuralLocator], texts: Sequence[str] + ) -> Optional[dict[str, str]]: + """The whole interruption: ``{text, title, window}`` of the dialog, + banner or toast that holds ``locator``'s control, or, with no + locator, of the one visible dialog that shows every one of + ``texts``. None when there is no such single container.""" + + try: + if locator is None: + found = self.page.evaluate( + f"texts => ({_INTERRUPTION_TEXT_JS})(null, texts)", list(texts) + ) + else: + resolved = self._locator_with_scope(locator) + if resolved is None: + return None + _scope, loc = resolved + if loc.count() != 1: + return None + found = loc.evaluate(_INTERRUPTION_TEXT_JS, list(texts)) + except Exception: # noqa: BLE001 - unreadable means unproven + return None + if not isinstance(found, dict): + return None + return {k: str(found.get(k) or "") for k in ("text", "title", "window")} + @contextlib.contextmanager def write_fence(self) -> Iterator["WriteFenceReport"]: """Run the ``with`` body while nothing on the page can write. @@ -3644,6 +3763,20 @@ def act_structural( self._assert_qualification_environment_current() close_expected = self._take_page_close_expectation() open_before = not self._page_is_closed() + witness = "__oaflow_witness_" + uuid.uuid4().hex + try: + armed = guard.scope.evaluate( + "([sel, key]) => { const el = document.querySelector(sel); " + f"if (!el) return false; ({_ARM_DISPATCH_WITNESS_JS})(el, key); " + "return true; }", + [f"[{self._token_attribute(guard.token)}]", witness], + ) + if armed is not True: + raise RuntimeError("the guarded target is gone") + except Exception as exc: + raise StructuralResolutionRefused( + "could not arm the check of where the click lands" + ) from exc try: if double: token_locator.dblclick(timeout=1000) @@ -3666,6 +3799,7 @@ def act_structural( target_fingerprint=fingerprint, cause_type=type(exc).__name__, ) from exc + self._check_dispatch_witness(scope, witness, fingerprint, double=double) except ActionDeliveryUncertain: raise except StructuralResolutionRefused: diff --git a/packages/flow/openadapt_flow/interruptions.py b/packages/flow/openadapt_flow/interruptions.py index 473954093e..0af3f0678d 100644 --- a/packages/flow/openadapt_flow/interruptions.py +++ b/packages/flow/openadapt_flow/interruptions.py @@ -17,6 +17,12 @@ (:mod:`openadapt_flow.runtime.effect_watch`): the browser's write fence and the system-of-record read-back. A watcher that sees a write, or a run with none, halts. + The whole interruption is classified, never its button word: the + backend reads the full dialog (text, title, owning window), and one + that mentions a patient, a record, or saving, applying or changing + anything (:data:`BUSINESS_WORDS`) is never benign. A backend that + can't read the whole dialog (a pixel surface) can't prove the class, + so the run halts. d. **The expected screen comes back.** The class's text is gone, and the step then goes through its normal identity check; a screen that isn't the one the step expects halts there. @@ -55,6 +61,106 @@ ) +#: Words that make an interruption a business question, never a benign one: +#: it mentions record content, a patient, or saving, applying or changing +#: something ("Keep my changes?", "Apply template to patient?"). A dialog +#: whose whole text has one is never cleared automatically, whatever its +#: button says. +BUSINESS_WORDS = re.compile( + r"\b(?:patients?|records?|charts?|encounters?|orders?|prescri\w*|" + r"medications?|claims?|invoices?|payments?|transfers?|templates?|" + r"sav(?:e|es|ed|ing)|appl(?:y|ies|ied|ying)|chang(?:e|es|ed|ing)|" + r"discard\w*|delet\w*|remov\w*|submit\w*|approv\w*|confirm\w*|" + r"overwrit\w*|replac\w*|unsaved|undo|revert\w*)\b", + re.IGNORECASE, +) + +#: Text a class's whole dialog may hold beyond its declared texts and its +#: controls' labels: a short system message, nothing more. +MAX_RESIDUAL_CHARS = 240 + + +def business_words(text: str) -> list[str]: + """The business words in ``text`` (see :data:`BUSINESS_WORDS`).""" + + return sorted({m.group(0).lower() for m in BUSINESS_WORDS.finditer(text)}) + + +def class_texts(interstitial: "Interstitial") -> list[str]: + """The texts a class is detected by (its ``TEXT_PRESENT`` operands).""" + + from openadapt_flow.ir import PredicateKind + + out: list[str] = [] + + def walk(pred: Any) -> None: + if pred.kind is PredicateKind.TEXT_PRESENT and pred.text: + out.append(pred.text) + elif pred.kind is PredicateKind.AND: + for op in pred.operands: + walk(op) + + walk(interstitial.detect) + return out + + +def _squash(text: str) -> str: + return " ".join(text.split()).casefold() + + +def whole_interruption_error( + interstitial: "Interstitial", found: Optional[Mapping[str, str]] +) -> Optional[str]: + """Why the interruption on screen is not this class, judged on the whole + dialog (``found``: its ``text``, ``title`` and ``window`` from the + backend's ``interruption_text``), or None when it is. + + The button word never decides. The dialog must show every class text, + carry the class's title and window when the class declares them, hold + no business word anywhere, and say little beyond the class texts. + """ + + if found is None: + return ( + "the whole interruption could not be read (its dialog, banner or " + "toast, with its title and window), so its class can't be proven" + ) + text = _squash(found.get("text") or "") + title = _squash(found.get("title") or "") + window = (found.get("window") or "").strip() + texts = class_texts(interstitial) + missing = [t for t in texts if _squash(t) not in text] + if missing: + return f"the interruption does not show the class text {missing}" + words = business_words(" ".join((text, title))) + if words: + return ( + f"the interruption mentions {', '.join(words)}: it is a business " + "question, not a benign interruption" + ) + if interstitial.dialog_title and _squash(interstitial.dialog_title) != title: + return f"the interruption's title is {title!r}, not the class's" + anchor = interstitial.dismiss_anchor + want_window = ( + anchor.structural.window_name + if anchor is not None and anchor.structural is not None + else None + ) + if want_window and want_window.strip() != window: + return "the interruption belongs to a different window than the class's" + residual = text + labels = [*texts, dismiss_label(interstitial) or ""] + for part in sorted({_squash(p) for p in labels if p}, key=len, reverse=True): + residual = residual.replace(part, " ") + residual = " ".join(residual.split()) + if len(residual) > MAX_RESIDUAL_CHARS: + return ( + f"the interruption says {len(residual)} characters beyond the " + "class's texts, more than a system message" + ) + return None + + def is_non_committing_label(label: Optional[str]) -> bool: """Whether ``label`` is one of the closed non-committing dismiss labels.""" @@ -100,6 +206,13 @@ def interruption_class_error(interstitial: "Interstitial") -> Optional[str]: "an interruption class must be detected by its text (TEXT_PRESENT), " "not only by how it looks or where it is" ) + words = business_words(" ".join(class_texts(interstitial))) + words += business_words(interstitial.dialog_title or "") + if words: + return ( + f"an interruption class can't mention {', '.join(sorted(set(words)))}: " + "that is a business question, not a benign interruption" + ) if interstitial.dismiss_key is not None: return None # Escape only (Interstitial's own validation) label = dismiss_label(interstitial) @@ -180,6 +293,7 @@ def interruption_from_data(entry: Mapping[str, Any]) -> "Interstitial": else Predicate(kind=PredicateKind.AND, operands=present) ) gone = str(entry.get("gone") or texts[0]) + title = entry.get("title") try: return Interstitial( name=name, @@ -190,6 +304,7 @@ def interruption_from_data(entry: Mapping[str, Any]) -> "Interstitial": consequential=False, clearance=Predicate(kind=PredicateKind.TEXT_ABSENT, text=gone), effect_watch=True, + dialog_title=str(title) if title else None, ) except Exception as exc: raise InterruptionClassError(f"{name}: {exc}") from exc @@ -221,6 +336,8 @@ def interruption_to_data(interstitial: "Interstitial") -> dict[str, Any]: "window": s.window_name, } entry["dismiss"] = {"control": {k: v for k, v in control.items() if v}} + if interstitial.dialog_title: + entry["title"] = interstitial.dialog_title clearance = interstitial.clearance if ( clearance is not None @@ -335,6 +452,11 @@ def learn_interruption( anchor = interstitial.dismiss_anchor assert anchor is not None and anchor.structural is not None locator: StructuralLocator = anchor.structural + reader = getattr(backend, "interruption_text", None) + whole = reader(locator, list(texts)) if callable(reader) else None + error = whole_interruption_error(interstitial, whole) + if error is not None: + raise LearnRefused(error) handle = locate(locator) if handle is None: raise LearnRefused(f"{dismiss_label(interstitial)!r} did not resolve uniquely") diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index 0a3f0ebad5..afe4fa0f3f 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -1412,14 +1412,21 @@ class Interstitial(BaseModel): "non-committing one (close, dismiss, not now, continue session).", ) + dialog_title: Optional[str] = Field( + default=None, + description="The interruption's accessible title, when an " + "interruption class declares it; the whole dialog must carry it.", + ) + @model_serializer(mode="wrap") def _serialize_compatible(self, handler: Any) -> dict[str, Any]: - """Leave ``effect_watch`` out when unset, so every existing + """Leave ``effect_watch`` and ``dialog_title`` out when unset, so every existing interstitial (and bundle digest) serializes exactly as before.""" data: dict[str, Any] = handler(self) - if self.effect_watch is None: - data.pop("effect_watch", None) + for key in ("effect_watch", "dialog_title"): + if data.get(key) is None: + data.pop(key, None) return data @model_validator(mode="after") diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index 365dc78fa2..548356d1a0 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -97,7 +97,11 @@ normalize_signal_text, parameterize_identity_text, ) -from openadapt_flow.interruptions import dismiss_label +from openadapt_flow.interruptions import ( + class_texts, + dismiss_label, + whole_interruption_error, +) from openadapt_flow.ir import ( ActionDeliveryUncertainty, ActionKind, @@ -15115,6 +15119,28 @@ def _handle_interstitials( mutation_error = self._interstitial_declaration_mutation(workflow) if mutation_error is not None: return before_png, mutation_error + if it.effect_watch: + # Classify the whole interruption, never its button word: a + # business question dressed as a session prompt halts here, + # before any input. + reader = getattr(self.backend, "interruption_text", None) + locator = ( + it.dismiss_anchor.structural + if it.dismiss_anchor is not None + else None + ) + try: + whole = ( + reader(locator, class_texts(it)) if callable(reader) else None + ) + except Exception: # noqa: BLE001 - unreadable means unproven + whole = None + class_error = whole_interruption_error(it, whole) + if class_error is not None: + return before_png, ( + f"Interruption '{it.name}' was not cleared: {class_error} " + "- refusing to act beneath it; run aborted." + ) # Append BEFORE delivery: a backend exception can never create an # unreported key/click attempt. The event carries the exact admitted # policy and expected visual outcome. diff --git a/packages/flow/tests/test_interruptions.py b/packages/flow/tests/test_interruptions.py index 79117ba46c..5d8fd800ac 100644 --- a/packages/flow/tests/test_interruptions.py +++ b/packages/flow/tests/test_interruptions.py @@ -66,8 +66,18 @@ def __init__(self, refusal=None, attempts=()): class WatchedBackend(FakeBackend): """A structural backend with a scripted write fence.""" - def __init__(self, *, fence=True, refusal=None, writes=()): + def __init__( + self, + *, + fence=True, + refusal=None, + writes=(), + whole="Your session will expire in 5 minutes. Stay signed in", + title="", + ): super().__init__() + self._whole = whole + self._title = title self._fence = fence self._refusal = refusal self._writes = list(writes) @@ -76,6 +86,11 @@ def __init__(self, *, fence=True, refusal=None, writes=()): if not fence: self.write_fence = None # type: ignore[assignment] + def interruption_text(self, locator, texts): + if self._whole is None: + return None + return {"text": self._whole, "title": self._title, "window": ""} + def locate_structural(self, locator): return StructuralHandle( point=(15, 15), region=(10, 10, 10, 10), target_fingerprint="a" * 64 @@ -383,3 +398,59 @@ def test_watch_needs_a_watcher(): with watch_effects(object(), None) as report: assert report.refusal is not None assert not report.held + + +# -- the whole dialog decides, never the button word ------------------------------ + + +@pytest.mark.parametrize( + "whole", + [ + "Your session will expire. Apply template to patient? Stay signed in", + "Your session will expire. Keep my changes? Stay signed in", + "Your session will expire. Unsaved record will be discarded. Stay signed in", + ], +) +def test_a_business_question_in_disguise_halts_before_any_input(bundle, run_dir, whole): + backend = WatchedBackend(whole=whole) + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + assert report.success is False + assert "business question" in (report.results[0].error or "") + assert backend.actions == [] + + +def test_a_surface_that_cannot_read_the_whole_dialog_halts(bundle, run_dir): + backend = WatchedBackend(whole=None) + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + assert report.success is False + assert "can't be proven" in (report.results[0].error or "") + assert backend.actions == [] + + +def test_a_dialog_with_much_more_to_say_is_not_the_class(bundle, run_dir): + backend = WatchedBackend( + whole="Your session will expire. " + "Lorem ipsum dolor sit amet. " * 12 + ) + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + assert report.success is False + assert backend.actions == [] + + +def test_a_declared_title_must_match(bundle, run_dir): + entry = dict(SESSION, title="Session timeout") + backend = WatchedBackend(title="Unsaved changes") + report = _run(backend, _vision(), bundle, run_dir, [interruption_from_data(entry)]) + assert report.success is False + assert backend.actions == [] + + +def test_a_class_that_mentions_business_content_is_refused(): + entry = dict(SESSION, texts=["Apply template to patient"]) + with pytest.raises(InterruptionClassError, match="business question"): + interruption_from_data(entry) diff --git a/packages/flow/tests/test_interruptions_browser.py b/packages/flow/tests/test_interruptions_browser.py index 0229153b74..728fb1f3da 100644 --- a/packages/flow/tests/test_interruptions_browser.py +++ b/packages/flow/tests/test_interruptions_browser.py @@ -13,6 +13,9 @@ session "Stay signed in" removes the dialog (GET ping) saved banner "Close" removes an "Update available" banner saved committing "Stay signed in" also POSTs /logout halt, no write +disguised session text + "Apply template to patient?" halt, no click +steal-hover a popup covers Save as the pointer arrives halt, no write +steal-press a popup covers Save between press and release halt, no write unknown an unlisted dialog halt, no click ============= =============================================== ============ """ @@ -63,6 +66,21 @@ } else if (kind === 'committing') { dialog('Your session will expire in 5 minutes', 'Stay signed in', () => fetch('/logout', {method: 'POST'})); +} else if (kind === 'disguised') { + dialog('Your session will expire in 5 minutes. Apply template to patient?', + 'Stay signed in', () => fetch('/apply', {method: 'POST'})); +} else if (kind === 'steal-hover' || kind === 'steal-press') { + // A popup that appears while the click is under way: after every check, + // as the pointer arrives (hover) or between press and release (press). + const evt = kind === 'steal-hover' ? 'pointerover' : 'pointerdown'; + document.getElementById('save').addEventListener(evt, () => { + const p = document.createElement('div'); + p.style = 'position:fixed;inset:0;background:#fee;font-size:28px'; + p.innerHTML = 'New message '; + p.querySelector('button').onclick = () => fetch('/accept', {method: 'POST'}); + p.onclick = () => fetch('/popup-click', {method: 'POST'}); + document.body.appendChild(p); + }, {once: true}); } else if (kind === 'unknown') { dialog('Unexpected error 42 occurred', 'Close', () => {}); } else if (kind === 'banner') { @@ -193,3 +211,33 @@ def test_an_unknown_dialog_halts_without_a_click(server: str, tmp_path: Any) -> assert report.success is False assert report.results[0].interstitial_actions == [] assert not any(seen.startswith("POST") for seen in _Server.seen) + + +def test_a_business_question_in_disguise_halts_without_a_click( + server: str, tmp_path: Any +) -> None: + report = _run(server, "disguised", tmp_path) + assert report.success is False + assert "business question" in (report.results[0].error or "") + assert report.results[0].interstitial_actions == [] + assert not any(seen.startswith("POST") for seen in _Server.seen) + + +@pytest.mark.parametrize("kind", ["steal-hover", "steal-press"]) +def test_a_popup_that_steals_the_click_halts_and_nothing_is_written( + server: str, kind: str, tmp_path: Any +) -> None: + report = _run(server, kind, tmp_path) + assert report.success is False, _Server.seen + result = report.results[0] + assert result.safety_halt is True + if kind == "steal-press": + # Pressed on Save, released on the popup: the click's witness saw + # the input land elsewhere. Reconcile, never retry. + assert result.delivery_uncertainty is not None + assert result.delivery_uncertainty.cause_type == "InputLandedElsewhere" + else: + # Covered as the pointer arrived: refused before any input. + assert "pre-dispatch" in (result.error or "") + # Neither the save nor anything on the popup went through. + assert not any(seen.startswith("POST") for seen in _Server.seen), _Server.seen From 389f10eeb607972332be4096a80fae5ef4759429 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 8 Nov 2026 20:50:59 -0500 Subject: [PATCH 03/10] fix(flow): check the clicked control is the declared dismiss control An interruption class checked the declared name against the closed non-committing set, but a selector or automation id resolves first and may point at any control. A class declaring "Close" whose selector found "Sign out" was cleared. Now a control class must declare role and accessible name, and before the click (and again at the last no-input boundary) the backend must show the bound element is the one control with that role and exact name (structural_control_is, Playwright get_by_role exact). A backend that can't say, a non-structural resolution, or any error halts. Learning applies the same check before its click. --- packages/flow/CHANGELOG.md | 2 +- packages/flow/docs/INTERRUPTION_RECOVERY.md | 11 +++ .../backends/playwright_backend.py | 36 +++++++++ packages/flow/openadapt_flow/interruptions.py | 73 ++++++++++++++++- .../flow/openadapt_flow/runtime/replayer.py | 27 +++++++ packages/flow/tests/test_interruptions.py | 78 +++++++++++++++++++ .../flow/tests/test_interruptions_browser.py | 30 ++++++- 7 files changed, 253 insertions(+), 4 deletions(-) diff --git a/packages/flow/CHANGELOG.md b/packages/flow/CHANGELOG.md index 4e7a84a3b4..03ccae0bc5 100644 --- a/packages/flow/CHANGELOG.md +++ b/packages/flow/CHANGELOG.md @@ -23,7 +23,7 @@ ### Features -- Interruption classes: a run clears a known interruption, such as a session warning, a banner or a tour, by itself, but only when the dismissal is provably read-only. The class must match by its text and control, the control's label must be in a closed non-committing set, and no write watcher may see a write. On the web the watcher is the browser write fence; on any backend, including desktop and RDP, it is the system-of-record read-back. Afterwards the step's normal identity check must pass. Classes are per-app data (`runtime.interruptions_path`) or learned from a halt (`learn_interruption`). Each dismissal is in the run report. See `docs/INTERRUPTION_RECOVERY.md`. +- Interruption classes: a run clears a known interruption, such as a session warning, a banner or a tour, by itself, but only when the dismissal is provably read-only. The class must match by its text and control, the control's label must be in a closed non-committing set and be the live label of the control that is clicked, and no write watcher may see a write. On the web the watcher is the browser write fence; on any backend, including desktop and RDP, it is the system-of-record read-back. Afterwards the step's normal identity check must pass. Classes are per-app data (`runtime.interruptions_path`) or learned from a halt (`learn_interruption`). Each dismissal is in the run report. See `docs/INTERRUPTION_RECOVERY.md`. ### Fixes diff --git a/packages/flow/docs/INTERRUPTION_RECOVERY.md b/packages/flow/docs/INTERRUPTION_RECOVERY.md index eec1f3e0c8..bcd9bbca29 100644 --- a/packages/flow/docs/INTERRUPTION_RECOVERY.md +++ b/packages/flow/docs/INTERRUPTION_RECOVERY.md @@ -35,6 +35,17 @@ A run clears an interruption only when all four of these hold: (`NON_COMMITTING_LABEL` in `openadapt_flow/interruptions.py`). "OK", "Continue", "Yes", "Accept" and any write word are outside the set, so a class that uses one is refused when it loads. + + The label checked is the label of the control that gets clicked. A + control class declares its role and accessible name, and a class without + both is refused when it loads. A selector or automation id may resolve to + any control, so before the click, and again at the last moment before + input, the backend must show that the resolved element is the one + control with the declared role and exact accessible name. If the class + says "Close" and its selector finds "Sign out", the run halts with no + click. So does a backend that can't read the live control + (`structural_control_is`), which today means every backend except the + browser. 3. **No watcher sees a write.** The dismissal runs under every write watcher the run has (`openadapt_flow/runtime/effect_watch.py`): - in a browser, the write fence. It is the same hold that guards a diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index 284be5b725..a2427ccf75 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -3479,6 +3479,42 @@ def interruption_text( return None return {k: str(found.get(k) or "") for k in ("text", "title", "window")} + def structural_control_is( + self, + locator: StructuralLocator, + handle: StructuralHandle, + *, + role: str, + name: str, + ) -> bool: + """Whether the element ``handle`` is bound to is the one control in + its scope with ARIA ``role`` and exact accessible ``name``. + + Uses the browser's own accessible-name computation (Playwright's + ``get_by_role(..., exact=True)``), so a selector that resolves to a + control named something else is told apart from the declared one. + Does not consume the handle. Any doubt (a stale handle, no match, more + than one, a different element, an error) is False. + """ + + try: + guard = self._structural_tokens.get(handle.target_fingerprint or "") + if guard is None or tuple(locator.frame_path or ()) != guard.frame_path: + return False + bound = self._token_locator(guard.token, guard.scope) + named = guard.scope.get_by_role(role, name=name, exact=True) + if bound.count() != 1 or named.count() != 1: + return False + other = named.element_handle(timeout=1000) + if other is None: + return False + try: + return bool(bound.evaluate("(el, other) => el === other", other)) + finally: + other.dispose() + except Exception: # noqa: BLE001 - unreadable means unproven + return False + @contextlib.contextmanager def write_fence(self) -> Iterator["WriteFenceReport"]: """Run the ``with`` body while nothing on the page can write. diff --git a/packages/flow/openadapt_flow/interruptions.py b/packages/flow/openadapt_flow/interruptions.py index 0af3f0678d..d59acc57ec 100644 --- a/packages/flow/openadapt_flow/interruptions.py +++ b/packages/flow/openadapt_flow/interruptions.py @@ -12,7 +12,10 @@ b. **A non-committing action.** The dismissal is Escape or a control whose label is in a closed set (:data:`NON_COMMITTING_LABEL`): close, dismiss, not now, continue session, and the like. "OK", "Continue", "Yes", - "Accept" and any write word are outside it. + "Accept" and any write word are outside it. The label is the live + control's, not only the declared one: before the click, the backend must + show that the element the selector or id resolved to is the one control + with the declared role and accessible name (:func:`live_control_error`). c. **No write.** The dismissal runs under the run's write watchers (:mod:`openadapt_flow.runtime.effect_watch`): the browser's write fence and the system-of-record read-back. A watcher that sees a write, or a run @@ -234,6 +237,71 @@ def interruption_class_error(interstitial: "Interstitial") -> Optional[str]: ) if not has_identity: return "the dismiss control must be found by its identity, not its position" + if ( + structural is None + or not (structural.role or "").strip() + or not (structural.name or "").strip() + ): + # The label that is checked must be the label of the control that is + # clicked. Only a declared role and accessible name can be compared + # with the live control (:func:`live_control_error`); an OCR text or + # a template can't. + return ( + "the dismiss control must declare its structural role and " + "accessible name, so its identity can be checked on the live " + "control before the click" + ) + return None + + +def live_control_error( + interstitial: "Interstitial", backend: Any, handle: Any +) -> Optional[str]: + """Why the control ``handle`` resolved to is not the declared dismiss + control, or None when it is. + + The closed-set label check (:func:`interruption_class_error`) is made on + the *declared* name. A selector or automation id resolves first and may + point at any control, so the declared name proves nothing until the + live control is shown to carry it: the backend's + ``structural_control_is(locator, handle, role=, name=)`` must confirm that + the very element bound by ``handle`` is the one control in its scope with + the declared role and exact accessible name. A backend that can't say, + a missing handle, or any error halts before the click. + """ + + anchor = interstitial.dismiss_anchor + if anchor is None: + return None # a key dismissal (Escape only) has no control + locator = anchor.structural + role = (locator.role or "").strip() if locator is not None else "" + name = (locator.name or "").strip() if locator is not None else "" + if locator is None or not role or not name: + return "the dismiss control declares no role and name to check" + if not is_non_committing_label(name): + return f"the dismiss control {name!r} is not a non-committing label" + if handle is None: + return ( + "the dismiss control was not bound to one live element, so its " + "label can't be checked" + ) + check = getattr(backend, "structural_control_is", None) + if not callable(check): + return ( + "this backend can't read the live control's role and name, so " + "the control that would be clicked can't be shown to be " + f"{name!r}" + ) + try: + same = check(locator, handle, role=role, name=name) is True + except Exception: # noqa: BLE001 - unreadable means unproven + same = False + if not same: + return ( + f"the control that would be clicked is not the {role} {name!r} the " + "class declares (its selector or id resolves to a different " + "control)" + ) return None @@ -460,6 +528,9 @@ def learn_interruption( handle = locate(locator) if handle is None: raise LearnRefused(f"{dismiss_label(interstitial)!r} did not resolve uniquely") + control_error = live_control_error(interstitial, backend, handle) + if control_error is not None: + raise LearnRefused(control_error) with watch_effects(backend, effect_verifier) as report: if report.refusal is not None: raise LearnRefused(f"the dismissal can't be watched: {report.refusal}") diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index a90df5aad8..1a87caafe3 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -101,6 +101,7 @@ from openadapt_flow.interruptions import ( class_texts, dismiss_label, + live_control_error, whole_interruption_error, ) from openadapt_flow.ir import ( @@ -14617,6 +14618,20 @@ def _canonical_interstitial_snapshot( ) return validated, canonical + def _interruption_control_error( + self, it: Interstitial, resolution: Optional[Resolution] + ) -> Optional[str]: + """Why an interruption class's resolved dismiss control can't be shown + to be the declared one, or None. Only a structural resolution, bound + to one live element, can be checked; anything else halts.""" + + if resolution is None or resolution.rung != "structural": + return ( + "the dismiss control did not resolve structurally, so the " + "control that would be clicked can't be checked" + ) + return live_control_error(it, self.backend, resolution.structural_handle) + def _interstitial_declaration_mutation( self, workflow: Optional[Workflow] ) -> Optional[str]: @@ -15251,6 +15266,10 @@ def _handle_interstitials( except Exception: # noqa: BLE001 - unreadable means unproven whole = None class_error = whole_interruption_error(it, whole) + if class_error is None and it.dismiss_anchor is not None: + # The declared label proves nothing until the control + # that will be clicked is shown to carry it. + class_error = self._interruption_control_error(it, resolution) if class_error is not None: return before_png, ( f"Interruption '{it.name}' was not cleared: {class_error} " @@ -15344,6 +15363,14 @@ def _record_watch() -> None: "structural dismissal cannot be delivered " "natively without a stable element handle" ) + if it.effect_watch: + # Again at the last no-input boundary: the + # label may have changed since the check. + control_error = self._interruption_control_error( + it, resolution + ) + if control_error is not None: + raise StructuralResolutionRefused(control_error) self._require_qualification_environment_current() self._deliver_backend_call( result, diff --git a/packages/flow/tests/test_interruptions.py b/packages/flow/tests/test_interruptions.py index 5d8fd800ac..6abd82c1e1 100644 --- a/packages/flow/tests/test_interruptions.py +++ b/packages/flow/tests/test_interruptions.py @@ -74,8 +74,15 @@ def __init__( writes=(), whole="Your session will expire in 5 minutes. Stay signed in", title="", + live=None, + can_read_live=True, ): super().__init__() + # The live control's (role, name); None means the declared one. + self._live = live + self.live_checks: list[tuple[str, str]] = [] + if not can_read_live: + self.structural_control_is = None # type: ignore[assignment] self._whole = whole self._title = title self._fence = fence @@ -91,6 +98,11 @@ def interruption_text(self, locator, texts): return None return {"text": self._whole, "title": self._title, "window": ""} + def structural_control_is(self, locator, handle, *, role, name): + self.live_checks.append((role, name)) + live = self._live or (locator.role, locator.name) + return tuple(live) == (role, name) + def locate_structural(self, locator): return StructuralHandle( point=(15, 15), region=(10, 10, 10, 10), target_fingerprint="a" * 64 @@ -454,3 +466,69 @@ def test_a_class_that_mentions_business_content_is_refused(): entry = dict(SESSION, texts=["Apply template to patient"]) with pytest.raises(InterruptionClassError, match="business question"): interruption_from_data(entry) + + +# -- the clicked control is the declared one -------------------------------------- + + +def test_a_selector_that_resolves_to_another_control_halts_before_any_input( + bundle, run_dir +): + # Declared "Close", but the selector resolves to a "Sign out" button. + entry = dict( + SESSION, + dismiss={"control": {"selector": "#danger", "role": "button", "name": "Close"}}, + ) + backend = WatchedBackend( + whole="Your session will expire in 5 minutes. Sign out", + live=("button", "Sign out"), + ) + report = _run(backend, _vision(), bundle, run_dir, [interruption_from_data(entry)]) + assert report.success is False + assert "not the button 'Close'" in (report.results[0].error or "") + assert report.results[0].interstitial_actions == [] + assert backend.actions == [] + assert backend.live_checks == [("button", "Close")] + + +def test_a_backend_that_cannot_read_the_live_control_halts(bundle, run_dir): + backend = WatchedBackend(can_read_live=False) + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + assert report.success is False + assert "can't read the live control" in (report.results[0].error or "") + assert backend.actions == [] + + +def test_the_live_control_is_checked_again_at_delivery(bundle, run_dir): + # The label changes between the first check and the click. + backend = WatchedBackend() + answers = iter([True, False]) + backend.structural_control_is = ( # type: ignore[method-assign] + lambda locator, handle, *, role, name: next(answers) + ) + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + assert report.success is False + assert not any(a[0] == "act_structural" for a in backend.actions) + + +def test_a_class_without_a_declared_role_and_name_is_refused(): + entry = dict(SESSION, dismiss={"control": {"selector": "#x", "name": "Close"}}) + with pytest.raises(InterruptionClassError, match="role and accessible name"): + interruption_from_data(entry) + + +def test_learning_refuses_a_control_whose_live_label_differs_without_clicking(): + backend = WatchedBackend(live=("button", "Sign out")) + with pytest.raises(LearnRefused, match="not the button 'Close'"): + learn_interruption( + name="n", + texts=["t"], + control={"selector": "#danger", "role": "button", "name": "Close"}, + backend=backend, + shows=lambda text: True, + ) + assert backend.actions == [] diff --git a/packages/flow/tests/test_interruptions_browser.py b/packages/flow/tests/test_interruptions_browser.py index 728fb1f3da..77e2d0cba3 100644 --- a/packages/flow/tests/test_interruptions_browser.py +++ b/packages/flow/tests/test_interruptions_browser.py @@ -17,6 +17,7 @@ steal-hover a popup covers Save as the pointer arrives halt, no write steal-press a popup covers Save between press and release halt, no write unknown an unlisted dialog halt, no click +mislabeled class says "Close", its selector is "Sign out" halt, no click ============= =============================================== ============ """ @@ -83,6 +84,9 @@ }, {once: true}); } else if (kind === 'unknown') { dialog('Unexpected error 42 occurred', 'Close', () => {}); +} else if (kind === 'mislabeled') { + dialog('Update available now', 'Sign out', () => fetch('/logout-everywhere')); + document.querySelector('[role=dialog] button').id = 'danger'; } else if (kind === 'banner') { const b = document.createElement('div'); b.style = 'position:fixed;top:0;left:0;right:0;height:120px;background:#ffd;' @@ -143,7 +147,7 @@ def server() -> Iterator[str]: srv.shutdown() -def _run(url: str, kind: str, tmp_path: Any) -> Any: +def _run(url: str, kind: str, tmp_path: Any, classes: Any = None) -> Any: from openadapt_flow.backends.playwright_backend import PlaywrightBackend from openadapt_flow.runtime.replayer import Replayer @@ -168,7 +172,7 @@ def _run(url: str, kind: str, tmp_path: Any) -> Any: risk="reversible", ) ], - interstitials=CLASSES, + interstitials=CLASSES if classes is None else classes, ) bundle = tmp_path / "bundle" bundle.mkdir() @@ -241,3 +245,25 @@ def test_a_popup_that_steals_the_click_halts_and_nothing_is_written( assert "pre-dispatch" in (result.error or "") # Neither the save nor anything on the popup went through. assert not any(seen.startswith("POST") for seen in _Server.seen), _Server.seen + + +def test_a_selector_that_points_at_another_control_halts_without_a_click( + server: str, tmp_path: Any +) -> None: + # The class declares a "Close" button, but its selector resolves to a + # "Sign out" button whose handler signs out with a GET the fence passes. + mislabeled = interruption_from_data( + { + "name": "update dialog", + "texts": ["Update available"], + "dismiss": { + "control": {"selector": "#danger", "role": "button", "name": "Close"} + }, + } + ) + report = _run(server, "mislabeled", tmp_path, classes=[mislabeled]) + assert report.success is False + assert "not the button 'Close'" in (report.results[0].error or "") + assert report.results[0].interstitial_actions == [] + assert not any("/logout-everywhere" in seen for seen in _Server.seen) + assert not any(seen.startswith("POST") for seen in _Server.seen) From 776afac7737537203c797aeff2e68ce62534067a Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 8 Nov 2026 20:51:27 -0500 Subject: [PATCH 04/10] docs: refresh llms-full.txt --- llms-full.txt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/llms-full.txt b/llms-full.txt index bb39c8c249..f082b6abf8 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -18,7 +18,7 @@ The open-source, verified execution layer for computer-use agents. [![CI](https://github.com/OpenAdaptAI/OpenAdapt/actions/workflows/main.yml/badge.svg)](https://github.com/OpenAdaptAI/OpenAdapt/actions/workflows/main.yml) [![Packages CI](https://github.com/OpenAdaptAI/OpenAdapt/actions/workflows/packages.yml/badge.svg)](https://github.com/OpenAdaptAI/OpenAdapt/actions/workflows/packages.yml) [![PyPI](https://img.shields.io/pypi/v/openadapt.svg)](https://pypi.org/project/openadapt/) [![Python 3.10–3.12](https://img.shields.io/badge/python-3.10%E2%80%933.12-blue)](https://www.python.org/downloads/) [![License: MIT](https://img.shields.io/badge/license-MIT-3e6b4f.svg)](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/LICENSE) [![Discord](https://img.shields.io/badge/Discord-community-5865F2?logo=discord&logoColor=white)](https://discord.gg/yF527cQbDG) -[What it can do](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/CAPABILITIES.md) · [Documentation](https://docs.openadapt.ai) · [Quickstart](https://docs.openadapt.ai/tutorials/quickstart/) · [Live demo](https://app.openadapt.ai/demo) · [Website](https://openadapt.ai) · [Discord](https://discord.gg/yF527cQbDG) +[What it can do](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/CAPABILITIES.md) · [Documentation](https://docs.openadapt.ai) · [Quickstart](https://docs.openadapt.ai/tutorials/quickstart/) · [Live demo](https://app.openadapt.ai/demo) · [Website](https://openadapt.ai) · [Get updates](https://openadapt.ai/#updates) · [Discord](https://discord.gg/yF527cQbDG) @@ -854,7 +854,7 @@ A test fails when a command has no file or this list is out of date. - `guard`: Attaches to the agent's own Chromium over CDP, holds each save until the right record is on screen, reads it back, and hands a verified run to the teach dir. - `learn`: Learn the app's own per-request writes from a shadow run, propose excuses, and approve them into the contracts. -- `shadow`: Shadow a FHIR or HTTP agent: check each write against the system of record without blocking it, and report. +- `shadow`: Shadow an agent: check each write it makes through an HTTP proxy, or check announced database actions for a desktop client, against the system of record without blocking it, and report. @@ -2145,7 +2145,8 @@ and exposes independently verifiable outcomes. | Condition | Current boundary | | --- | --- | | Zoom, DPI, font, layout, viewport, or monitor changes | Browser attach recording accepts a stable viewport or device-scale change between actions, starts a new exact geometry epoch, and refuses an action that overlaps the transition. Native offline conversion can normalize a stable source-window move, resize, monitor change, or scale change into one fixed output viewport when every timeline row carries valid mapping metadata. It refuses a malformed timeline or a changed output viewport. RDP and Citrix live backends can rebaseline geometry between actions and refuse a change during the exact-frame lease. On remote-desktop replays, a session redrawn at another display scale is detected at the first step: the runtime fits one scale from the recorded labels and continues on a view resampled to the recorded geometry, with every check unchanged. In the [display-scaling sweep](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/../benchmark/pixel_scale/README.md) (in-process renderings of the RDP and canvas kiosks, 100 to 175%, ClearType on and off) this took healthy runs from 3 of 24 to 17 of 24 with no wrong write. A reflowed layout, a font change without a scale change, a scale change during the run, and a changed pinned FreeRDP session profile still halt. Browser zoom on the web path still halts at the first step ([`cosmetic_drift`](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/../benchmark/cosmetic_drift/COSMETIC_DRIFT.md)). Other rescale or reflow behavior remains workflow-specific. | -| Native select menus, file choosers, permission prompts, and secure desktops | OS or browser chrome may not appear in the captured surface and may be unrecordable or undrivable. Prefer an application-level or keyboard/API path. | +| Native HTML single-selects | Driven `Recorder` capture can combine a focus-only click, literal TYPE, and Enter into one guarded `SELECT_OPTION`. It requires a changed value and literal text matching the selected label exactly or a unique enabled label prefix. It retains the previous and committed values, label, and census, then checks the same control after replay. Missing, unchanged, or ambiguous choice evidence refuses compilation; original source events remain available for diagnosis. This requires complete control and frame continuity; ordinary TYPE and remote selection keep their existing checks. The DOM operation emits input/change events, so each workflow must qualify application handlers and independently verify business effects. Interactive recording retains its existing native-select refusal. | +| Other native menus, file choosers, permission prompts, and secure desktops | OS or browser chrome may not appear in the captured surface and may be unrecordable or undrivable. Prefer an application-level or keyboard/API path. | | New tabs and windows | A bundle recorded with `Recorder` can declare the popups and tabs a step opens, act in them after an explicit surface switch, and halt on any page it did not declare ([MULTI_SURFACE.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/MULTI_SURFACE.md)). Such bundles run governed, qualified, and durable. Resume re-binds surfaces by identity, and managed Cloud dispatch needs only the `surfaces:` map. A program (graph) resume is refused. `record --url` and attach mode (`--cdp`, only pages a followed page opened) capture popups and tabs, and `--surface NAME=KIND[:TARGET]` binds backend surfaces at replay. Native windows and JavaScript dialogs are screen states, not surfaces. A dialog is answered by the step that raises it (browser backend, scripted recorder), and a hidden native window is reached by an explicit focus step. | | Drag and drop or gesture-heavy controls | Not a general supported primitive. Use a structured/API alternative or validate a purpose-built workflow. | | Dates, locale-sensitive fields, and auto-formatting | The same keystrokes can produce different values across platforms or locales. Require an explicit read-back or system effect and test the deployment locale. | From da48f878237d40203539a296e48e0f7eec7dcc79 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 8 Nov 2026 22:49:42 -0500 Subject: [PATCH 05/10] fix(flow): keep the dismiss control's label for the whole click The live-name check passed on Close before the click, but a trusted pointerover could change an external aria-labelledby label to Sign out and the press still landed (GET /logout-everywhere passes the fence). A click guard is now installed by an init script at the start of every document, with the per-context WebSocket guard, so its window capture-phase listeners run ahead of page listeners. Armed per click on the bound control (structural_label_guard), it watches the control, its aria-labelledby/describedby nodes, its labels and stylesheets with a MutationObserver (flushed with takeRecords) and a snapshot, and cancels any press or click that arrives after a change or off the control. A blocked input halts with LabelChangedDuringClick delivery uncertainty, the dismissal reported as delivered. A document without the guard, a guard that can't arm, or one whose listeners are gone afterwards halts. Learning uses the same guard. The write fence now logs every request (requests_seen), reports writes its check let through, and any write in the log that no attempt names becomes an attempt, so the report can't claim no writes the fence saw. --- packages/flow/CHANGELOG.md | 2 +- packages/flow/docs/INTERRUPTION_RECOVERY.md | 21 ++ .../backends/playwright_backend.py | 276 +++++++++++++++++- packages/flow/openadapt_flow/interruptions.py | 38 ++- packages/flow/openadapt_flow/ir.py | 14 +- .../openadapt_flow/runtime/effect_watch.py | 27 ++ .../flow/openadapt_flow/runtime/replayer.py | 83 +++++- packages/flow/tests/test_interruptions.py | 154 ++++++++++ .../flow/tests/test_interruptions_browser.py | 98 +++++++ 9 files changed, 701 insertions(+), 12 deletions(-) diff --git a/packages/flow/CHANGELOG.md b/packages/flow/CHANGELOG.md index 03ccae0bc5..42b20fd18c 100644 --- a/packages/flow/CHANGELOG.md +++ b/packages/flow/CHANGELOG.md @@ -23,7 +23,7 @@ ### Features -- Interruption classes: a run clears a known interruption, such as a session warning, a banner or a tour, by itself, but only when the dismissal is provably read-only. The class must match by its text and control, the control's label must be in a closed non-committing set and be the live label of the control that is clicked, and no write watcher may see a write. On the web the watcher is the browser write fence; on any backend, including desktop and RDP, it is the system-of-record read-back. Afterwards the step's normal identity check must pass. Classes are per-app data (`runtime.interruptions_path`) or learned from a halt (`learn_interruption`). Each dismissal is in the run report. See `docs/INTERRUPTION_RECOVERY.md`. +- Interruption classes: a run clears a known interruption, such as a session warning, a banner or a tour, by itself, but only when the dismissal is provably read-only. The class must match by its text and control, the control's label must be in a closed non-committing set and be the live label of the control that is clicked, held for the whole click by a page-side click guard, and no write watcher may see a write. On the web the watcher is the browser write fence; on any backend, including desktop and RDP, it is the system-of-record read-back. Afterwards the step's normal identity check must pass. Classes are per-app data (`runtime.interruptions_path`) or learned from a halt (`learn_interruption`). Each dismissal is in the run report. See `docs/INTERRUPTION_RECOVERY.md`. ### Fixes diff --git a/packages/flow/docs/INTERRUPTION_RECOVERY.md b/packages/flow/docs/INTERRUPTION_RECOVERY.md index bcd9bbca29..e93827b29b 100644 --- a/packages/flow/docs/INTERRUPTION_RECOVERY.md +++ b/packages/flow/docs/INTERRUPTION_RECOVERY.md @@ -46,6 +46,27 @@ A run clears an interruption only when all four of these hold: click. So does a backend that can't read the live control (`structural_control_is`), which today means every backend except the browser. + + The label must also hold for the whole click. In the browser, a click + guard is installed at the start of every document, ahead of any page + script, so its capture-phase listeners on `window` run before any page + listener. It is dormant until a dismissal arms it on the bound control. + While armed, it watches the control, the nodes its `aria-labelledby` and + `aria-describedby` name, its labels, and the page's stylesheets. If any + of them changed before a press or click arrives, or the input is not on + the control, the guard cancels that input before the page sees it. The + run then halts with the dismissal reported as delivered and its outcome + uncertain (`LabelChangedDuringClick`), so it is reconciled, never + retried. A label that turns "Close" into "Sign out" as the pointer + arrives is caught this way. A document that loaded before the guard was + installed, a guard that can't arm, or one whose listeners are gone after + the click (for example, erased by `document.open`) halts too. Learning a + class from a halt uses the same guard. + + The report agrees with the fence. Every request made while the fence is + up is listed in the dismissal's `requests_seen`, reads included, and a + write in that log that no write attempt names is added as one. So a + report never says "no write attempts" while the fence saw a write. 3. **No watcher sees a write.** The dismissal runs under every write watcher the run has (`openadapt_flow/runtime/effect_watch.py`): - in a browser, the write fence. It is the same hold that guards a diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index a2427ccf75..6f3635a165 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -2167,6 +2167,149 @@ def _normalize_chord(key: str) -> str: return "+".join(normalized) +#: The click guard (:meth:`PlaywrightBackend.structural_label_guard`), +#: installed by an init script at the start of every document, so its +#: capture-phase listeners on ``window`` run ahead of every page listener. +#: Dormant until armed for one click on one element; armed, it blocks the +#: press and the click whenever the element's label inputs changed since it +#: was armed (a MutationObserver, flushed with ``takeRecords`` in every +#: listener, and a fresh snapshot compared with the armed one), or the input +#: is not on the element. A page that erases window listeners +#: (``document.open``) fails the guard's heartbeat. Formatted with the key. +_CLICK_GUARD_JS = r""" +((key) => { + if (Object.prototype.hasOwnProperty.call(window, key)) return; + const R = Reflect.apply; + const getAttr = Element.prototype.getAttribute; + const textOf = Object.getOwnPropertyDescriptor(Node.prototype, 'textContent').get; + const connected = Object.getOwnPropertyDescriptor(Node.prototype, 'isConnected').get; + const rootOf = Node.prototype.getRootNode; + const qsa = Document.prototype.querySelectorAll; + const docById = Document.prototype.getElementById; + const fragById = DocumentFragment.prototype.getElementById; + const path = Event.prototype.composedPath; + const prevent = Event.prototype.preventDefault; + const stopNow = Event.prototype.stopImmediatePropagation; + const MO = window.MutationObserver; + const observe = MO.prototype.observe; + const take = MO.prototype.takeRecords; + const disconnect = MO.prototype.disconnect; + const add = EventTarget.prototype.addEventListener; + const dispatch = EventTarget.prototype.dispatchEvent; + const Ev = window.Event; + const stringify = JSON.stringify; + const ATTRS = ['role', 'aria-label', 'aria-labelledby', 'aria-describedby', + 'title', 'alt', 'value', 'hidden', 'aria-hidden', 'class', 'style', 'id', + 'type', 'disabled']; + const EVENTS = ['pointerdown', 'mousedown', 'pointerup', 'mouseup', 'click', + 'auxclick', 'dblclick', 'contextmenu', 'keydown', 'keypress', 'keyup', + 'touchstart', 'touchend']; + const probe = key + '_probe'; + let beat = 0; + let st = null; + const attr = (el, a) => R(getAttr, el, [a]); + const text = n => R(textOf, n, []); + const refs = el => { + const out = []; + const root = R(rootOf, el, []); + for (const a of ['aria-labelledby', 'aria-describedby']) { + for (const id of (attr(el, a) || '').split(/\s+/).filter(Boolean)) { + const n = root instanceof Document ? R(docById, root, [id]) + : (root instanceof DocumentFragment ? R(fragById, root, [id]) : null); + out.push([id, n]); + } + } + return out; + }; + const snapshot = el => { + const parts = [R(connected, el, []), el.tagName]; + for (const a of ATTRS) parts.push(attr(el, a)); + parts.push(text(el)); + if ('value' in el) parts.push(String(el.value)); + for (const [id, n] of refs(el)) { + parts.push(id, n ? text(n) : null); + if (n) for (const a of ATTRS) parts.push(attr(n, a)); + } + if (el.labels) for (const l of el.labels) parts.push(text(l), attr(l, 'class'), attr(l, 'style')); + return stringify(parts); + }; + const styled = records => records.some(r => { + const t = r.target; + if (r.type === 'characterData') return !!(t.parentNode && t.parentNode.nodeName === 'STYLE'); + if (t && (t.nodeName === 'STYLE' || t.nodeName === 'LINK')) return true; + for (const n of [...r.addedNodes, ...r.removedNodes]) { + if (n.nodeName === 'STYLE' || n.nodeName === 'LINK') return true; + } + return false; + }); + const flush = () => { + if (!st) return; + if (R(take, st.mo, []).length) st.tainted = 'the control or its label changed'; + if (styled(R(take, st.docMo, []))) st.tainted = 'a stylesheet changed'; + if (!st.tainted && snapshot(st.el) !== st.snap) st.tainted = 'the control or its label changed'; + }; + const alive = () => { + const before = beat; + try { R(dispatch, window, [new Ev(probe)]); } catch (e) { return false; } + return beat === before + 1; + }; + R(add, window, [probe, () => { beat += 1; }, true]); + const guard = e => { + if (!st) return; + flush(); + let why = st.tainted; + if (!why) { + let inside = false; + try { inside = R(path, e, []).includes(st.el); } catch (err) { inside = false; } + if (!inside) why = 'the input is not on the control'; + } + st.events.push(e.type + (e.isTrusted ? '' : ' (synthetic)')); + if (why) { + R(prevent, e, []); + R(stopNow, e, []); + st.violations.push(e.type + ': ' + why); + } + }; + for (const t of EVENTS) R(add, window, [t, guard, true]); + const arm = (tokenAttr, secret) => { + if (st) { st.violations.push('armed twice'); return {ok: false, reason: 'the guard is already armed'}; } + if (!alive()) return {ok: false, reason: "the guard's listeners are not in this document"}; + const found = R(qsa, document, ['[' + tokenAttr + ']']); + if (found.length !== 1) return {ok: false, reason: 'the control is not in this document'}; + const el = found[0]; + const opts = {attributes: true, characterData: true, childList: true, subtree: true}; + const mo = new MO(() => { if (st) st.tainted = 'the control or its label changed'; }); + const docMo = new MO(rs => { if (st && styled(rs)) st.tainted = 'a stylesheet changed'; }); + try { + R(observe, mo, [el, opts]); + for (const [, n] of refs(el)) if (n) R(observe, mo, [n, opts]); + if (el.labels) for (const l of el.labels) R(observe, mo, [l, opts]); + R(observe, docMo, [document, {characterData: true, childList: true, subtree: true, attributes: true, attributeFilter: ['href', 'media', 'disabled']}]); + } catch (err) { + R(disconnect, mo, []); R(disconnect, docMo, []); + return {ok: false, reason: 'the label could not be watched'}; + } + st = {el, secret, snap: snapshot(el), tainted: null, violations: [], events: [], mo, docMo}; + return {ok: true}; + }; + const finish = secret => { + if (!st || st.secret !== secret) return {ok: false, violations: ['the guard was not armed by this click'], events: []}; + // A change after the last input is the click's own effect (the dialog + // closing); only an input judged after a change is a violation. + const out = {ok: true, violations: st.violations.slice(), events: st.events.slice()}; + if (!alive()) out.violations.push("the guard's listeners were removed during the click"); + R(disconnect, st.mo, []); R(disconnect, st.docMo, []); + st = null; + return out; + }; + Object.defineProperty(window, key, { + value: Object.freeze({arm, finish}), enumerable: false, + configurable: false, writable: false, + }); +})(%s); +""" + + class PlaywrightBackend: """`Backend` implementation over a Playwright sync-API `Page`. @@ -3515,6 +3658,76 @@ def structural_control_is( except Exception: # noqa: BLE001 - unreadable means unproven return False + @contextlib.contextmanager + def structural_label_guard( + self, + locator: StructuralLocator, + handle: StructuralHandle, + *, + role: str, + name: str, + ) -> Iterator["LabelGuardReport"]: + """Keep the control's checked label for the whole click. + + Arms the document's click guard (:data:`_CLICK_GUARD_JS`) on the + element ``handle`` is bound to, after checking once more that it is + the one control with ``role`` and ``name`` + (:meth:`structural_control_is`). While the ``with`` body clicks, any + change to the element, the nodes its ``aria-labelledby`` and + ``aria-describedby`` name, its labels, or a stylesheet, and any input + that is not on the element, blocks the press and the click before a + page listener sees them, and is recorded. A guard that can't arm + sets ``refusal`` and the body must not act. After the body, + ``violations`` lists what was blocked; a guard that can't be read + afterwards counts as a violation. + """ + + report = LabelGuardReport() + guard = self._structural_tokens.get(handle.target_fingerprint or "") + key = _click_guard_key(self.page.context) + if guard is None or key is None: + report.refusal = "the click guard can't be armed for this control" + yield report + return + if not self.structural_control_is(locator, handle, role=role, name=name): + report.refusal = f"the control is no longer the {role} {name!r}" + yield report + return + secret = uuid.uuid4().hex + try: + armed = guard.scope.evaluate( + "([k, a, s]) => { const g = window[k];" + " return g ? g.arm(a, s) : {ok: false," + " reason: 'this document has no click guard (it loaded before" + " the guard was installed)'}; }", + [key, self._token_attribute(guard.token), secret], + ) + except Exception as exc: # noqa: BLE001 - no guard, no click + armed = {"ok": False, "reason": f"arming failed ({type(exc).__name__})"} + if not isinstance(armed, dict) or armed.get("ok") is not True: + reason = armed.get("reason") if isinstance(armed, dict) else None + report.refusal = f"the click guard did not arm: {reason or 'unknown'}" + yield report + return + try: + yield report + finally: + try: + seen = guard.scope.evaluate( + "([k, s]) => { const g = window[k];" + " return g ? g.finish(s) : null; }", + [key, secret], + ) + except Exception: # noqa: BLE001 - the document went away + seen = None + if not isinstance(seen, dict) or seen.get("ok") is not True: + report.violations.append( + "the click guard could not be read after the click" + ) + else: + report.violations.extend(str(v) for v in seen.get("violations") or ()) + report.events = [str(e) for e in seen.get("events") or ()] + @contextlib.contextmanager def write_fence(self) -> Iterator["WriteFenceReport"]: """Run the ``with`` body while nothing on the page can write. @@ -3533,9 +3746,31 @@ def write_fence(self) -> Iterator["WriteFenceReport"]: report.refusal = "a save is already held on this page" yield report return - refusal = self._arm_wire_check(None, check=WireCheck()) + check = WireCheck() + context = self.page.context + + def on_request(request: Any) -> None: + try: + parts = urlsplit(request.url) + report.requests.append( + f"{str(request.method).upper()} {parts.path or '/'}" + ) + except Exception: # noqa: BLE001 - a request this can't read + report.requests.append("? (a request that could not be read)") + + try: + context.on("request", on_request) + except Exception: # noqa: BLE001 - no request log, no proof + report.refusal = "the fence could not log the page's requests" + yield report + return + refusal = self._arm_wire_check(None, check=check) if refusal is not None: report.refusal = refusal + try: + context.remove_listener("request", on_request) + except Exception: # noqa: BLE001 + pass yield report return try: @@ -3544,6 +3779,12 @@ def write_fence(self) -> Iterator["WriteFenceReport"]: self.wait_form_submit_guard(WRITE_FENCE_TAIL_MS) finally: report.attempts.extend(self._disarm_wire_check()) + # A write the check let through is still a write. + report.attempts.extend(f"{sent} left the page" for sent in check.sent) + try: + context.remove_listener("request", on_request) + except Exception: # noqa: BLE001 - the context is gone + pass def wait_form_submit_guard(self, ms: int) -> None: """Let the page and the wire check run for ``ms`` milliseconds.""" @@ -6179,14 +6420,44 @@ def _ws_guard_secrets(target: Any) -> tuple[str, str, str]: context._oaflow_ws_guard = secrets except Exception: # noqa: BLE001 - a stand-in context; holding refuses pass + _click_guard_key(context) return secrets +def _click_guard_key(context: Any) -> Optional[str]: + """The context's click-guard key (:data:`_CLICK_GUARD_JS`), installing + the guard's init script on first use. None when it can't be installed; + a guarded click then refuses. Documents opened before it have no guard, + so a guarded click in one refuses too.""" + + known = getattr(context, "_oaflow_click_guard", None) + if isinstance(known, str): + return known + key = f"__oaflow_click_{uuid.uuid4().hex}" + try: + context.add_init_script(_CLICK_GUARD_JS % json.dumps(key)) + context._oaflow_click_guard = key + except Exception: # noqa: BLE001 - a stand-in context; guarding refuses + return None + return key + + #: How long a write fence stays up after its action, so a request the action #: queued just before the fence comes down is still caught. WRITE_FENCE_TAIL_MS = 500 +@dataclass +class LabelGuardReport: + """What the click guard saw. ``refusal`` set means it never armed.""" + + refusal: Optional[str] = None + #: Each input the guard blocked, and why. + violations: list[str] = field(default_factory=list) + #: The input events that reached the guard. + events: list[str] = field(default_factory=list) + + @dataclass class WriteFenceReport: """What a write fence saw. ``refusal`` set means it never went up.""" @@ -6194,6 +6465,9 @@ class WriteFenceReport: refusal: Optional[str] = None #: Each write the fence stopped, and how. attempts: list[str] = field(default_factory=list) + #: Every request the page made while the fence was up (``METHOD /path``), + #: reads included, so a report can be reconciled with what was sent. + requests: list[str] = field(default_factory=list) @property def held(self) -> bool: diff --git a/packages/flow/openadapt_flow/interruptions.py b/packages/flow/openadapt_flow/interruptions.py index d59acc57ec..15ba5b4321 100644 --- a/packages/flow/openadapt_flow/interruptions.py +++ b/packages/flow/openadapt_flow/interruptions.py @@ -469,6 +469,19 @@ def save_interruption_class(path: Path | str, interstitial: "Interstitial") -> N # -- learning a class from a halt ----------------------------------------------- +class LabelChangedDuringClick(RuntimeError): + """The dismiss control, or what labels it, changed while it was being + clicked, and the click guard blocked the input. Delivery was attempted; + the outcome must be reconciled, never retried.""" + + def __init__(self, violations: Sequence[str]) -> None: + self.violations = [str(v) for v in violations] + super().__init__( + "the control's label changed during the click; the guard blocked " + "the input (" + "; ".join(self.violations) + ")" + ) + + class LearnRefused(RuntimeError): """The interruption can't be learned as a class; the reason says why.""" @@ -531,10 +544,33 @@ def learn_interruption( control_error = live_control_error(interstitial, backend, handle) if control_error is not None: raise LearnRefused(control_error) + opener = getattr(backend, "structural_label_guard", None) + if not callable(opener): + raise LearnRefused( + "this backend can't keep the control's label for the whole click, " + "so the class can't be learned here" + ) + role = (locator.role or "").strip() + label = (locator.name or "").strip() with watch_effects(backend, effect_verifier) as report: if report.refusal is not None: raise LearnRefused(f"the dismissal can't be watched: {report.refusal}") - act(locator, handle) + guarded: Any = None + try: + with opener(locator, handle, role=role, name=label) as guarded: + if guarded.refusal is not None: + raise LearnRefused(f"the click can't be guarded: {guarded.refusal}") + act(locator, handle) + except LearnRefused: + raise + except Exception as exc: + if guarded is not None and guarded.violations: + raise LearnRefused( + str(LabelChangedDuringClick(guarded.violations)) + ) from exc + raise + if guarded.violations: + raise LearnRefused(str(LabelChangedDuringClick(guarded.violations))) settle() if not report.held: raise LearnRefused( diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index cade2ef218..0c03ca3de8 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -4423,6 +4423,12 @@ class InterstitialActionResult(BaseModel): effect_watchers: list[str] = Field(default_factory=list) #: Each write a watcher saw. Non-empty halts the run. write_attempts: list[str] = Field(default_factory=list) + #: Every request the network watcher logged during the dismissal + #: (``METHOD /path``), reads included, so the report shows what was sent. + requests_seen: list[str] = Field(default_factory=list) + #: Inputs the click guard blocked because the control or its label + #: changed during the click. Non-empty halts the run. + label_violations: list[str] = Field(default_factory=list) #: The dismiss control's label, for the receipt. control_label: Optional[str] = None @@ -4432,7 +4438,13 @@ def _serialize_compatible(self, handler: Any) -> dict[str, Any]: existing interstitials serialize exactly as before.""" data: dict[str, Any] = handler(self) - for key in ("effect_watchers", "write_attempts", "control_label"): + for key in ( + "effect_watchers", + "write_attempts", + "requests_seen", + "label_violations", + "control_label", + ): if not data.get(key): data.pop(key, None) return data diff --git a/packages/flow/openadapt_flow/runtime/effect_watch.py b/packages/flow/openadapt_flow/runtime/effect_watch.py index 9c49bc6f79..ab94e5abc7 100644 --- a/packages/flow/openadapt_flow/runtime/effect_watch.py +++ b/packages/flow/openadapt_flow/runtime/effect_watch.py @@ -39,6 +39,10 @@ class EffectWatchReport: refusal: Optional[str] = None #: Each write a watcher saw. attempts: list[str] = field(default_factory=list) + #: Every request the network watcher logged (``METHOD /path``), reads + #: included. A write among them that no attempt accounts for is added to + #: ``attempts`` (:func:`reconcile_requests`). + requests: list[str] = field(default_factory=list) #: The system-of-record digest before and after, when it was read. record_before: Optional[str] = None record_after: Optional[str] = None @@ -119,6 +123,8 @@ def watch_effects( pass if fence is not None and "network" in report.watchers: report.attempts.extend(f"network: {a}" for a in fence.attempts) + report.requests.extend(str(r) for r in getattr(fence, "requests", ()) or ()) + report.attempts.extend(reconcile_requests(report.requests, report.attempts)) if "record" in report.watchers: report.record_after = record_digest(effect_verifier) if report.record_after is None: @@ -128,3 +134,24 @@ def watch_effects( ) elif report.record_after != report.record_before: report.attempts.append("record: the system of record changed") + + +#: Methods that only read. Any other method in the request log is a write. +READ_METHODS = frozenset({"GET", "HEAD", "OPTIONS"}) + + +def reconcile_requests(requests: list[str], attempts: list[str]) -> list[str]: + """The writes in ``requests`` that no entry in ``attempts`` names. + + The report must never say "no write attempts" while the request log holds + a write: each unnamed one (or an unreadable request) becomes an attempt. + """ + + missing: list[str] = [] + for request in dict.fromkeys(requests): + method = request.split(" ", 1)[0].upper() + if method in READ_METHODS: + continue + if not any(request in attempt for attempt in attempts): + missing.append(f"network: {request} was sent while the fence was up") + return missing diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index 1a87caafe3..741fc8861d 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -99,6 +99,7 @@ parameterize_identity_text, ) from openadapt_flow.interruptions import ( + LabelChangedDuringClick, class_texts, dismiss_label, live_control_error, @@ -145,6 +146,8 @@ StateKind, Step, StepResult, + StructuralHandle, + StructuralLocator, UnarmedStep, VisualResolutionEvidence, Workflow, @@ -14618,6 +14621,50 @@ def _canonical_interstitial_snapshot( ) return validated, canonical + def _deliver_guarded_dismissal( + self, + result: StepResult, + event: InterstitialActionResult, + locator: StructuralLocator, + handle: Optional[StructuralHandle], + native_act: Callable[..., Any], + ) -> None: + """Click an interruption class's dismiss control under the backend's + click guard, which keeps the checked label for the whole click + (``structural_label_guard``). No guard, or one that can't arm, is a + refusal before input; anything the guard blocked raises + :class:`LabelChangedDuringClick` after it.""" + + opener = getattr(self.backend, "structural_label_guard", None) + if ( + not callable(opener) + or handle is None + or not locator.role + or not locator.name + ): + raise StructuralResolutionRefused( + "this backend can't keep the control's label for the whole " + "click, so the dismissal is not delivered" + ) + label_report: Any = None + try: + with opener( + locator, handle, role=locator.role, name=locator.name + ) as label_report: + if label_report.refusal is not None: + raise StructuralResolutionRefused(label_report.refusal) + self._deliver_backend_call(result, lambda: native_act(locator, handle)) + except StructuralResolutionRefused: + if label_report is not None and label_report.violations: + raise LabelChangedDuringClick(label_report.violations) from None + raise + except Exception as exc: + if label_report is not None and label_report.violations: + raise LabelChangedDuringClick(label_report.violations) from exc + raise + if label_report.violations: + raise LabelChangedDuringClick(label_report.violations) + def _interruption_control_error( self, it: Interstitial, resolution: Optional[Resolution] ) -> Optional[str]: @@ -15326,6 +15373,7 @@ def _record_watch() -> None: if watched: event.effect_watchers = list(watched[0].watchers) event.write_attempts = list(watched[0].attempts) + event.requests_seen = list(watched[0].requests) watch_stack.callback(_record_watch) watch = watch_stack.enter_context( @@ -15371,14 +15419,23 @@ def _record_watch() -> None: ) if control_error is not None: raise StructuralResolutionRefused(control_error) - self._require_qualification_environment_current() - self._deliver_backend_call( - result, - lambda: native_act( + self._require_qualification_environment_current() + self._deliver_guarded_dismissal( + result, + event, structural_anchor, resolution.structural_handle, - ), - ) + native_act, + ) + else: + self._require_qualification_environment_current() + self._deliver_backend_call( + result, + lambda: native_act( + structural_anchor, + resolution.structural_handle, + ), + ) else: self._require_qualification_environment_current() self._deliver_backend_call( @@ -15389,6 +15446,13 @@ def _record_watch() -> None: ) event.delivered = True except Exception as exc: + if isinstance(exc, LabelChangedDuringClick): + # The press reached the page's guard, which blocked + # it: delivery was attempted, and nothing beneath + # the label change is trusted. + event.delivered = True + event.label_violations = list(exc.violations) + result.safety_halt = True if not isinstance( exc, (FreshActuationRequired, StructuralResolutionRefused) ): @@ -15414,10 +15478,13 @@ def _record_watch() -> None: "dismissal delivery failed " f"({type(exc).__name__}); outcome is unknown" ) + if it.effect_watch and str(exc): + event.error += f": {exc}" return before_png, ( f"Interstitial '{it.name}' dismissal delivery failed " - f"({type(exc).__name__}) - refusing to act beneath it; run " - "aborted." + f"({type(exc).__name__}" + + (f": {exc}" if it.effect_watch and str(exc) else "") + + ") - refusing to act beneath it; run aborted." ) before_png, settle_error = self._resettle_after_interstitial(before_png) diff --git a/packages/flow/tests/test_interruptions.py b/packages/flow/tests/test_interruptions.py index 6abd82c1e1..14352ed4a7 100644 --- a/packages/flow/tests/test_interruptions.py +++ b/packages/flow/tests/test_interruptions.py @@ -63,6 +63,19 @@ def __init__(self, refusal=None, attempts=()): self.attempts = list(attempts) +class _Guard: + def __init__(self, refusal=None): + self.refusal = refusal + self.violations: list[str] = [] + self.events: list[str] = [] + + +class _LoggingFence(_Fence): + def __init__(self, requests): + super().__init__() + self.requests = list(requests) + + class WatchedBackend(FakeBackend): """A structural backend with a scripted write fence.""" @@ -76,8 +89,17 @@ def __init__( title="", live=None, can_read_live=True, + guard_refusal=None, + guard_violations=(), + can_guard=True, ): super().__init__() + self._guard_refusal = guard_refusal + self._guard_violations = list(guard_violations) + self.guarded: list[bool] = [] + self._guard_up = False + if not can_guard: + self.structural_label_guard = None # type: ignore[assignment] # The live control's (role, name); None means the declared one. self._live = live self.live_checks: list[tuple[str, str]] = [] @@ -108,7 +130,19 @@ def locate_structural(self, locator): point=(15, 15), region=(10, 10, 10, 10), target_fingerprint="a" * 64 ) + @contextmanager + def structural_label_guard(self, locator, handle, *, role, name): + report = _Guard(refusal=self._guard_refusal) + self._guard_up = self._guard_refusal is None + try: + yield report + finally: + self._guard_up = False + if self._guard_refusal is None: + report.violations.extend(self._guard_violations) + def act_structural(self, locator, handle, *, double=False): + self.guarded.append(self._guard_up) self.fenced.append(self._up) self.actions.append(("act_structural", locator.name)) @@ -532,3 +566,123 @@ def test_learning_refuses_a_control_whose_live_label_differs_without_clicking(): shows=lambda text: True, ) assert backend.actions == [] + + +# -- the label holds for the whole click ------------------------------------------ + + +def test_a_watched_dismissal_is_clicked_under_the_click_guard(bundle, run_dir): + backend = WatchedBackend() + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + assert report.success is True, report.results[0].error + assert backend.guarded == [True] + + +def test_a_backend_without_a_click_guard_never_clicks(bundle, run_dir): + backend = WatchedBackend(can_guard=False) + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + assert report.success is False + assert "for the whole click" in (report.results[0].error or "") + assert not any(a[0] == "act_structural" for a in backend.actions) + + +def test_a_click_guard_that_cannot_arm_never_clicks(bundle, run_dir): + backend = WatchedBackend(guard_refusal="this document has no click guard") + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + assert report.success is False + assert not any(a[0] == "act_structural" for a in backend.actions) + event = report.results[0].interstitial_actions[0] + assert event.delivered is False + assert "no click guard" in (event.error or "") + + +def test_a_label_change_during_the_click_halts_as_uncertain_delivery(bundle, run_dir): + backend = WatchedBackend( + guard_violations=["pointerdown: the control or its label changed"] + ) + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + result = report.results[0] + assert report.success is False + assert result.safety_halt is True + assert result.delivery_uncertainty is not None + assert result.delivery_uncertainty.cause_type == "LabelChangedDuringClick" + event = result.interstitial_actions[0] + assert event.delivered is True + assert event.label_violations == ["pointerdown: the control or its label changed"] + + +def test_learning_refuses_when_the_label_changes_during_the_click(): + backend = WatchedBackend( + guard_violations=["click: the control or its label changed"] + ) + with pytest.raises(LearnRefused, match="label changed during the click"): + learn_interruption( + name="n", + texts=["t"], + control={"role": "button", "name": "Close"}, + backend=backend, + shows=lambda text: False if text == "t" and backend.actions else True, + ) + + +def test_learning_refuses_without_a_click_guard(): + backend = WatchedBackend(can_guard=False) + with pytest.raises(LearnRefused, match="for the whole click"): + learn_interruption( + name="n", + texts=["t"], + control={"role": "button", "name": "Close"}, + backend=backend, + shows=lambda text: True, + ) + assert backend.actions == [] + + +# -- the report agrees with the fence's request log ------------------------------- + + +def test_a_write_in_the_request_log_is_never_reported_as_no_write(): + from openadapt_flow.runtime.effect_watch import reconcile_requests + + assert reconcile_requests(["GET /ping"], []) == [] + assert ( + reconcile_requests( + ["POST /logout"], ["network: POST /logout; it was cancelled"] + ) + == [] + ) + assert reconcile_requests(["POST /logout"], []) == [ + "network: POST /logout was sent while the fence was up" + ] + assert reconcile_requests(["? (a request that could not be read)"], []) + + +def test_the_fence_log_reaches_the_report(bundle, run_dir): + class Logging(WatchedBackend): + @contextmanager + def write_fence(self): # type: ignore[override] + report = _LoggingFence(["GET /ping", "PUT /x"]) + self._up = True + try: + yield report + finally: + self._up = False + + backend = Logging() + report = _run( + backend, _vision(), bundle, run_dir, [interruption_from_data(SESSION)] + ) + result = report.results[0] + assert report.success is False + assert result.safety_halt is True + event = result.interstitial_actions[0] + assert event.requests_seen == ["GET /ping", "PUT /x"] + assert event.write_attempts == ["network: PUT /x was sent while the fence was up"] diff --git a/packages/flow/tests/test_interruptions_browser.py b/packages/flow/tests/test_interruptions_browser.py index 77e2d0cba3..d8b3fbbee3 100644 --- a/packages/flow/tests/test_interruptions_browser.py +++ b/packages/flow/tests/test_interruptions_browser.py @@ -18,6 +18,9 @@ steal-press a popup covers Save between press and release halt, no write unknown an unlisted dialog halt, no click mislabeled class says "Close", its selector is "Sign out" halt, no click +relabel external label turns "Close" into "Sign out" halt, no GET + on pointerover +external a stable external label "Close" saved ============= =============================================== ============ """ @@ -87,6 +90,23 @@ } else if (kind === 'mislabeled') { dialog('Update available now', 'Sign out', () => fetch('/logout-everywhere')); document.querySelector('[role=dialog] button').id = 'danger'; +} else if (kind === 'relabel' || kind === 'external') { + // The button's name comes from an external aria-labelledby node. + const d = document.createElement('div'); + d.setAttribute('role', 'dialog'); + d.style = 'position:fixed;inset:0;background:#fff;padding:40px;font-size:28px'; + d.innerHTML = '

Update available now

Close ' + + ''; + document.body.appendChild(d); + const b = d.querySelector('#x'); + if (kind === 'relabel') { + b.addEventListener('pointerover', () => { + document.getElementById('lbl').textContent = 'Sign out'; + }); + b.onclick = () => { fetch('/logout-everywhere'); d.remove(); }; + } else { + b.onclick = () => d.remove(); + } } else if (kind === 'banner') { const b = document.createElement('div'); b.style = 'position:fixed;top:0;left:0;right:0;height:120px;background:#ffd;' @@ -267,3 +287,81 @@ def test_a_selector_that_points_at_another_control_halts_without_a_click( assert report.results[0].interstitial_actions == [] assert not any("/logout-everywhere" in seen for seen in _Server.seen) assert not any(seen.startswith("POST") for seen in _Server.seen) + + +LABELLED = interruption_from_data( + { + "name": "update dialog", + "texts": ["Update available"], + "dismiss": {"control": {"selector": "#x", "role": "button", "name": "Close"}}, + } +) + + +def test_a_label_that_changes_as_the_pointer_arrives_halts_with_no_request( + server: str, tmp_path: Any +) -> None: + report = _run(server, "relabel", tmp_path, classes=[LABELLED]) + assert report.success is False + result = report.results[0] + assert result.safety_halt is True + assert result.delivery_uncertainty is not None + assert result.delivery_uncertainty.cause_type == "LabelChangedDuringClick" + event = result.interstitial_actions[0] + assert event.delivered is True and event.label_violations + assert not any("/logout-everywhere" in seen for seen in _Server.seen) + assert not any(seen.startswith("POST") for seen in _Server.seen) + + +def test_a_stable_external_label_is_cleared_and_the_save_lands( + server: str, tmp_path: Any +) -> None: + report = _run(server, "external", tmp_path, classes=[LABELLED]) + assert report.success is True, report.results[0].error + event = report.results[0].interstitial_actions[0] + assert event.ok and event.label_violations == [] + assert "POST /save" in _Server.seen + + +def _learn(url: str, kind: str) -> Any: + from openadapt_flow.backends.playwright_backend import PlaywrightBackend + from openadapt_flow.interruptions import learn_interruption + + try: + backend, close = PlaywrightBackend.launch(url + "#" + kind, headless=True) + except Exception as exc: # pragma: no cover - no browser installed + pytest.skip(f"no Playwright browser: {exc}") + + def shows(text: str) -> bool: + return bool( + backend.page.evaluate("t => document.body.innerText.includes(t)", text) + ) + + try: + return learn_interruption( + name="update dialog", + texts=["Update available"], + control={"selector": "#x", "role": "button", "name": "Close"}, + backend=backend, + shows=shows, + settle=lambda: backend.page.wait_for_timeout(200), + ) + finally: + backend.page.wait_for_timeout(300) + close() + + +def test_learning_refuses_a_label_that_changes_as_the_pointer_arrives( + server: str, +) -> None: + from openadapt_flow.interruptions import LearnRefused + + with pytest.raises(LearnRefused, match="label changed during the click"): + _learn(server, "relabel") + assert not any("/logout-everywhere" in seen for seen in _Server.seen) + assert not any(seen.startswith("POST") for seen in _Server.seen) + + +def test_learning_keeps_a_stable_external_label(server: str) -> None: + learned = _learn(server, "external") + assert learned.watchers == ("network",) From edb08b7a5bea1d5639cc36d60953a4dd2b15c48a Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 9 Nov 2026 00:34:08 -0500 Subject: [PATCH 06/10] fix(flow): count a CSSOM relabel of the dismiss control as a change A stylesheet rule added through insertRule or adoptedStyleSheets could change a label's ::before/::after content with no DOM mutation, and the click guard let the press through. The guard's snapshot, re-taken at every guarded input, now also reads the computed ::before, ::after and own content, display, visibility and content-visibility of the control, its aria-labelledby/describedby nodes, its labels and their first descendants, using built-ins captured at install. Any difference blocks the input. --- packages/flow/docs/INTERRUPTION_RECOVERY.md | 9 ++- .../backends/playwright_backend.py | 41 ++++++++++++- .../flow/tests/test_interruptions_browser.py | 57 +++++++++++++++++++ 3 files changed, 103 insertions(+), 4 deletions(-) diff --git a/packages/flow/docs/INTERRUPTION_RECOVERY.md b/packages/flow/docs/INTERRUPTION_RECOVERY.md index e93827b29b..30fe01ad2a 100644 --- a/packages/flow/docs/INTERRUPTION_RECOVERY.md +++ b/packages/flow/docs/INTERRUPTION_RECOVERY.md @@ -52,8 +52,13 @@ A run clears an interruption only when all four of these hold: script, so its capture-phase listeners on `window` run before any page listener. It is dormant until a dismissal arms it on the bound control. While armed, it watches the control, the nodes its `aria-labelledby` and - `aria-describedby` name, its labels, and the page's stylesheets. If any - of them changed before a press or click arrives, or the input is not on + `aria-describedby` name, its labels, and the page's stylesheets. At + every press and click it also re-reads how each of them renders: the + computed `::before` and `::after` content, their own computed content, + and `display` and `visibility`, so a stylesheet rule added through the + CSSOM (`insertRule`, `adoptedStyleSheets`) that relabels or hides the + text counts as a change. If any of them changed before a press or click + arrives, or the input is not on the control, the guard cancels that input before the page sees it. The run then halts with the dismissal reported as delivered and its outcome uncertain (`LabelChangedDuringClick`), so it is reconciled, never diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index 4af246762c..aec24701d3 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -2173,7 +2173,10 @@ def _normalize_chord(key: str) -> str: #: Dormant until armed for one click on one element; armed, it blocks the #: press and the click whenever the element's label inputs changed since it #: was armed (a MutationObserver, flushed with ``takeRecords`` in every -#: listener, and a fresh snapshot compared with the armed one), or the input +#: listener, and a fresh snapshot compared with the armed one: attributes, +#: text, and the computed ``::before``/``::after`` and own ``content``, +#: ``display`` and ``visibility`` of the element, its label nodes and their +#: first descendants, so a CSSOM relabel counts), or the input #: is not on the element. A page that erases window listeners #: (``document.open``) fails the guard's heartbeat. Formatted with the key. _CLICK_GUARD_JS = r""" @@ -2198,6 +2201,33 @@ def _normalize_chord(key: str) -> str: const dispatch = EventTarget.prototype.dispatchEvent; const Ev = window.Event; const stringify = JSON.stringify; + const computed = window.getComputedStyle; + const propOf = CSSStyleDeclaration.prototype.getPropertyValue; + const elQsa = Element.prototype.querySelectorAll; + const isEl = n => n instanceof Element; + // How a node renders its label: generated ::before/::after text, its own + // computed content, and whether it is shown at all. Read for the node and + // its first descendants, so a CSSOM rule (insertRule, adoptedStyleSheets) + // that relabels or hides the text is a change too. + const LOOK = ['content', 'display', 'visibility', 'content-visibility']; + const MAX_LOOK = 64; + const look = (node, parts) => { + if (!isEl(node)) return; + const all = [node, ...R(elQsa, node, ['*'])]; + parts.push(all.length); + for (const n of all.slice(0, MAX_LOOK)) { + try { + const own = R(computed, window, [n]); + for (const p of LOOK) parts.push(R(propOf, own, [p])); + for (const pseudo of ['::before', '::after']) { + const ps = R(computed, window, [n, pseudo]); + for (const p of LOOK) parts.push(R(propOf, ps, [p])); + } + } catch (err) { + parts.push('unreadable'); + } + } + }; const ATTRS = ['role', 'aria-label', 'aria-labelledby', 'aria-describedby', 'title', 'alt', 'value', 'hidden', 'aria-hidden', 'class', 'style', 'id', 'type', 'disabled']; @@ -2226,11 +2256,18 @@ def _normalize_chord(key: str) -> str: for (const a of ATTRS) parts.push(attr(el, a)); parts.push(text(el)); if ('value' in el) parts.push(String(el.value)); + look(el, parts); for (const [id, n] of refs(el)) { parts.push(id, n ? text(n) : null); if (n) for (const a of ATTRS) parts.push(attr(n, a)); + if (n) look(n, parts); + } + if (el.labels) { + for (const l of el.labels) { + parts.push(text(l), attr(l, 'class'), attr(l, 'style')); + look(l, parts); + } } - if (el.labels) for (const l of el.labels) parts.push(text(l), attr(l, 'class'), attr(l, 'style')); return stringify(parts); }; const styled = records => records.some(r => { diff --git a/packages/flow/tests/test_interruptions_browser.py b/packages/flow/tests/test_interruptions_browser.py index d8b3fbbee3..d34c9a7694 100644 --- a/packages/flow/tests/test_interruptions_browser.py +++ b/packages/flow/tests/test_interruptions_browser.py @@ -21,6 +21,9 @@ relabel external label turns "Close" into "Sign out" halt, no GET on pointerover external a stable external label "Close" saved +cssom-rule insertRule turns the ::before "Close" into halt, no GET + "Sign out" on pointerover +cssom-adopted an adopted stylesheet does the same halt, no GET ============= =============================================== ============ """ @@ -107,6 +110,30 @@ } else { b.onclick = () => d.remove(); } +} else if (kind === 'cssom-rule' || kind === 'cssom-adopted') { + // The name is generated CSS content; a CSSOM change relabels it without + // any DOM mutation. + const st = document.createElement('style'); + st.textContent = '.l::before { content: "Close"; }'; + document.head.appendChild(st); + const d = document.createElement('div'); + d.setAttribute('role', 'dialog'); + d.style = 'position:fixed;inset:0;background:#fff;padding:40px;font-size:28px'; + d.innerHTML = '

Update available now

' + + ''; + document.body.appendChild(d); + const b = d.querySelector('#x'); + const rule = '.l::before { content: "Sign out" !important; }'; + b.addEventListener('pointerover', () => { + if (kind === 'cssom-rule') { + st.sheet.insertRule(rule, st.sheet.cssRules.length); + } else { + const sheet = new CSSStyleSheet(); + sheet.replaceSync(rule); + document.adoptedStyleSheets = [...document.adoptedStyleSheets, sheet]; + } + }); + b.onclick = () => { fetch('/logout-everywhere'); d.remove(); }; } else if (kind === 'banner') { const b = document.createElement('div'); b.style = 'position:fixed;top:0;left:0;right:0;height:120px;background:#ffd;' @@ -365,3 +392,33 @@ def test_learning_refuses_a_label_that_changes_as_the_pointer_arrives( def test_learning_keeps_a_stable_external_label(server: str) -> None: learned = _learn(server, "external") assert learned.watchers == ("network",) + + +CSSOM = ["cssom-rule", "cssom-adopted"] + + +@pytest.mark.parametrize("kind", CSSOM) +def test_a_cssom_relabel_as_the_pointer_arrives_halts_with_no_request( + server: str, kind: str, tmp_path: Any +) -> None: + report = _run(server, kind, tmp_path, classes=[LABELLED]) + assert report.success is False + result = report.results[0] + assert result.safety_halt is True, result.error + assert result.delivery_uncertainty is not None + assert result.delivery_uncertainty.cause_type == "LabelChangedDuringClick" + assert result.interstitial_actions[0].label_violations + assert not any("/logout-everywhere" in seen for seen in _Server.seen) + assert not any(seen.startswith("POST") for seen in _Server.seen) + + +@pytest.mark.parametrize("kind", CSSOM) +def test_learning_refuses_a_cssom_relabel_as_the_pointer_arrives( + server: str, kind: str +) -> None: + from openadapt_flow.interruptions import LearnRefused + + with pytest.raises(LearnRefused, match="label changed during the click"): + _learn(server, kind) + assert not any("/logout-everywhere" in seen for seen in _Server.seen) + assert not any(seen.startswith("POST") for seen in _Server.seen) From 223711df6bdd8ea22a5f81af4f16340e8d35bda8 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 9 Nov 2026 01:41:48 -0500 Subject: [PATCH 07/10] fix(flow): refuse a click guard that can't observe the whole label The guard's style snapshot read only the first 64 nodes of a label subtree, so a CSSOM relabel on a 65th node compared equal and the click landed on a control Chromium then named Sign out. The snapshot now reads every node of the control's and each label node's subtree, and is null when one has more than LABEL_GUARD_MAX_NODES (64, the node included) or its walk fails. Arming then refuses and nothing is clicked; at an input, a null snapshot (a label that grew past the cap) is a change and the input is blocked. The node count of each subtree is part of the snapshot. --- packages/flow/docs/INTERRUPTION_RECOVERY.md | 8 +- .../backends/playwright_backend.py | 60 ++++++++++---- .../flow/tests/test_interruptions_browser.py | 80 +++++++++++++++++++ 3 files changed, 131 insertions(+), 17 deletions(-) diff --git a/packages/flow/docs/INTERRUPTION_RECOVERY.md b/packages/flow/docs/INTERRUPTION_RECOVERY.md index 30fe01ad2a..aa4d53b90c 100644 --- a/packages/flow/docs/INTERRUPTION_RECOVERY.md +++ b/packages/flow/docs/INTERRUPTION_RECOVERY.md @@ -57,8 +57,12 @@ A run clears an interruption only when all four of these hold: computed `::before` and `::after` content, their own computed content, and `display` and `visibility`, so a stylesheet rule added through the CSSOM (`insertRule`, `adoptedStyleSheets`) that relabels or hides the - text counts as a change. If any of them changed before a press or click - arrives, or the input is not on + text counts as a change. This evidence must be whole: if the control or + any node that labels it has more than 64 nodes in its subtree (itself + included, `LABEL_GUARD_MAX_NODES`), or the walk can't finish, the guard + refuses to arm and nothing is clicked. A label that grows past that + limit during the click is a change. If any of them changed before a + press or click arrives, or the input is not on the control, the guard cancels that input before the page sees it. The run then halts with the dismissal reported as delivered and its outcome uncertain (`LabelChangedDuringClick`), so it is reconciled, never diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index aec24701d3..bdfb3c7eb8 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -2179,6 +2179,11 @@ def _normalize_chord(key: str) -> str: #: first descendants, so a CSSOM relabel counts), or the input #: is not on the element. A page that erases window listeners #: (``document.open``) fails the guard's heartbeat. Formatted with the key. +#: The most nodes (the node itself included) the click guard observes in the +#: control and in each node that labels it. A larger label can't be observed +#: whole, so the guard refuses to arm and nothing is clicked. +LABEL_GUARD_MAX_NODES = 64 + _CLICK_GUARD_JS = r""" ((key) => { if (Object.prototype.hasOwnProperty.call(window, key)) return; @@ -2206,16 +2211,25 @@ def _normalize_chord(key: str) -> str: const elQsa = Element.prototype.querySelectorAll; const isEl = n => n instanceof Element; // How a node renders its label: generated ::before/::after text, its own - // computed content, and whether it is shown at all. Read for the node and - // its first descendants, so a CSSOM rule (insertRule, adoptedStyleSheets) - // that relabels or hides the text is a change too. + // computed content, and whether it is shown at all, for the node and every + // node under it, so a CSSOM rule (insertRule, adoptedStyleSheets) that + // relabels or hides the text is a change too. The evidence must be whole: + // a subtree of more than MAX_LOOK nodes (the node included), or one whose + // walk fails, can't be observed, and the snapshot is null. Arming then + // refuses; at an input, a null snapshot differs from the armed one. const LOOK = ['content', 'display', 'visibility', 'content-visibility']; - const MAX_LOOK = 64; + const MAX_LOOK = %(max_look)d; const look = (node, parts) => { - if (!isEl(node)) return; - const all = [node, ...R(elQsa, node, ['*'])]; + if (!isEl(node)) return true; + let all; + try { + all = [node, ...R(elQsa, node, ['*'])]; + } catch (err) { + return false; + } + if (all.length > MAX_LOOK) return false; parts.push(all.length); - for (const n of all.slice(0, MAX_LOOK)) { + for (const n of all) { try { const own = R(computed, window, [n]); for (const p of LOOK) parts.push(R(propOf, own, [p])); @@ -2224,9 +2238,10 @@ def _normalize_chord(key: str) -> str: for (const p of LOOK) parts.push(R(propOf, ps, [p])); } } catch (err) { - parts.push('unreadable'); + return false; } } + return true; }; const ATTRS = ['role', 'aria-label', 'aria-labelledby', 'aria-describedby', 'title', 'alt', 'value', 'hidden', 'aria-hidden', 'class', 'style', 'id', @@ -2256,16 +2271,16 @@ def _normalize_chord(key: str) -> str: for (const a of ATTRS) parts.push(attr(el, a)); parts.push(text(el)); if ('value' in el) parts.push(String(el.value)); - look(el, parts); + if (!look(el, parts)) return null; for (const [id, n] of refs(el)) { parts.push(id, n ? text(n) : null); if (n) for (const a of ATTRS) parts.push(attr(n, a)); - if (n) look(n, parts); + if (n && !look(n, parts)) return null; } if (el.labels) { for (const l of el.labels) { parts.push(text(l), attr(l, 'class'), attr(l, 'style')); - look(l, parts); + if (!look(l, parts)) return null; } } return stringify(parts); @@ -2283,7 +2298,11 @@ def _normalize_chord(key: str) -> str: if (!st) return; if (R(take, st.mo, []).length) st.tainted = 'the control or its label changed'; if (styled(R(take, st.docMo, []))) st.tainted = 'a stylesheet changed'; - if (!st.tainted && snapshot(st.el) !== st.snap) st.tainted = 'the control or its label changed'; + if (!st.tainted) { + const now = snapshot(st.el); + if (now === null) st.tainted = 'the label grew too large to observe whole'; + else if (now !== st.snap) st.tainted = 'the control or its label changed'; + } }; const alive = () => { const before = beat; @@ -2326,7 +2345,15 @@ def _normalize_chord(key: str) -> str: R(disconnect, mo, []); R(disconnect, docMo, []); return {ok: false, reason: 'the label could not be watched'}; } - st = {el, secret, snap: snapshot(el), tainted: null, violations: [], events: [], mo, docMo}; + let snap = null; + try { snap = snapshot(el); } catch (err) { snap = null; } + if (snap === null) { + R(disconnect, mo, []); R(disconnect, docMo, []); + return {ok: false, reason: 'the control or its label has more than ' + + MAX_LOOK + ' nodes in one part, or could not be read, so the whole ' + + 'label can\'t be observed'}; + } + st = {el, secret, snap, tainted: null, violations: [], events: [], mo, docMo}; return {ok: true}; }; const finish = secret => { @@ -2343,7 +2370,7 @@ def _normalize_chord(key: str) -> str: value: Object.freeze({arm, finish}), enumerable: false, configurable: false, writable: false, }); -})(%s); +})(%(key)s); """ @@ -6472,7 +6499,10 @@ def _click_guard_key(context: Any) -> Optional[str]: return known key = f"__oaflow_click_{uuid.uuid4().hex}" try: - context.add_init_script(_CLICK_GUARD_JS % json.dumps(key)) + context.add_init_script( + _CLICK_GUARD_JS + % {"key": json.dumps(key), "max_look": LABEL_GUARD_MAX_NODES} + ) context._oaflow_click_guard = key except Exception: # noqa: BLE001 - a stand-in context; guarding refuses return None diff --git a/packages/flow/tests/test_interruptions_browser.py b/packages/flow/tests/test_interruptions_browser.py index d34c9a7694..1dcd0b8c5a 100644 --- a/packages/flow/tests/test_interruptions_browser.py +++ b/packages/flow/tests/test_interruptions_browser.py @@ -24,6 +24,11 @@ cssom-rule insertRule turns the ::before "Close" into halt, no GET "Sign out" on pointerover cssom-adopted an adopted stylesheet does the same halt, no GET +nodes-65 a 65-node label, relabeled by CSSOM on its last halt, no click + node (too large to observe whole) +nodes-64 a stable 64-node label saved +grow a 64-node label grows past the cap on halt, no GET + pointerover ============= =============================================== ============ """ @@ -134,6 +139,32 @@ } }); b.onclick = () => { fetch('/logout-everywhere'); d.remove(); }; +} else if (kind.startsWith('nodes-') || kind === 'grow') { + // A label of N nodes (itself and N-1 empty children); the last child's + // generated content names the button. + const n = kind === 'nodes-65' ? 65 : 64; + const st = document.createElement('style'); + st.textContent = '.t::before { content: "Close"; }'; + document.head.appendChild(st); + const d = document.createElement('div'); + d.setAttribute('role', 'dialog'); + d.style = 'position:fixed;inset:0;background:#fff;padding:40px;font-size:28px'; + d.innerHTML = '

Update available now

' + + ''.repeat(n - 2) + ' ' + + ''; + document.body.appendChild(d); + const b = d.querySelector('#x'); + b.addEventListener('pointerover', () => { + if (kind === 'nodes-65') { + st.sheet.insertRule('.t::before { content: "Sign out" !important; }', 1); + } else if (kind === 'grow') { + document.getElementById('lbl').appendChild(document.createElement('i')); + } + }); + b.onclick = () => { + if (kind !== 'nodes-64') fetch('/logout-everywhere'); + d.remove(); + }; } else if (kind === 'banner') { const b = document.createElement('div'); b.style = 'position:fixed;top:0;left:0;right:0;height:120px;background:#ffd;' @@ -422,3 +453,52 @@ def test_learning_refuses_a_cssom_relabel_as_the_pointer_arrives( _learn(server, kind) assert not any("/logout-everywhere" in seen for seen in _Server.seen) assert not any(seen.startswith("POST") for seen in _Server.seen) + + +def test_a_label_too_large_to_observe_whole_is_never_clicked( + server: str, tmp_path: Any +) -> None: + report = _run(server, "nodes-65", tmp_path, classes=[LABELLED]) + assert report.success is False + event = report.results[0].interstitial_actions[0] + assert event.delivered is False + assert "observed" in (event.error or ""), event.error + assert report.results[0].delivery_uncertainty is None + assert not any("/logout-everywhere" in seen for seen in _Server.seen) + assert not any(seen.startswith("POST") for seen in _Server.seen) + + +def test_learning_refuses_a_label_too_large_to_observe_whole(server: str) -> None: + from openadapt_flow.interruptions import LearnRefused + + with pytest.raises(LearnRefused, match="observed"): + _learn(server, "nodes-65") + assert not any("/logout-everywhere" in seen for seen in _Server.seen) + assert not any(seen.startswith("POST") for seen in _Server.seen) + + +def test_a_stable_label_at_the_cap_is_cleared_and_the_save_lands( + server: str, tmp_path: Any +) -> None: + report = _run(server, "nodes-64", tmp_path, classes=[LABELLED]) + assert report.success is True, report.results[0].error + assert report.results[0].interstitial_actions[0].label_violations == [] + assert "POST /save" in _Server.seen + + +def test_learning_keeps_a_stable_label_at_the_cap(server: str) -> None: + learned = _learn(server, "nodes-64") + assert learned.watchers == ("network",) + + +def test_a_label_that_grows_past_the_cap_during_the_click_is_blocked( + server: str, tmp_path: Any +) -> None: + report = _run(server, "grow", tmp_path, classes=[LABELLED]) + assert report.success is False + result = report.results[0] + assert result.safety_halt is True + assert result.delivery_uncertainty is not None + assert result.delivery_uncertainty.cause_type == "LabelChangedDuringClick" + assert not any("/logout-everywhere" in seen for seen in _Server.seen) + assert not any(seen.startswith("POST") for seen in _Server.seen) From 1cfff129933755e043ab612f2e5e48e684377e28 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 9 Nov 2026 03:14:08 -0500 Subject: [PATCH 08/10] fix(flow): block the input when the click guard's check can't finish A page getter that threw on a property the guard's snapshot read (an own button.value getter, installed on hover) aborted the capture listener before it cancelled the input, so a CSSOM-relabeled click landed. Each guarded listener now blocks unless the whole check finishes and passes: any exception blocks the input and records a violation. The guard reads only through built-ins captured at install (brand-checked getters for value, labels, tagName and the like, NodeList and MutationRecord getters, index loops, string-built snapshots), so a getter or method the page defines later never runs. arm and finish answer with strings the guard built, parsed in Python, so a patched toJSON or Array method can't shape the answer. --- packages/flow/docs/INTERRUPTION_RECOVERY.md | 8 +- .../backends/playwright_backend.py | 386 ++++++++++++------ .../flow/tests/test_interruptions_browser.py | 78 ++++ 3 files changed, 352 insertions(+), 120 deletions(-) diff --git a/packages/flow/docs/INTERRUPTION_RECOVERY.md b/packages/flow/docs/INTERRUPTION_RECOVERY.md index aa4d53b90c..caf9a65b71 100644 --- a/packages/flow/docs/INTERRUPTION_RECOVERY.md +++ b/packages/flow/docs/INTERRUPTION_RECOVERY.md @@ -61,8 +61,12 @@ A run clears an interruption only when all four of these hold: any node that labels it has more than 64 nodes in its subtree (itself included, `LABEL_GUARD_MAX_NODES`), or the walk can't finish, the guard refuses to arm and nothing is clicked. A label that grows past that - limit during the click is a change. If any of them changed before a - press or click arrives, or the input is not on + limit during the click is a change. The guard reads all of this only + through the browser's own getters and methods, captured before any page + script runs, so a getter or method the page defines later never runs. + An input is let through only when the whole check finishes and passes: + an error anywhere in it blocks the input and counts as a change. If any + of them changed before a press or click arrives, or the input is not on the control, the guard cancels that input before the page sees it. The run then halts with the dismissal reported as delivered and its outcome uncertain (`LabelChangedDuringClick`), so it is reconciled, never diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index bdfb3c7eb8..c8202ef1ba 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -2187,14 +2187,36 @@ def _normalize_chord(key: str) -> str: _CLICK_GUARD_JS = r""" ((key) => { if (Object.prototype.hasOwnProperty.call(window, key)) return; + // Every read goes through a built-in captured here, before any page + // script, and called with Reflect.apply: a getter or method the page + // defines later (on a node, a prototype, Array or Object) never runs. + // Lists are walked by index and results are built as strings, so a + // patched iterator, push or toJSON can't change them either. const R = Reflect.apply; + const defProp = Reflect.defineProperty; + const getOwn = Object.getOwnPropertyDescriptor; + const getter = (proto, name) => { + const d = proto ? getOwn(proto, name) : undefined; + return d && d.get ? d.get : null; + }; const getAttr = Element.prototype.getAttribute; - const textOf = Object.getOwnPropertyDescriptor(Node.prototype, 'textContent').get; - const connected = Object.getOwnPropertyDescriptor(Node.prototype, 'isConnected').get; + const tagOf = getter(Element.prototype, 'tagName'); + const textOf = getter(Node.prototype, 'textContent'); + const connected = getter(Node.prototype, 'isConnected'); + const nameOf = getter(Node.prototype, 'nodeName'); + const parentOf = getter(Node.prototype, 'parentNode'); const rootOf = Node.prototype.getRootNode; const qsa = Document.prototype.querySelectorAll; + const elQsa = Element.prototype.querySelectorAll; const docById = Document.prototype.getElementById; const fragById = DocumentFragment.prototype.getElementById; + const listLen = getter(NodeList.prototype, 'length'); + const listItem = NodeList.prototype.item; + const recType = getter(MutationRecord.prototype, 'type'); + const recTarget = getter(MutationRecord.prototype, 'target'); + const recAdded = getter(MutationRecord.prototype, 'addedNodes'); + const recRemoved = getter(MutationRecord.prototype, 'removedNodes'); + const evType = getter(Event.prototype, 'type'); const path = Event.prototype.composedPath; const prevent = Event.prototype.preventDefault; const stopNow = Event.prototype.stopImmediatePropagation; @@ -2208,40 +2230,93 @@ def _normalize_chord(key: str) -> str: const stringify = JSON.stringify; const computed = window.getComputedStyle; const propOf = CSSStyleDeclaration.prototype.getPropertyValue; - const elQsa = Element.prototype.querySelectorAll; - const isEl = n => n instanceof Element; - // How a node renders its label: generated ::before/::after text, its own - // computed content, and whether it is shown at all, for the node and every - // node under it, so a CSSOM rule (insertRule, adoptedStyleSheets) that - // relabels or hides the text is a change too. The evidence must be whole: - // a subtree of more than MAX_LOOK nodes (the node included), or one whose - // walk fails, can't be observed, and the snapshot is null. Arming then - // refuses; at an input, a null snapshot differs from the armed one. + const freeze = Object.freeze; + // Brand-checked getters: a built-in getter throws on the wrong kind of + // object, so trying each one reads the element's own built-in value. + const VALUE_TYPES = ['HTMLInputElement', 'HTMLTextAreaElement', + 'HTMLSelectElement', 'HTMLButtonElement', 'HTMLOptionElement', + 'HTMLOutputElement', 'HTMLDataElement', 'HTMLMeterElement', + 'HTMLProgressElement', 'HTMLLIElement']; + const LABEL_TYPES = ['HTMLInputElement', 'HTMLTextAreaElement', + 'HTMLSelectElement', 'HTMLButtonElement', 'HTMLOutputElement', + 'HTMLMeterElement', 'HTMLProgressElement']; + const collect = (types, name) => { + const out = []; + for (let i = 0; i < types.length; i += 1) { + const ctor = window[types[i]]; + const g = ctor ? getter(ctor.prototype, name) : null; + if (g) defProp(out, out.length, {value: g, enumerable: true}); + } + return out; + }; + const valueGetters = collect(VALUE_TYPES, 'value'); + const labelGetters = collect(LABEL_TYPES, 'labels'); + const branded = (getters, node) => { + for (let i = 0; i < getters.length; i += 1) { + let v; + let ok = true; + try { v = R(getters[i], node, []); } catch (err) { ok = false; } + if (ok) return {found: true, v}; + } + return {found: false, v: null}; + }; + const put = (list, v) => { defProp(list, list.length, {value: v, enumerable: true}); }; + const nodes = list => { + const out = []; + if (!list) return out; + const n = R(listLen, list, []); + for (let i = 0; i < n; i += 1) put(out, R(listItem, list, [i])); + return out; + }; + const isEl = n => { + if (!n) return false; + try { R(tagOf, n, []); return true; } catch (err) { return false; } + }; + const enc = v => stringify(v === undefined ? null : v) + '\u0001'; + const attr = (el, a) => R(getAttr, el, [a]); + const text = n => R(textOf, n, []); + const words = s => { + const out = []; + let cur = ''; + const str = s === null ? '' : '' + s; + for (let i = 0; i < str.length; i += 1) { + const c = str[i]; + if (c === ' ' || c === '\t' || c === '\n' || c === '\r' || c === '\f') { + if (cur) put(out, cur); + cur = ''; + } else { + cur += c; + } + } + if (cur) put(out, cur); + return out; + }; + const byId = (root, id) => { + try { return R(docById, root, [id]); } catch (err) { /* not a document */ } + try { return R(fragById, root, [id]); } catch (err) { return null; } + }; const LOOK = ['content', 'display', 'visibility', 'content-visibility']; + const PSEUDO = ['::before', '::after']; const MAX_LOOK = %(max_look)d; - const look = (node, parts) => { - if (!isEl(node)) return true; - let all; - try { - all = [node, ...R(elQsa, node, ['*'])]; - } catch (err) { - return false; - } - if (all.length > MAX_LOOK) return false; - parts.push(all.length); - for (const n of all) { - try { - const own = R(computed, window, [n]); - for (const p of LOOK) parts.push(R(propOf, own, [p])); - for (const pseudo of ['::before', '::after']) { - const ps = R(computed, window, [n, pseudo]); - for (const p of LOOK) parts.push(R(propOf, ps, [p])); - } - } catch (err) { - return false; + // How a node renders its label, for the node and every node under it: + // generated ::before/::after text, its own computed content, and whether + // it is shown. More than MAX_LOOK nodes (the node included) can't be + // observed whole: the snapshot is then null. + const look = node => { + if (!isEl(node)) return ''; + const all = nodes(R(elQsa, node, ['*'])); + if (all.length + 1 > MAX_LOOK) return null; + let out = enc(all.length + 1); + for (let i = -1; i < all.length; i += 1) { + const n = i < 0 ? node : all[i]; + const own = R(computed, window, [n]); + for (let j = 0; j < LOOK.length; j += 1) out += enc(R(propOf, own, [LOOK[j]])); + for (let k = 0; k < PSEUDO.length; k += 1) { + const ps = R(computed, window, [n, PSEUDO[k]]); + for (let j = 0; j < LOOK.length; j += 1) out += enc(R(propOf, ps, [LOOK[j]])); } } - return true; + return out; }; const ATTRS = ['role', 'aria-label', 'aria-labelledby', 'aria-describedby', 'title', 'alt', 'value', 'hidden', 'aria-hidden', 'class', 'style', 'id', @@ -2249,60 +2324,97 @@ def _normalize_chord(key: str) -> str: const EVENTS = ['pointerdown', 'mousedown', 'pointerup', 'mouseup', 'click', 'auxclick', 'dblclick', 'contextmenu', 'keydown', 'keypress', 'keyup', 'touchstart', 'touchend']; - const probe = key + '_probe'; - let beat = 0; - let st = null; - const attr = (el, a) => R(getAttr, el, [a]); - const text = n => R(textOf, n, []); + const attrs = n => { + let out = ''; + for (let i = 0; i < ATTRS.length; i += 1) out += enc(attr(n, ATTRS[i])); + return out; + }; + // The nodes that label the control: [id, node] pairs, then its labels. const refs = el => { const out = []; const root = R(rootOf, el, []); - for (const a of ['aria-labelledby', 'aria-describedby']) { - for (const id of (attr(el, a) || '').split(/\s+/).filter(Boolean)) { - const n = root instanceof Document ? R(docById, root, [id]) - : (root instanceof DocumentFragment ? R(fragById, root, [id]) : null); - out.push([id, n]); - } + const kinds = ['aria-labelledby', 'aria-describedby']; + for (let k = 0; k < kinds.length; k += 1) { + const ids = words(attr(el, kinds[k])); + for (let i = 0; i < ids.length; i += 1) put(out, [ids[i], byId(root, ids[i])]); } return out; }; + const labels = el => { + const b = branded(labelGetters, el); + return b.found ? nodes(b.v) : []; + }; + // The whole evidence as one string, or null when it can't be read whole. + // Throws on anything unreadable; callers treat a throw as null. const snapshot = el => { - const parts = [R(connected, el, []), el.tagName]; - for (const a of ATTRS) parts.push(attr(el, a)); - parts.push(text(el)); - if ('value' in el) parts.push(String(el.value)); - if (!look(el, parts)) return null; - for (const [id, n] of refs(el)) { - parts.push(id, n ? text(n) : null); - if (n) for (const a of ATTRS) parts.push(attr(n, a)); - if (n && !look(n, parts)) return null; - } - if (el.labels) { - for (const l of el.labels) { - parts.push(text(l), attr(l, 'class'), attr(l, 'style')); - if (!look(l, parts)) return null; + let out = enc(R(connected, el, [])) + enc(R(tagOf, el, [])) + attrs(el) + + enc(text(el)); + const value = branded(valueGetters, el); + out += enc(value.found) + enc(value.found ? '' + value.v : null); + const own = look(el); + if (own === null) return null; + out += own; + const rs = refs(el); + for (let i = 0; i < rs.length; i += 1) { + const n = rs[i][1]; + out += enc(rs[i][0]) + enc(n ? text(n) : null); + if (n) { + out += attrs(n); + const seen = look(n); + if (seen === null) return null; + out += seen; } } - return stringify(parts); + const ls = labels(el); + for (let i = 0; i < ls.length; i += 1) { + out += enc(text(ls[i])) + enc(attr(ls[i], 'class')) + enc(attr(ls[i], 'style')); + const seen = look(ls[i]); + if (seen === null) return null; + out += seen; + } + return out; + }; + const styledNode = n => { + const name = R(nameOf, n, []); + return name === 'STYLE' || name === 'LINK'; }; - const styled = records => records.some(r => { - const t = r.target; - if (r.type === 'characterData') return !!(t.parentNode && t.parentNode.nodeName === 'STYLE'); - if (t && (t.nodeName === 'STYLE' || t.nodeName === 'LINK')) return true; - for (const n of [...r.addedNodes, ...r.removedNodes]) { - if (n.nodeName === 'STYLE' || n.nodeName === 'LINK') return true; + const styled = records => { + for (let i = 0; i < records.length; i += 1) { + const r = records[i]; + const t = R(recTarget, r, []); + if (R(recType, r, []) === 'characterData') { + const p = R(parentOf, t, []); + if (p && styledNode(p)) return true; + continue; + } + if (t && styledNode(t)) return true; + const moved = [R(recAdded, r, []), R(recRemoved, r, [])]; + for (let j = 0; j < 2; j += 1) { + const ns = nodes(moved[j]); + for (let k = 0; k < ns.length; k += 1) if (styledNode(ns[k])) return true; + } } return false; - }); - const flush = () => { - if (!st) return; + }; + const probe = key + '_probe'; + let beat = 0; + let st = null; + // Why the evidence no longer holds, or null. Throws when it can't tell. + const changed = () => { + if (st.tainted) return st.tainted; if (R(take, st.mo, []).length) st.tainted = 'the control or its label changed'; - if (styled(R(take, st.docMo, []))) st.tainted = 'a stylesheet changed'; - if (!st.tainted) { + else if (styled(R(take, st.docMo, []))) st.tainted = 'a stylesheet changed'; + else { const now = snapshot(st.el); if (now === null) st.tainted = 'the label grew too large to observe whole'; else if (now !== st.snap) st.tainted = 'the control or its label changed'; } + return st.tainted; + }; + const onControl = e => { + const p = R(path, e, []); + for (let i = 0; i < p.length; i += 1) if (p[i] === st.el) return true; + return false; }; const alive = () => { const before = beat; @@ -2312,62 +2424,87 @@ def _normalize_chord(key: str) -> str: R(add, window, [probe, () => { beat += 1; }, true]); const guard = e => { if (!st) return; - flush(); - let why = st.tainted; - if (!why) { - let inside = false; - try { inside = R(path, e, []).includes(st.el); } catch (err) { inside = false; } - if (!inside) why = 'the input is not on the control'; - } - st.events.push(e.type + (e.isTrusted ? '' : ' (synthetic)')); - if (why) { - R(prevent, e, []); - R(stopNow, e, []); - st.violations.push(e.type + ': ' + why); + // Blocked unless the whole check finishes and passes: an exception + // anywhere in it (a page getter, an unreadable node) blocks the input. + let why = 'the label evidence could not be read'; + let type = '?'; + try { + type = '' + R(evType, e, []); + const c = changed(); + why = c ? c : (onControl(e) ? null : 'the input is not on the control'); + } catch (err) { + why = 'the label evidence could not be read'; + st.tainted = st.tainted || why; + } finally { + try { put(st.events, type); } catch (err) { /* recorded below */ } + if (why !== null) { + R(prevent, e, []); + R(stopNow, e, []); + put(st.violations, type + ': ' + why); + } } }; - for (const t of EVENTS) R(add, window, [t, guard, true]); + for (let i = 0; i < EVENTS.length; i += 1) R(add, window, [EVENTS[i], guard, true]); + const answer = (ok, reason) => (ok ? 'ok' : 'refused: ' + reason); const arm = (tokenAttr, secret) => { - if (st) { st.violations.push('armed twice'); return {ok: false, reason: 'the guard is already armed'}; } - if (!alive()) return {ok: false, reason: "the guard's listeners are not in this document"}; - const found = R(qsa, document, ['[' + tokenAttr + ']']); - if (found.length !== 1) return {ok: false, reason: 'the control is not in this document'}; - const el = found[0]; - const opts = {attributes: true, characterData: true, childList: true, subtree: true}; - const mo = new MO(() => { if (st) st.tainted = 'the control or its label changed'; }); - const docMo = new MO(rs => { if (st && styled(rs)) st.tainted = 'a stylesheet changed'; }); + if (st) { + put(st.violations, 'armed twice'); + return answer(false, 'the guard is already armed'); + } + if (!alive()) return answer(false, "the guard's listeners are not in this document"); + let mo = null; + let docMo = null; try { + const found = nodes(R(qsa, document, ['[' + tokenAttr + ']'])); + if (found.length !== 1) return answer(false, 'the control is not in this document'); + const el = found[0]; + const opts = {attributes: true, characterData: true, childList: true, subtree: true}; + mo = new MO(() => { if (st) st.tainted = 'the control or its label changed'; }); + docMo = new MO(rs => { + if (!st) return; + try { + if (styled(rs)) st.tainted = 'a stylesheet changed'; + } catch (err) { + st.tainted = 'the label evidence could not be read'; + } + }); R(observe, mo, [el, opts]); - for (const [, n] of refs(el)) if (n) R(observe, mo, [n, opts]); - if (el.labels) for (const l of el.labels) R(observe, mo, [l, opts]); - R(observe, docMo, [document, {characterData: true, childList: true, subtree: true, attributes: true, attributeFilter: ['href', 'media', 'disabled']}]); + const rs = refs(el); + for (let i = 0; i < rs.length; i += 1) if (rs[i][1]) R(observe, mo, [rs[i][1], opts]); + const ls = labels(el); + for (let i = 0; i < ls.length; i += 1) R(observe, mo, [ls[i], opts]); + R(observe, docMo, [document, {characterData: true, childList: true, + subtree: true, attributes: true, attributeFilter: ['href', 'media', 'disabled']}]); + const snap = snapshot(el); + if (snap === null) throw new Error('too large'); + st = {el, secret, snap, tainted: null, violations: [], events: [], mo, docMo}; + return answer(true); } catch (err) { - R(disconnect, mo, []); R(disconnect, docMo, []); - return {ok: false, reason: 'the label could not be watched'}; + try { if (mo) R(disconnect, mo, []); if (docMo) R(disconnect, docMo, []); } catch (e2) { /* gone */ } + return answer(false, 'the control or its label has more than ' + MAX_LOOK + + " nodes in one part, or could not be read, so the whole label can't be observed"); } - let snap = null; - try { snap = snapshot(el); } catch (err) { snap = null; } - if (snap === null) { - R(disconnect, mo, []); R(disconnect, docMo, []); - return {ok: false, reason: 'the control or its label has more than ' - + MAX_LOOK + ' nodes in one part, or could not be read, so the whole ' - + 'label can\'t be observed'}; - } - st = {el, secret, snap, tainted: null, violations: [], events: [], mo, docMo}; - return {ok: true}; + }; + const list = xs => { + let out = '['; + for (let i = 0; i < xs.length; i += 1) out += (i ? ',' : '') + stringify('' + xs[i]); + return out + ']'; }; const finish = secret => { - if (!st || st.secret !== secret) return {ok: false, violations: ['the guard was not armed by this click'], events: []}; + if (!st || st.secret !== secret) { + return '{"ok":false,"violations":["the guard was not armed by this click"],"events":[]}'; + } // A change after the last input is the click's own effect (the dialog // closing); only an input judged after a change is a violation. - const out = {ok: true, violations: st.violations.slice(), events: st.events.slice()}; - if (!alive()) out.violations.push("the guard's listeners were removed during the click"); - R(disconnect, st.mo, []); R(disconnect, st.docMo, []); + const violations = st.violations; + if (!alive()) put(violations, "the guard's listeners were removed during the click"); + const out = '{"ok":true,"violations":' + list(violations) + ',"events":' + list(st.events) + '}'; + try { R(disconnect, st.mo, []); R(disconnect, st.docMo, []); } catch (err) { /* gone */ } st = null; return out; }; - Object.defineProperty(window, key, { - value: Object.freeze({arm, finish}), enumerable: false, + defProp(window, key, { + value: freeze({arm, finish}), enumerable: false, configurable: false, writable: false, }); })(%(key)s); @@ -3761,15 +3898,18 @@ def structural_label_guard( try: armed = guard.scope.evaluate( "([k, a, s]) => { const g = window[k];" - " return g ? g.arm(a, s) : {ok: false," - " reason: 'this document has no click guard (it loaded before" - " the guard was installed)'}; }", + " return g ? g.arm(a, s) : 'refused: this document has no click" + " guard (it loaded before the guard was installed)'; }", [key, self._token_attribute(guard.token), secret], ) except Exception as exc: # noqa: BLE001 - no guard, no click - armed = {"ok": False, "reason": f"arming failed ({type(exc).__name__})"} - if not isinstance(armed, dict) or armed.get("ok") is not True: - reason = armed.get("reason") if isinstance(armed, dict) else None + armed = f"refused: arming failed ({type(exc).__name__})" + if armed != "ok": + reason = ( + armed[len("refused: ") :] + if isinstance(armed, str) and armed.startswith("refused: ") + else None + ) report.refusal = f"the click guard did not arm: {reason or 'unknown'}" yield report return @@ -3784,7 +3924,17 @@ def structural_label_guard( ) except Exception: # noqa: BLE001 - the document went away seen = None - if not isinstance(seen, dict) or seen.get("ok") is not True: + # The guard answers with a string it built itself, so nothing the + # page patched (toJSON, Array methods) can shape the answer. + try: + seen = json.loads(seen) if isinstance(seen, str) else None + except ValueError: + seen = None + if ( + not isinstance(seen, dict) + or seen.get("ok") is not True + or not isinstance(seen.get("violations"), list) + ): report.violations.append( "the click guard could not be read after the click" ) diff --git a/packages/flow/tests/test_interruptions_browser.py b/packages/flow/tests/test_interruptions_browser.py index 1dcd0b8c5a..2ec38d5d6d 100644 --- a/packages/flow/tests/test_interruptions_browser.py +++ b/packages/flow/tests/test_interruptions_browser.py @@ -29,6 +29,9 @@ nodes-64 a stable 64-node label saved grow a 64-node label grows past the cap on halt, no GET pointerover +getter-* on pointerover a page getter that throws, plus halt, no GET + a CSSOM relabel to "Sign out" +getter-stable the same throwing getter, no relabel saved ============= =============================================== ============ """ @@ -165,6 +168,37 @@ if (kind !== 'nodes-64') fetch('/logout-everywhere'); d.remove(); }; +} else if (kind.startsWith('getter-')) { + // A page getter that throws when read, installed as the pointer arrives, + // together with a CSSOM relabel; the stable variant only installs it. + const st = document.createElement('style'); + st.textContent = '.l::before { content: "Close"; }'; + document.head.appendChild(st); + const d = document.createElement('div'); + d.setAttribute('role', 'dialog'); + d.style = 'position:fixed;inset:0;background:#fff;padding:40px;font-size:28px'; + d.innerHTML = '

Update available now

' + + ''; + document.body.appendChild(d); + const b = d.querySelector('#x'); + const boom = { get() { throw new Error('boom'); }, configurable: true }; + b.addEventListener('pointerover', () => { + const what = kind.slice('getter-'.length); + if (what === 'proto') { + Object.defineProperty(HTMLButtonElement.prototype, 'value', boom); + } else if (what === 'stable') { + Object.defineProperty(b, 'value', boom); + } else { + Object.defineProperty(b, what, boom); + } + if (what !== 'stable') { + st.sheet.insertRule('.l::before { content: "Sign out" !important; }', 1); + } + }); + b.onclick = () => { + if (kind !== 'getter-stable') fetch('/logout-everywhere'); + d.remove(); + }; } else if (kind === 'banner') { const b = document.createElement('div'); b.style = 'position:fixed;top:0;left:0;right:0;height:120px;background:#ffd;' @@ -502,3 +536,47 @@ def test_a_label_that_grows_past_the_cap_during_the_click_is_blocked( assert result.delivery_uncertainty.cause_type == "LabelChangedDuringClick" assert not any("/logout-everywhere" in seen for seen in _Server.seen) assert not any(seen.startswith("POST") for seen in _Server.seen) + + +GETTERS = [ + "getter-value", + "getter-labels", + "getter-textContent", + "getter-tagName", + "getter-proto", +] + + +@pytest.mark.parametrize("kind", GETTERS) +def test_a_throwing_page_getter_cannot_let_a_relabeled_click_through( + server: str, kind: str, tmp_path: Any +) -> None: + report = _run(server, kind, tmp_path, classes=[LABELLED]) + assert report.success is False + result = report.results[0] + assert result.safety_halt is True, result.error + assert result.delivery_uncertainty is not None + assert result.delivery_uncertainty.cause_type == "LabelChangedDuringClick" + assert result.interstitial_actions[0].label_violations + assert not any("/logout-everywhere" in seen for seen in _Server.seen) + assert not any(seen.startswith("POST") for seen in _Server.seen) + + +@pytest.mark.parametrize("kind", GETTERS) +def test_learning_refuses_a_relabeled_click_behind_a_throwing_getter( + server: str, kind: str +) -> None: + from openadapt_flow.interruptions import LearnRefused + + with pytest.raises(LearnRefused, match="label changed during the click"): + _learn(server, kind) + assert not any("/logout-everywhere" in seen for seen in _Server.seen) + assert not any(seen.startswith("POST") for seen in _Server.seen) + + +def test_a_throwing_page_getter_alone_does_not_block_a_stable_label( + server: str, tmp_path: Any +) -> None: + report = _run(server, "getter-stable", tmp_path, classes=[LABELLED]) + assert report.success is True, report.results[0].error + assert "POST /save" in _Server.seen From c1508e0b5fb55d229f3e3675d5493a5d3bb39b2d Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 9 Nov 2026 05:17:47 -0500 Subject: [PATCH 09/10] fix(flow): keep the click guard from reading any inherited property The guard passed ordinary objects to built-ins: property descriptors in its list helper and MutationObserver options. Converting them reads absent fields through Object.prototype, so a page getter for configurable, writable or attributeOldValue ran from inside the guard. Every dictionary the guard hands a built-in now has a null prototype and only own data fields, the observer's attributeFilter is a closed iterable (own iterator, own next, null-prototype results), the heartbeat event type is random, and the listener blocks before it records. The evaluate wrappers no longer destructure their arguments. The init script carries a sourceURL so a test can attribute frames to it: the new browser tests trap Object.prototype keys and the built-ins the guard could reach, and require that no call from the guard reaches them, for a stable click and a relabel, in replay and learning. --- packages/flow/docs/INTERRUPTION_RECOVERY.md | 5 + .../backends/playwright_backend.py | 109 ++++++++--- .../flow/tests/test_interruptions_browser.py | 183 ++++++++++++++++++ 3 files changed, 267 insertions(+), 30 deletions(-) diff --git a/packages/flow/docs/INTERRUPTION_RECOVERY.md b/packages/flow/docs/INTERRUPTION_RECOVERY.md index caf9a65b71..db60feac86 100644 --- a/packages/flow/docs/INTERRUPTION_RECOVERY.md +++ b/packages/flow/docs/INTERRUPTION_RECOVERY.md @@ -64,6 +64,11 @@ A run clears an interruption only when all four of these hold: limit during the click is a change. The guard reads all of this only through the browser's own getters and methods, captured before any page script runs, so a getter or method the page defines later never runs. + Every dictionary it hands to a built-in (a property descriptor, observer + options) has no prototype, and every list a built-in iterates brings its + own iterator, so no inherited property is ever read. A test traps + `Object.prototype` keys and the built-ins the guard could reach, and + checks that no call from the guard's script reaches them. An input is let through only when the whole check finishes and passes: an error anywhere in it blocks the input and counts as a change. If any of them changed before a press or click arrives, or the input is not on diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index c8202ef1ba..604f7d7810 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -2184,6 +2184,10 @@ def _normalize_chord(key: str) -> str: #: whole, so the guard refuses to arm and nothing is clicked. LABEL_GUARD_MAX_NODES = 64 +#: The click guard script's name in a stack trace (``//# sourceURL``), so a +#: test can tell the guard's own frames from page code. +CLICK_GUARD_SOURCE_URL = "oaflow-click-guard.js" + _CLICK_GUARD_JS = r""" ((key) => { if (Object.prototype.hasOwnProperty.call(window, key)) return; @@ -2191,7 +2195,12 @@ def _normalize_chord(key: str) -> str: // script, and called with Reflect.apply: a getter or method the page // defines later (on a node, a prototype, Array or Object) never runs. // Lists are walked by index and results are built as strings, so a - // patched iterator, push or toJSON can't change them either. + // patched iterator, push or toJSON can't change them either. Every + // dictionary handed to a built-in (a property descriptor, observer + // options) has a null prototype and only own data properties, and a list + // a built-in iterates is a closed iterable, so converting them never + // reads an inherited property the page may have defined. Entry points are + // named oaflowClickGuard* so their frames can be told apart in a trace. const R = Reflect.apply; const defProp = Reflect.defineProperty; const getOwn = Object.getOwnPropertyDescriptor; @@ -2231,6 +2240,23 @@ def _normalize_chord(key: str) -> str: const computed = window.getComputedStyle; const propOf = CSSStyleDeclaration.prototype.getPropertyValue; const freeze = Object.freeze; + const ITER = Symbol.iterator; + // A property descriptor with no prototype: ToPropertyDescriptor reads only + // what is set here. + const dataDesc = v => ({__proto__: null, value: v, writable: true, + enumerable: true, configurable: true}); + // A list a built-in converts as a sequence, iterated without + // Array.prototype: its own iterator, own next, and null-prototype results. + const seq = arr => { + const o = {__proto__: null}; + defProp(o, ITER, {__proto__: null, value: () => { + let i = 0; + return {__proto__: null, next: () => (i < arr.length + ? {__proto__: null, done: false, value: arr[i++]} + : {__proto__: null, done: true, value: undefined})}; + }}); + return o; + }; // Brand-checked getters: a built-in getter throws on the wrong kind of // object, so trying each one reads the element's own built-in value. const VALUE_TYPES = ['HTMLInputElement', 'HTMLTextAreaElement', @@ -2245,7 +2271,7 @@ def _normalize_chord(key: str) -> str: for (let i = 0; i < types.length; i += 1) { const ctor = window[types[i]]; const g = ctor ? getter(ctor.prototype, name) : null; - if (g) defProp(out, out.length, {value: g, enumerable: true}); + if (g) defProp(out, out.length, dataDesc(g)); } return out; }; @@ -2256,11 +2282,11 @@ def _normalize_chord(key: str) -> str: let v; let ok = true; try { v = R(getters[i], node, []); } catch (err) { ok = false; } - if (ok) return {found: true, v}; + if (ok) return {__proto__: null, found: true, v}; } - return {found: false, v: null}; + return {__proto__: null, found: false, v: null}; }; - const put = (list, v) => { defProp(list, list.length, {value: v, enumerable: true}); }; + const put = (list, v) => { defProp(list, list.length, dataDesc(v)); }; const nodes = list => { const out = []; if (!list) return out; @@ -2396,7 +2422,8 @@ def _normalize_chord(key: str) -> str: } return false; }; - const probe = key + '_probe'; + // A random event type no page code can know, for the heartbeat. + const probe = 'oaflow' + Math.random() + Math.random(); let beat = 0; let st = null; // Why the evidence no longer holds, or null. Throws when it can't tell. @@ -2421,8 +2448,9 @@ def _normalize_chord(key: str) -> str: try { R(dispatch, window, [new Ev(probe)]); } catch (e) { return false; } return beat === before + 1; }; - R(add, window, [probe, () => { beat += 1; }, true]); - const guard = e => { + const oaflowClickGuardBeat = () => { beat += 1; }; + R(add, window, [probe, oaflowClickGuardBeat, true]); + const oaflowClickGuardListener = e => { if (!st) return; // Blocked unless the whole check finishes and passes: an exception // anywhere in it (a page getter, an unreadable node) blocks the input. @@ -2436,17 +2464,24 @@ def _normalize_chord(key: str) -> str: why = 'the label evidence could not be read'; st.tainted = st.tainted || why; } finally { - try { put(st.events, type); } catch (err) { /* recorded below */ } + // Block first: nothing recorded after the decision can undo it. if (why !== null) { R(prevent, e, []); R(stopNow, e, []); - put(st.violations, type + ': ' + why); + } + try { + put(st.events, type); + if (why !== null) put(st.violations, type + ': ' + why); + } catch (err) { + st.tainted = st.tainted || 'the label evidence could not be read'; } } }; - for (let i = 0; i < EVENTS.length; i += 1) R(add, window, [EVENTS[i], guard, true]); + for (let i = 0; i < EVENTS.length; i += 1) { + R(add, window, [EVENTS[i], oaflowClickGuardListener, true]); + } const answer = (ok, reason) => (ok ? 'ok' : 'refused: ' + reason); - const arm = (tokenAttr, secret) => { + const oaflowClickGuardArm = (tokenAttr, secret) => { if (st) { put(st.violations, 'armed twice'); return answer(false, 'the guard is already armed'); @@ -2458,9 +2493,13 @@ def _normalize_chord(key: str) -> str: const found = nodes(R(qsa, document, ['[' + tokenAttr + ']'])); if (found.length !== 1) return answer(false, 'the control is not in this document'); const el = found[0]; - const opts = {attributes: true, characterData: true, childList: true, subtree: true}; - mo = new MO(() => { if (st) st.tainted = 'the control or its label changed'; }); - docMo = new MO(rs => { + const opts = {__proto__: null, attributes: true, characterData: true, + childList: true, subtree: true}; + const oaflowClickGuardObserved = () => { + if (st) st.tainted = 'the control or its label changed'; + }; + mo = new MO(oaflowClickGuardObserved); + docMo = new MO(function oaflowClickGuardStyled(rs) { if (!st) return; try { if (styled(rs)) st.tainted = 'a stylesheet changed'; @@ -2473,24 +2512,28 @@ def _normalize_chord(key: str) -> str: for (let i = 0; i < rs.length; i += 1) if (rs[i][1]) R(observe, mo, [rs[i][1], opts]); const ls = labels(el); for (let i = 0; i < ls.length; i += 1) R(observe, mo, [ls[i], opts]); - R(observe, docMo, [document, {characterData: true, childList: true, - subtree: true, attributes: true, attributeFilter: ['href', 'media', 'disabled']}]); + R(observe, docMo, [document, {__proto__: null, characterData: true, + childList: true, subtree: true, attributes: true, + attributeFilter: seq(['href', 'media', 'disabled'])}]); const snap = snapshot(el); - if (snap === null) throw new Error('too large'); - st = {el, secret, snap, tainted: null, violations: [], events: [], mo, docMo}; - return answer(true); + if (snap !== null) { + st = {__proto__: null, el, secret, snap, tainted: null, violations: [], + events: [], mo, docMo}; + return answer(true); + } } catch (err) { - try { if (mo) R(disconnect, mo, []); if (docMo) R(disconnect, docMo, []); } catch (e2) { /* gone */ } - return answer(false, 'the control or its label has more than ' + MAX_LOOK - + " nodes in one part, or could not be read, so the whole label can't be observed"); + /* refused below */ } + try { if (mo) R(disconnect, mo, []); if (docMo) R(disconnect, docMo, []); } catch (e2) { /* gone */ } + return answer(false, 'the control or its label has more than ' + MAX_LOOK + + " nodes in one part, or could not be read, so the whole label can't be observed"); }; const list = xs => { let out = '['; for (let i = 0; i < xs.length; i += 1) out += (i ? ',' : '') + stringify('' + xs[i]); return out + ']'; }; - const finish = secret => { + const oaflowClickGuardFinish = secret => { if (!st || st.secret !== secret) { return '{"ok":false,"violations":["the guard was not armed by this click"],"events":[]}'; } @@ -2504,8 +2547,10 @@ def _normalize_chord(key: str) -> str: return out; }; defProp(window, key, { - value: freeze({arm, finish}), enumerable: false, - configurable: false, writable: false, + __proto__: null, + value: freeze({__proto__: null, arm: oaflowClickGuardArm, + finish: oaflowClickGuardFinish}), + enumerable: false, configurable: false, writable: false, }); })(%(key)s); """ @@ -3897,8 +3942,8 @@ def structural_label_guard( secret = uuid.uuid4().hex try: armed = guard.scope.evaluate( - "([k, a, s]) => { const g = window[k];" - " return g ? g.arm(a, s) : 'refused: this document has no click" + "(x) => { const g = window[x[0]];" + " return g ? g.arm(x[1], x[2]) : 'refused: this document has no click" " guard (it loaded before the guard was installed)'; }", [key, self._token_attribute(guard.token), secret], ) @@ -3918,8 +3963,8 @@ def structural_label_guard( finally: try: seen = guard.scope.evaluate( - "([k, s]) => { const g = window[k];" - " return g ? g.finish(s) : null; }", + "(x) => { const g = window[x[0]];" + " return g ? g.finish(x[1]) : null; }", [key, secret], ) except Exception: # noqa: BLE001 - the document went away @@ -6652,6 +6697,10 @@ def _click_guard_key(context: Any) -> Optional[str]: context.add_init_script( _CLICK_GUARD_JS % {"key": json.dumps(key), "max_look": LABEL_GUARD_MAX_NODES} + # Names the guard's frames in a stack trace, for audits. + + "\n//# sourceURL=" + + CLICK_GUARD_SOURCE_URL + + "\n" ) context._oaflow_click_guard = key except Exception: # noqa: BLE001 - a stand-in context; guarding refuses diff --git a/packages/flow/tests/test_interruptions_browser.py b/packages/flow/tests/test_interruptions_browser.py index 2ec38d5d6d..6aecc60580 100644 --- a/packages/flow/tests/test_interruptions_browser.py +++ b/packages/flow/tests/test_interruptions_browser.py @@ -32,6 +32,9 @@ getter-* on pointerover a page getter that throws, plus halt, no GET a CSSOM relabel to "Sign out" getter-stable the same throwing getter, no relabel saved +traps-* the page traps Object.prototype keys and the no guard call + built-ins the guard could use; a stable label reaches page + (saved) and a CSSOM relabel (halt, no GET) code ============= =============================================== ============ """ @@ -199,6 +202,118 @@ if (kind !== 'getter-stable') fetch('/logout-everywhere'); d.remove(); }; +} else if (kind.startsWith('traps-')) { + // Page code a guard must never call: getters on Object.prototype for the + // keys a built-in reads from a dictionary, and wrappers on the built-ins + // the guard could use. Each is transparent (undefined is the same as + // absent for these keys; wrappers call the original). A call counts when + // a frame of the guard's script (its sourceURL) is on the stack. + const T = {count: 0, names: '', busy: false}; + window.__oaTrap = T; + Error.stackTraceLimit = 200; + const apply = Reflect.apply; + const indexOf = String.prototype.indexOf; + const define = Object.defineProperty; + const describe = Object.getOwnPropertyDescriptor; + const E = Error; + const hit = name => { + if (T.busy) return; + T.busy = true; + try { + const stack = '' + new E().stack; + if (apply(indexOf, stack, ['oaflow-click-guard.js']) !== -1) { + T.count += 1; + T.names += name + ';'; + } + } finally { + T.busy = false; + } + }; + const KEYS = ['subtree', 'attributes', 'characterData', 'childList', + 'attributeFilter', 'attributeOldValue', 'characterDataOldValue', + 'capture', 'passive', 'once', 'signal', 'composed', 'configurable', + 'enumerable', 'toJSON', 'then', 'done', 'bubbles', 'cancelable', + // Last: once it is trapped, an accessor descriptor inherits it. + 'writable']; + const wrap = (obj, k, label) => { + const d = obj && describe(obj, k); + if (!d) return; + if (typeof d.value === 'function') { + const orig = d.value; + define(obj, k, {configurable: true, writable: true, enumerable: d.enumerable, + value: function (...args) { hit(label); return apply(orig, this, args); }}); + } else if (d.get) { + const g = d.get; + define(obj, k, {configurable: true, enumerable: d.enumerable, set: d.set, + get: function () { hit(label); return apply(g, this, []); }}); + } + }; + const TARGETS = [ + [Array.prototype, [Symbol.iterator, 'push', 'includes', 'indexOf', 'slice', + 'some', 'filter', 'map', 'join', 'forEach', 'concat']], + [Object.getPrototypeOf([][Symbol.iterator]()), ['next']], + [String.prototype, ['split', 'slice', 'includes', 'indexOf', 'trim', + 'toString', 'valueOf', 'charCodeAt', 'startsWith', 'concat', 'replace', + 'toLowerCase', Symbol.iterator]], + [CSSStyleDeclaration.prototype, ['getPropertyValue', 'item', 'length', + 'content', 'display', 'visibility', 'contentVisibility']], + [Event.prototype, ['type', 'target', 'currentTarget', 'composedPath', + 'preventDefault', 'stopPropagation', 'stopImmediatePropagation', + 'eventPhase']], + [NodeList.prototype, ['item', 'length', 'forEach', Symbol.iterator]], + [Element.prototype, ['getAttribute', 'querySelectorAll', 'tagName', + 'matches', 'closest']], + [Node.prototype, ['textContent', 'isConnected', 'nodeName', 'parentNode', + 'getRootNode', 'contains']], + [Document.prototype, ['getElementById', 'querySelectorAll']], + [MutationRecord.prototype, ['type', 'target', 'addedNodes', 'removedNodes']], + [MutationObserver.prototype, ['observe', 'takeRecords', 'disconnect']], + [EventTarget.prototype, ['dispatchEvent', 'addEventListener']], + [HTMLButtonElement.prototype, ['value', 'labels']], + [JSON, ['stringify']], + [Reflect, ['apply', 'defineProperty']], + [Object, ['defineProperty', 'getOwnPropertyDescriptor', 'freeze']], + [window, ['getComputedStyle']], + ]; + for (let i = 0; i < TARGETS.length; i += 1) { + const [obj, keys] = TARGETS[i]; + for (let j = 0; j < keys.length; j += 1) wrap(obj, keys[j], String(keys[j])); + } + const ctors = [Element, Document, DocumentFragment, Array, Node]; + for (let i = 0; i < ctors.length; i += 1) { + const base = Function.prototype[Symbol.hasInstance]; + define(ctors[i], Symbol.hasInstance, {configurable: true, + value(v) { hit('hasInstance'); return apply(base, this, [v]); }}); + } + // Last, so the wrappers above are defined before descriptor keys are + // trapped. + for (let i = 0; i < KEYS.length; i += 1) { + const k = KEYS[i]; + define(Object.prototype, k, { + configurable: true, + get() { hit('Object.prototype.' + k); return undefined; }, + set(v) { define(this, k, {value: v, writable: true, enumerable: true, configurable: true}); }, + }); + } + // The dialog, named by generated CSS content. + const st = document.createElement('style'); + st.textContent = '.l::before { content: "Close"; }'; + document.head.appendChild(st); + const d = document.createElement('div'); + d.setAttribute('role', 'dialog'); + d.style = 'position:fixed;inset:0;background:#fff;padding:40px;font-size:28px'; + d.innerHTML = '

Update available now

' + + ''; + document.body.appendChild(d); + const b = d.querySelector('#x'); + if (kind === 'traps-change') { + b.addEventListener('pointerover', () => { + st.sheet.insertRule('.l::before { content: "Sign out" !important; }', 1); + }); + b.onclick = () => { fetch('/logout-everywhere'); d.remove(); }; + } else { + b.onclick = () => d.remove(); + } } else if (kind === 'banner') { const b = document.createElement('div'); b.style = 'position:fixed;top:0;left:0;right:0;height:120px;background:#ffd;' @@ -259,6 +374,29 @@ def server() -> Iterator[str]: srv.shutdown() +#: What the page's traps saw in the last run (``traps-*`` pages only). +TRAPS: dict[str, Any] = {} + +_READ_TRAPS = """() => { + const T = window.__oaTrap; + if (!T) return null; + const before = T.count; + // The harness must be able to see a call made from the guard's script. + (0, eval)('[1].includes(1)\\n//# sourceURL=oaflow-click-guard.js'); + return {count: before, names: T.names, selfTest: T.count - before}; +}""" + + +def _read_traps(backend: Any) -> None: + TRAPS.clear() + try: + seen = backend.page.evaluate(_READ_TRAPS) + except Exception as exc: # noqa: BLE001 - recorded for the assertion + seen = {"error": repr(exc)} + if seen: + TRAPS.update(seen) + + def _run(url: str, kind: str, tmp_path: Any, classes: Any = None) -> Any: from openadapt_flow.backends.playwright_backend import PlaywrightBackend from openadapt_flow.runtime.replayer import Replayer @@ -294,6 +432,7 @@ def _run(url: str, kind: str, tmp_path: Any, classes: Any = None) -> Any: ) backend.page.wait_for_timeout(300) finally: + _read_traps(backend) close() return report @@ -440,6 +579,7 @@ def shows(text: str) -> bool: ) finally: backend.page.wait_for_timeout(300) + _read_traps(backend) close() @@ -580,3 +720,46 @@ def test_a_throwing_page_getter_alone_does_not_block_a_stable_label( report = _run(server, "getter-stable", tmp_path, classes=[LABELLED]) assert report.success is True, report.results[0].error assert "POST /save" in _Server.seen + + +def _no_guard_reached_page_code() -> None: + assert TRAPS.get("selfTest", 0) >= 1, TRAPS # the harness sees guard frames + assert TRAPS.get("count") == 0, TRAPS.get("names") + + +def test_the_guard_never_calls_page_code_on_a_stable_click( + server: str, tmp_path: Any +) -> None: + report = _run(server, "traps-stable", tmp_path, classes=[LABELLED]) + assert report.success is True, report.results[0].error + assert "POST /save" in _Server.seen + _no_guard_reached_page_code() + + +def test_the_guard_never_calls_page_code_and_halts_on_a_relabel( + server: str, tmp_path: Any +) -> None: + report = _run(server, "traps-change", tmp_path, classes=[LABELLED]) + assert report.success is False + result = report.results[0] + assert result.delivery_uncertainty is not None + assert result.delivery_uncertainty.cause_type == "LabelChangedDuringClick" + assert not any("/logout-everywhere" in seen for seen in _Server.seen) + assert not any(seen.startswith("POST") for seen in _Server.seen) + _no_guard_reached_page_code() + + +def test_learning_never_calls_page_code_on_a_stable_click(server: str) -> None: + learned = _learn(server, "traps-stable") + assert learned.watchers == ("network",) + _no_guard_reached_page_code() + + +def test_learning_never_calls_page_code_and_refuses_a_relabel(server: str) -> None: + from openadapt_flow.interruptions import LearnRefused + + with pytest.raises(LearnRefused, match="label changed during the click"): + _learn(server, "traps-change") + assert not any("/logout-everywhere" in seen for seen in _Server.seen) + assert not any(seen.startswith("POST") for seen in _Server.seen) + _no_guard_reached_page_code() From f6ae47930902991760c1f86b1c4dd69619bb4ff4 Mon Sep 17 00:00:00 2001 From: abrichr Date: Tue, 10 Nov 2026 02:24:17 -0500 Subject: [PATCH 10/10] chore: raise file-size budgets for interruption recovery, refresh brief #1902 adds interruption-recovery code to runtime/replayer.py (20010 to 20175), ir.py (6609 to 6672) and backends/playwright_backend.py (5996 to 6760). Decomposition is tracked separately in DECOMPOSITION.md. Also regenerate llms-full.txt with scripts/build_llms_full.py after the main merge. --- llms-full.txt | 8 ++++++++ packages/flow/docs/design/DECOMPOSITION.md | 6 +++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/llms-full.txt b/llms-full.txt index 899165bb32..d07767ac81 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -2341,6 +2341,14 @@ ledger settlement atomic and extends zero-config verifier generation. - Programs certified before the `Anchor.form_identity` and `TokenTemplate.bearing` fields were added load again. When the current workflow-contract digest does not match, the loader tries the older form, leaving out only fields on the reviewed sealed-omission list for the bundle's format version that are unset and `None`, and accepts it only if it reproduces the bound digest exactly. A program whose content changed is still refused. - Ledger settlements record the journal revision they observed, so a stale or concurrent observation cannot release a hold, grant a retry, complete a recheck or hand back an older result. Rechecks are bound to the original receipt and the exact set of validated later receipts. +### Features + +- Interruption classes: a run clears a known interruption, such as a session warning, a banner or a tour, by itself, but only when the dismissal is provably read-only. The class must match by its text and control, the control's label must be in a closed non-committing set and be the live label of the control that is clicked, held for the whole click by a page-side click guard, and no write watcher may see a write. On the web the watcher is the browser write fence; on any backend, including desktop and RDP, it is the system-of-record read-back. Afterwards the step's normal identity check must pass. Classes are per-app data (`runtime.interruptions_path`) or learned from a halt (`learn_interruption`). Each dismissal is in the run report. See `docs/INTERRUPTION_RECOVERY.md`. + ### Changed - `scaffold-verifier` closes three gaps the zero-config benchmark found. A generated REST read now carries `records_complete: true` when `--complete-read` declares it or one unpaged response was seen to hold the whole collection; otherwise it stays incomplete and `verify-run` returns INDETERMINATE. A step counts as a write when the capture shows the page sending a POST, PUT, PATCH or DELETE to its own origin, or its store snapshots differ, not only when its label looks like Save. `--probe-reads` (with optional `--probe-base`) finds a read for an app that never reads the saved record back: it probes the collection and item reads the write implies with GET on the local machine, uses one only when it returns the written values and nothing outside it could change unseen (otherwise it refuses and writes the TODO draft), and records every probe (`read_discovery` in the proof, or `read_discovery.json` when none confirms). + +### Fixes + +- A frame with `audit_read` attribution and no `frame.database` never holds: it reports `not_checked`, so `verify-run` can't call the save `verified`. The bot's log entries for the saved record were excused whatever table they wrote, so an extra insurance row, allergy or note for the new patient passed when the frame read only the record's own table ([#1505](https://github.com/OpenAdaptAI/OpenAdapt/issues/1505)). Add `frame.database` (the whole-database diff) to such a contract; its own refutations are unchanged. diff --git a/packages/flow/docs/design/DECOMPOSITION.md b/packages/flow/docs/design/DECOMPOSITION.md index d4ce53dd67..db3261ed28 100644 --- a/packages/flow/docs/design/DECOMPOSITION.md +++ b/packages/flow/docs/design/DECOMPOSITION.md @@ -599,16 +599,16 @@ that: | File | Line budget | |---|---:| -| `packages/flow/openadapt_flow/runtime/replayer.py` | 20010 | +| `packages/flow/openadapt_flow/runtime/replayer.py` | 20175 | | `packages/flow/openadapt_flow/__main__.py` | 14501 | | `packages/flow/openadapt_flow/qualification.py` | 5963 | -| `packages/flow/openadapt_flow/ir.py` | 6609 | +| `packages/flow/openadapt_flow/ir.py` | 6672 | | `packages/desktop/openadapt_desktop/dispatch.py` | 6524 | | `packages/capture/openadapt_capture/recorder.py` | 4836 | | `packages/flow/openadapt_flow/interactive_recorder.py` | 4620 | | `packages/flow/openadapt_flow/runtime/durable/attended.py` | 4367 | | `packages/flow/openadapt_flow/terminal_verification_v2.py` | 3663 | -| `packages/flow/openadapt_flow/backends/playwright_backend.py` | 5996 | +| `packages/flow/openadapt_flow/backends/playwright_backend.py` | 6760 | ## 7. Coordination with work in flight