flow: count a self-closing popup click as delivered only when the step expects it

packages/flow/docs/MULTI_SURFACE.md

@@ -278,6 +278,17 @@ clear reason: a popup whose *first* action closes it before Flow captured any
 frame of it. Nothing would anchor that action. Act in the popup once more
 first, or record that step with the scripted recorder.
 
+The browser can report the popup closed while the closing click is still
+being delivered ("Target page, context or browser has been closed"). The
+recorder marks a click that left its popup closed as `closes_surface`, and
+that step is the only one for which the report counts as a delivered click.
+At replay the browser backend accepts it when three things hold: the step is
+marked `closes_surface`, the click ran on a page surface that was open when
+the click started, and the page reads as closed within a second. Any other
+"page closed" report, including one on the primary page or on a step not
+marked, leaves delivery uncertain, and the run goes on only as far as its
+postconditions and effect checks confirm.
+
 A click in a new page within the first poll tick after it loads can arrive
 before the listener is installed and is not recorded. Replay then halts at the
 next step rather than skipping it silently.

packages/flow/openadapt_flow/backend.py

@@ -604,6 +604,33 @@ def reset_fresh_actuation_state(self) -> None:
         ...
 
 
+@runtime_checkable
+class PageCloseExpectationBackend(Protocol):
+    """Optional seam for a click that closes its own popup.
+
+    A popup or tab that a step opened can close itself from the click that
+    finishes it (an approval or sign-in window). The browser may then report
+    the page closed while the click is still being delivered. Without an
+    expectation, that error leaves delivery uncertain. The runtime arms an
+    expectation only for a step whose recording shows it closed its page
+    (``Step.closes_surface``); the backend then counts the click as delivered
+    only when the page was open before the click, the browser reports the
+    page closed, and the page is confirmed closed afterwards.
+    """
+
+    def expect_page_close(self) -> None:
+        """Expect the next click on this page to close it. One-shot."""
+        ...
+
+    def cancel_page_close(self) -> None:
+        """Drop an unconsumed expectation."""
+        ...
+
+    def page_closed(self) -> bool:
+        """Whether this page surface has closed."""
+        ...
+
+
 @runtime_checkable
 class PreparedPointerActuationBackend(Protocol):
     """Optional pointer pre-positioning for opaque remote-display clients.

packages/flow/openadapt_flow/backends/playwright_backend.py

@@ -61,6 +61,16 @@
     "Execution context was destroyed",
 )
 
+# How long a click that reported its page closed waits for the page to read
+# as closed before the close counts as confirmed.
+_PAGE_CLOSE_CONFIRM_S = 1.0
+
+
+def _is_target_closed_error(exc: BaseException) -> bool:
+    """Whether Playwright reported the page, context or browser closed."""
+    return type(exc).__name__ == "TargetClosedError"
+
+
 _MODIFIER_ALIASES = {
     "meta": "Meta",
     "cmd": "Meta",
@@ -1081,6 +1091,9 @@ def __init__(
         self._fill_store_key = f"__oaflow_filled_{uuid.uuid4().hex}"
         self._structural_tokens: dict[str, _StructuralGuard] = {}
         self._guarded_coordinate: Optional[_CoordinateGuard] = None
+        # One-shot expectation that the next click closes this page surface
+        # (Step.closes_surface; see expect_page_close).
+        self._page_close_expected = False
         self._guarded_keyboard: Optional[_KeyboardGuard] = None
         self._qualification_environment: Optional[tuple[str, str, str, str]] = None
         self._qualification_input_guard: Optional[Callable[[], None]] = None
@@ -2142,6 +2155,8 @@ def act_structural(
             # cannot declare (for example a browser inside a managed remote
             # session), so the DOM guard alone is not sufficient.
             self._assert_qualification_environment_current()
+            close_expected = self._take_page_close_expectation()
+            open_before = not self._page_is_closed()
             try:
                 if double:
                     token_locator.dblclick(timeout=1000)
@@ -2152,13 +2167,18 @@ def act_structural(
                 # this call.  Once Playwright begins its action, an exception
                 # (notably frame detach/navigation) cannot prove that the
                 # browser emitted no input.  Surface a typed uncertainty so the
-                # runtime verifies the effect exactly once and never retries.
-                raise ActionDeliveryUncertain(
-                    operation="dom_double_click" if double else "dom_click",
-                    native=False,
-                    target_fingerprint=fingerprint,
-                    cause_type=type(exc).__name__,
-                ) from exc
+                # runtime verifies the effect exactly once and never retries,
+                # unless this page was expected to close itself on this click.
+                if not (
+                    _is_target_closed_error(exc)
+                    and self._closed_itself_as_expected(close_expected, open_before)
+                ):
+                    raise ActionDeliveryUncertain(
+                        operation="dom_double_click" if double else "dom_click",
+                        native=False,
+                        target_fingerprint=fingerprint,
+                        cause_type=type(exc).__name__,
+                    ) from exc
         except ActionDeliveryUncertain:
             raise
         except StructuralResolutionRefused:
@@ -2548,6 +2568,8 @@ def act_guarded_coordinate(
                     "pre-dispatch actionability trial"
                 )
             self._assert_qualification_environment_current()
+            close_expected = self._take_page_close_expectation()
+            open_before = not self._page_is_closed()
             try:
                 if double:
                     token_locator.dblclick(position=position, timeout=1000)
@@ -2558,12 +2580,16 @@ def act_guarded_coordinate(
                         button=button,
                     )
             except Exception as exc:
-                raise ActionDeliveryUncertain(
-                    operation=operation,
-                    native=False,
-                    target_fingerprint=pending.fingerprint,
-                    cause_type=type(exc).__name__,
-                ) from exc
+                if not (
+                    _is_target_closed_error(exc)
+                    and self._closed_itself_as_expected(close_expected, open_before)
+                ):
+                    raise ActionDeliveryUncertain(
+                        operation=operation,
+                        native=False,
+                        target_fingerprint=pending.fingerprint,
+                        cause_type=type(exc).__name__,
+                    ) from exc
         except ActionDeliveryUncertain:
             raise
         except StructuralResolutionRefused:
@@ -3130,12 +3156,77 @@ def _live_screenshot(self) -> bytes:
         )
 
     def click(self, x: int, y: int, *, double: bool = False) -> None:
-        """Click (or double-click) at pixel coordinates via the mouse."""
+        """Click (or double-click) at pixel coordinates via the mouse.
+
+        A click that closes its own page surface completes normally when that
+        close was expected (:meth:`expect_page_close`). An unexpected report
+        that the page closed during the click raises
+        :class:`~openadapt_flow.backend.ActionDeliveryUncertain`.
+        """
         self._assert_qualification_environment_current()
-        if double:
-            self.page.mouse.dblclick(x, y)
-        else:
-            self.page.mouse.click(x, y)
+        expected = self._take_page_close_expectation()
+        open_before = not self._page_is_closed()
+        try:
+            if double:
+                self.page.mouse.dblclick(x, y)
+            else:
+                self.page.mouse.click(x, y)
+        except Exception as exc:
+            if not _is_target_closed_error(exc):
+                raise
+            if self._closed_itself_as_expected(expected, open_before):
+                return
+            raise ActionDeliveryUncertain(
+                operation="coordinate_double_click" if double else "coordinate_click",
+                native=False,
+                cause_type=type(exc).__name__,
+            ) from exc
+
+    # -- a click that closes its own popup ---------------------------------
+
+    def expect_page_close(self) -> None:
+        """Expect the next click on this page to close it (one-shot).
+
+        Only a page surface (``closed_page_frame=True``: a popup or tab a step
+        opened) honours it. On the primary page a close during a click stays
+        an uncertain delivery.
+        """
+        self._page_close_expected = True
+
+    def cancel_page_close(self) -> None:
+        """Drop an expectation no click consumed."""
+        self._page_close_expected = False
+
+    def page_closed(self) -> bool:
+        """Whether this page has closed."""
+        return self._page_is_closed()
+
+    def _take_page_close_expectation(self) -> bool:
+        expected, self._page_close_expected = self._page_close_expected, False
+        return expected and self._closed_page_frame
+
+    def _closed_itself_as_expected(self, expected: bool, open_before: bool) -> bool:
+        """Whether a click that raised "page closed" was delivered.
+
+        It was when the step expected its page to close, the page was open
+        when the click started, and the page is confirmed closed now. The
+        browser reports the close only after the page began handling input,
+        and an open page cannot close itself without running its own script.
+        """
+        if not (expected and open_before):
+            return False
+        deadline = time.monotonic() + _PAGE_CLOSE_CONFIRM_S
+        while True:
+            if self._page_is_closed():
+                return True
+            if time.monotonic() >= deadline:
+                return False
+            try:
+                # The sync API applies the page's close event only while it
+                # is inside a call; a short wait lets it arrive.
+                self.page.wait_for_timeout(20)
+            except Exception:
+                time.sleep(0.02)
 
     def right_click(self, x: int, y: int) -> None:
         """Open the context menu at a resolved point."""

packages/flow/openadapt_flow/bundle_validation.py

@@ -230,6 +230,16 @@ class _SealedCanonicalOmission:
                 "fields; a recorded origin remains sealed content"
             ),
         ),
+        _SealedCanonicalOmission(
+            owner=Step,
+            field_name="closes_surface",
+            omitted_values=(False,),
+            reason=(
+                "a click that leaves its page open is what every earlier "
+                "bundle recorded; a click declared to close its popup is "
+                "sealed content"
+            ),
+        ),
         _SealedCanonicalOmission(
             owner=Step,
             field_name="dialog",

packages/flow/openadapt_flow/compiler/compile.py

@@ -2525,6 +2525,8 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]:
                 stamped.surface = str(event["surface"])
             if event.get("opens"):
                 stamped.opens = [str(sid) for sid in event["opens"]]
+            if event.get("closes_surface") is True:
+                stamped.closes_surface = True
 
     # Collapse a demonstrated native/pixel option selection into one explicit
     # SELECT_OPTION contract. This is deliberately evidence-gated and remote-only:

packages/flow/openadapt_flow/ir.py

@@ -1232,6 +1232,13 @@ def surface_contract_errors(workflow: "Workflow") -> list[str]:
     for step in all_steps:
         if step.surface is not None and step.surface not in by_id:
             errors.append(f"step {step.id!r} names undeclared surface")
+        if step.closes_surface and not (
+            step.surface in by_id and by_id[step.surface].is_page
+        ):
+            errors.append(
+                f"step {step.id!r} closes its surface, but it does not run on "
+                "a page a step opened"
+            )
         declared = [sid for _, sid in sorted(expected_opens.get(step.id, []))]
         if list(step.opens) != declared:
             errors.append(
@@ -1462,6 +1469,17 @@ class Step(BaseModel):
             " the target page showed when the demonstration switched to it."
         ),
     )
+    closes_surface: bool = Field(
+        default=False,
+        description=(
+            "A click on a page surface (a popup or tab a step opened) that "
+            "closed that page in the demonstration, such as an approval "
+            "window's final button. At replay, the browser backend may then "
+            "count the click as delivered when the browser reports the page "
+            "closed during the click and the page is confirmed closed. "
+            "Without it, that error leaves delivery uncertain."
+        ),
+    )
     dialog: Optional[DialogResponse] = Field(
         default=None,
         description=(
@@ -1477,6 +1495,11 @@ def _validate_surface_contract(self) -> "Step":
         for sid in self.opens:
             if not re.fullmatch(SURFACE_ID_PATTERN, sid):
                 raise ValueError("opens holds an invalid surface id")
+        if self.closes_surface:
+            if self.action not in (ActionKind.CLICK, ActionKind.DOUBLE_CLICK):
+                raise ValueError("closes_surface is valid only on a click")
+            if self.surface is None:
+                raise ValueError("closes_surface needs the page surface it closes")
         if self.action is not ActionKind.SWITCH_SURFACE:
             if self.surface_origin is not None:
                 raise ValueError("surface_origin is valid only on SWITCH_SURFACE")

packages/flow/openadapt_flow/recorder.py

@@ -59,7 +59,11 @@
 
 from PIL import Image, ImageDraw
 
-from openadapt_flow.backend import Backend, RichPointerActionBackend
+from openadapt_flow.backend import (
+    Backend,
+    PageCloseExpectationBackend,
+    RichPointerActionBackend,
+)
 from openadapt_flow.image_hash import perceptual_hash
 from openadapt_flow.ir import (
     MAX_PAGES_OPENED_PER_STEP,
@@ -526,6 +530,23 @@ def _opened_by_another_known_page(page: Any) -> bool:
         self._multi_surface = True
         return ids
 
+    def _closed_its_page(self, event: dict[str, Any]) -> bool:
+        """Whether a click left the popup or tab it ran on closed."""
+        return (
+            event.get("kind") in ("click", "double_click")
+            and self._active_surface in self._opened_page_ids()
+            and isinstance(self._backend, PageCloseExpectationBackend)
+            and self._backend.page_closed()
+        )
+
+    def _opened_page_ids(self) -> set[str]:
+        """Ids of the page surfaces (popups, tabs) a recorded action opened."""
+        return {
+            str(meta["id"])
+            for meta in self._surface_meta
+            if meta.get("opened_by_event") is not None
+        }
+
     @property
     def event_count(self) -> int:
         """Number of complete events already persisted."""
@@ -584,7 +605,23 @@ def _record(self, event: dict[str, Any], act: Callable[[], None]) -> None:
                     "backend record the clicks that answer the dialog instead"
                 )
             arm(dialog_response, None)
-        act()
+        # A popup or tab may close itself from the click that finishes it (an
+        # approval window). On a page surface, expect that close so the click
+        # counts as delivered, and record it so replay expects the same.
+        page_backend = (
+            self._backend
+            if event.get("kind") in ("click", "double_click")
+            and self._active_surface in self._opened_page_ids()
+            and isinstance(self._backend, PageCloseExpectationBackend)
+            else None
+        )
+        if page_backend is not None:
+            page_backend.expect_page_close()
+        try:
+            act()
+        finally:
+            if page_backend is not None:
+                page_backend.cancel_page_close()
         if dialog_response is not None:
             seen = self._backend.take_dialogs(_DIALOG_WAIT_S)  # type: ignore[attr-defined]
             if len(seen) != 1:
@@ -603,6 +640,10 @@ def _record(self, event: dict[str, Any], act: Callable[[], None]) -> None:
         opened = self._claim_new_pages(self._i)
         if opened:
             event = {**event, "opens": opened}
+        # Checked after the settle: a page closes asynchronously after the
+        # click that closes it.
+        if self._closed_its_page(event):
+            event = {**event, "closes_surface": True}
         self._commit(event, before, after, structural_before)
 
     def record_observed(
@@ -679,6 +720,8 @@ def record_observed(
         opened = self._claim_new_pages(self._i)
         if opened:
             event = {**event, "opens": opened}
+        if self._closed_its_page(event):
+            event = {**event, "closes_surface": True}
         self._commit(
             event,
             before_png,