From b87a7b9c373b77cf0ecd17294c3a63d55c1e9a80 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sat, 24 Oct 2026 23:39:06 -0400 Subject: [PATCH 01/15] flow: run a form's own script before an API save instead of refusing it A form whose fields have handlers, whose onsubmit validates, or whose validate.js rules use a custom kind (OpenEMR's pastDate) kept its save on the GUI. The compiler now records that script in the proposal (client_scripts), and before each save the runtime runs it: the form page opens in a scratch copy of the live session with every non-GET request and every script-sent request after load blocked, the run's typed and picked values go in with input, change and blur events, and the form is submitted through its own handlers with a final listener that records the verdict and stops the submit. The save is sent only when the page let the submit through, raised no error, showed no dialog, tried to send nothing, and would send exactly the body the API step built. Otherwise the GUI steps run. Co-Authored-By: Claude Opus 5.5 --- CAPABILITIES.md | 1 + packages/flow/docs/API_TIER.md | 53 ++- .../openadapt_flow/compiler/api_proposal.py | 52 +-- packages/flow/openadapt_flow/ir.py | 10 + .../runtime/actuators/form_eval.py | 305 +++++++++++++++ .../runtime/actuators/web_form.py | 105 ++++- .../tests/test_api_step_native_validation.py | 32 +- .../flow/tests/test_api_step_page_script.py | 367 ++++++++++++++++++ 8 files changed, 857 insertions(+), 68 deletions(-) create mode 100644 packages/flow/openadapt_flow/runtime/actuators/form_eval.py create mode 100644 packages/flow/tests/test_api_step_page_script.py diff --git a/CAPABILITIES.md b/CAPABILITIES.md index e541d413f9..f7c2b78ff2 100644 --- a/CAPABILITIES.md +++ b/CAPABILITIES.md @@ -199,6 +199,7 @@ improvising or retrying when something is off. | Run a step as a direct API call (`ApiActuator`, `ApiBinding`) | Released, hand-authored bindings | Synthetic benchmarks | `runtime/actuators/api.py` | | Capture a recording's network traffic and compile a clean save into an API step, with the same record check | On main ([#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269)) | Synthetic patients on a local OpenEMR test copy | network capture, `ApiBinding` | | Fall back from the API step to page structure when the API is unavailable, before anything is sent | On main ([#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269)) | Synthetic | [#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269) | +| Run a form's own script before an API save instead of refusing the form: field handlers, `onsubmit`, submit listeners and custom validate.js rules (OpenEMR's `pastDate`) run on the run's values in a scratch copy of the session with every write blocked, and the save is sent only when the page lets the submit through and would send the same body | On main | Synthetic (headless Chromium against a local test page) | [API_TIER.md](packages/flow/docs/API_TIER.md#what-happens-at-run-time), `runtime/actuators/form_eval.py` | | Approve an API step's exact request under per-item approval: one digest binds the request bytes, the record and the record check, and a changed request is not sent | On main ([#1355](https://github.com/OpenAdaptAI/OpenAdapt/pull/1355)) | Synthetic | [API_TIER.md](packages/flow/docs/API_TIER.md#under-an-approval-mode) | | Run an API step in a durable run: checkpoint the span after the save is sent, never resend or redo a save that may have landed, and reconcile it by reading the record back | On main ([#1358](https://github.com/OpenAdaptAI/OpenAdapt/pull/1358)) | Synthetic | [API_TIER.md](packages/flow/docs/API_TIER.md#in-a-durable-run) | | Run an admitted API step under a governed run authorization and the Standard or Regulated profile: the authorization names the API step by digest, the person who admits it names the right-record parameter, and the outcome reads `VERIFIED` | On main ([#1361](https://github.com/OpenAdaptAI/OpenAdapt/pull/1361)) | Synthetic | [API_TIER.md](packages/flow/docs/API_TIER.md#under-governance) | diff --git a/packages/flow/docs/API_TIER.md b/packages/flow/docs/API_TIER.md index 4399ac911e..2eca30b092 100644 --- a/packages/flow/docs/API_TIER.md +++ b/packages/flow/docs/API_TIER.md @@ -4,8 +4,10 @@ `application/x-www-form-urlencoded` POST of one HTML form, read again from the live page, in place of the clicks and keystrokes that filled it. It is not a general API client: JSON or script-built requests, more than one -write, multi-page server state, and forms whose script validates or rewrites -values all stay on the GUI. +write and multi-page server state stay on the GUI. A form whose script +validates or rewrites values is run, not refused: the API step runs that +script on the run's values before each save (below), and the save stays on +the GUI when the script can't be run. The right-record check before the save and the record check after it stay the same. If anything the API step depends on has changed, the recorded GUI @@ -125,16 +127,31 @@ Before it sends the save, the API step: 1. reads the form page again in this session; 2. refuses if the form's fields or its client-side validation rules (for example, OpenEMR's `constraints` object) differ from the recording; -3. refuses if the form runs script it can't run: an `on*` handler on the - form (`onsubmit`) or on a field it sends (`onchange`, `oninput`, - `onblur`, script that can validate or rewrite a value), a page script - listening for the submit, or a validate.js rule kind other than - `presence`, `length`, `format`, `numericality` and `email`; +3. refuses if the form's script isn't the script recorded with the + proposal (`client_scripts`: an `on*` handler on the form, such as + `onsubmit`, or on a field it sends, such as `onchange`; a page script + listening for the submit; a validate.js rule kind other than `presence`, + `length`, `format`, `numericality` and `email`, such as OpenEMR's + `pastDate`); 4. builds the body and checks it the way the browser would before it submits: native HTML constraints (`required`, `minlength`, `maxlength`, `pattern`, `min`, `max`, and the `email`, `url`, `number` and `date` input types) and the page's validate.js rules; -5. runs the pre-checks. +5. when the form has script, **runs it** (`runtime/actuators/form_eval.py`): + it opens the form page in a scratch copy of the live session (a separate + browser context with the session's cookies), blocks every request that + isn't a GET and every script-sent request once the page has loaded, puts + the run's typed and picked values into the form with the same `input`, + `change` and `blur` events a browser fires, reads the body the browser + would send, runs the browser's own constraint checks, submits the form + through its own handlers with a final listener that records whether + they let the submit through and then stops it, and runs the page's + validate.js `constraints` when it declares them. The save goes ahead only + when the page let the submit through, raised no error, showed no dialog, + tried to send nothing, and would send exactly the body the API step + built (session values aside). Anything else is a reason, and the GUI + steps run; +6. runs the pre-checks. Each of these refusals means the save wasn't sent, so the GUI steps run. After the save request starts, the API tier's no-double-write rule @@ -357,15 +374,17 @@ recording. - The server may enforce less than the page. On OpenEMR, the duplicate check and the required fields run only in the browser. The API step carries over the checks it can evaluate (pre-checks, native HTML - constraints and validate.js rules of the kinds above), and leaves a form - whose script it can't run to the GUI. A `pattern` is evaluated with - Python's regular expressions, which differ from the browser's in rare - syntax; one that doesn't compile keeps the save on the GUI. Letting - qualification show that the server enforces a script-only rule, so the - API step can run anyway, isn't built yet. -- OpenEMR's new-patient form runs script on submit (`submitme(...)`) and on - each name field (`capitalizeMe`, which rewrites the typed value), and its - `constraints` use a custom `pastDate` rule. So its save stays on the GUI. + constraints and validate.js rules of the kinds above) and runs the + form's own script for the rest. A `pattern` is evaluated with Python's + regular expressions, which differ from the browser's in rare syntax; one + that doesn't compile keeps the save on the GUI. +- Running the form's script covers what the script does on `input`, + `change`, `blur` and `submit`. Script that validates on a click handler + of the submit button only, or asynchronously after the submit event, + isn't seen; qualification on test cases, both paths, is what shows the + two paths agree. A form whose script needs the server while it validates + (a script-sent request after load) is blocked and stays on the GUI. Each + save with script costs one extra load of the form page. - Multi-step server state (wizards, uploads, locks held between pages) needs the whole request sequence, and isn't proposed. diff --git a/packages/flow/openadapt_flow/compiler/api_proposal.py b/packages/flow/openadapt_flow/compiler/api_proposal.py index 58297d4512..d85e5694e7 100644 --- a/packages/flow/openadapt_flow/compiler/api_proposal.py +++ b/packages/flow/openadapt_flow/compiler/api_proposal.py @@ -37,6 +37,12 @@ - a folded step made a read whose response must show text the GUI checks, and that text is in no response. +Page script on the form (field handlers, an ``onsubmit`` validator, a submit +listener, validate.js rules such as ``pastDate``) is not a reason to refuse: +it is listed in ``client_scripts`` and the runtime runs it on the run's +values before each save, with writes blocked; when it can't, the GUI steps +run. + Model-free and deterministic. """ @@ -67,8 +73,7 @@ client_rules, folded_checks_digest, form_controls, - form_scripts, - unsupported_client_rules, + page_scripts, ) @@ -351,30 +356,16 @@ def carried(line: dict[str, Any]) -> int: form_path, url_problems = _url_template(form_line["url"], by_value) if url_problems: return workflow, None, url_problems - # A browser runs the page's validation and script before it submits; the - # API step can only stand in for what it can evaluate itself. - unsupported = unsupported_client_rules(client_rules(form_line["response_text"])) - if unsupported: - return ( - workflow, - None, - [ - "the form validates with rules the API step can't evaluate " - f"({', '.join(unsupported)}); it stays on the GUI" - ], - ) - scripts = form_scripts( - form_line["response_text"], _path(post["url"]), [n for n, _ in post["form"]] + # A browser runs the page's validation and script before it submits. What + # reading the form can't evaluate (handlers, submit listeners, custom + # validate.js rules) the API step runs itself before each save, in a + # scratch page with every write blocked (runtime/actuators/form_eval.py). + client_scripts = page_scripts( + form_line["response_text"], + _path(post["url"]), + [n for n, _ in post["form"]], + client_rules(form_line["response_text"]), ) - if scripts: - return ( - workflow, - None, - [ - "the form runs script the API step can't run " - f"({', '.join(scripts[:5])}); it stays on the GUI" - ], - ) # Steps the API step stands in for: those recorded on the form page. folded: list[Step] = [] @@ -609,6 +600,7 @@ def carried(line: dict[str, Any]) -> int: fields=fields, controls=sorted(controls), client_rules=client_rules(form_line["response_text"]), + client_scripts=client_scripts, prechecks=prechecks, success_pattern=_success_pattern(post), replaces_steps=[s.id for s in folded], @@ -681,6 +673,16 @@ def describe_api_step(step: Step) -> str: f"in {', '.join(p.removeprefix('/body/') for p in identity.request_pointers)} " f"and the record check selects {identity.effect_field} = {{{identity.param}}}" ) + if spec.client_scripts: + shown = spec.client_scripts[:6] + ( + [f"and {len(spec.client_scripts) - 6} more"] + if len(spec.client_scripts) > 6 + else [] + ) + lines.append( + " the form's own script runs before each save, in a scratch page with " + f"writes blocked, on the run's values: {', '.join(shown)}" + ) if spec.screen_only_checks: lines.append(f" screen-only checks, not repeated: {spec.screen_only_checks}") if spec.other_writes_seen: diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index 6c88d79824..139b33a069 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -627,6 +627,16 @@ class WebFormSpec(BaseModel): fields: list[WebFormField] = Field(min_length=1) controls: list[str] = Field(default_factory=list) client_rules: Optional[dict[str, Any]] = None + #: Page script the browser runs on this save that the API step can't + #: evaluate by reading the form: the form's ``on*`` handlers, the ``on*`` + #: handlers of the fields it sends, a page script listening for submit, + #: and validate.js rule kinds such as OpenEMR's ``pastDate``. Before each + #: save the runtime runs that script itself, in a scratch page of the live + #: session with every write blocked, on the run's values, and sends only + #: when the page would submit exactly the body the API step built. When + #: it can't run the script, the GUI steps run. The live form must list the + #: same script, or the API step doesn't run. + client_scripts: list[str] = Field(default_factory=list) prechecks: list[WebFormPrecheck] = Field(default_factory=list) #: Regular expression the save response must match to count as accepted. success_pattern: Optional[str] = None diff --git a/packages/flow/openadapt_flow/runtime/actuators/form_eval.py b/packages/flow/openadapt_flow/runtime/actuators/form_eval.py new file mode 100644 index 0000000000..a73c2f67fd --- /dev/null +++ b/packages/flow/openadapt_flow/runtime/actuators/form_eval.py @@ -0,0 +1,305 @@ +"""Run a form's own script on the run's values, with every write blocked. + +The ``web_form`` API step sends one request in place of the clicks that fill +and submit a form. Most forms run script on the way: field ``onchange`` +handlers, an ``onsubmit`` validator, a page listener for ``submit``, custom +validate.js rules such as OpenEMR's ``pastDate``. The API step can't read +what that script does, so it runs it, before each save: + +1. open the form page in a scratch copy of the live session (a separate + browser context with the session's cookies, or a scratch tab when the + browser can't make one); +2. block every request that isn't a GET for the whole visit, and every + script-sent request once the page has loaded, so nothing the page does + here can write; +3. put the run's values into the fields the person typed or picked, the way + a browser does (``input`` and ``change`` events, then ``blur``); +4. read back the body the browser would send (``FormData``), check the + browser's own constraints, and submit the form with a listener that + records whether the page's own handlers let the submit through and then + stops it; where the page declares validate.js ``constraints``, run them + too. + +The result is ``valid`` only when nothing failed, nothing tried to write, +the page raised no error, asked no question, and its handlers let the submit +through. Anything else is a reason for the GUI steps to run instead. +Comparing the body the page would send with the one the API step built is +the caller's job (:meth:`WebFormActuator.prepare`). Model-free. +""" + +from __future__ import annotations + +import time +import urllib.parse as up +from dataclasses import dataclass, field +from typing import Any, Optional + +#: Before any page script: a form's own ``submit()`` (which skips the submit +#: event) is counted instead of navigating, and window.open opens nothing. +_INSTRUMENT_JS = """ +(() => { + const state = { programmaticSubmits: 0, opened: [] }; + Object.defineProperty(window, '__openadaptFormEval', { value: state }); + HTMLFormElement.prototype.submit = function () { state.programmaticSubmits += 1; }; + window.open = function (url) { state.opened.push(String(url || '')); return null; }; +})(); +""" + +_EVALUATE_JS = """ +async ({ actionPath, leaf, fields, settleMs }) => { + const state = window.__openadaptFormEval || { programmaticSubmits: 0, opened: [] }; + const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); + const form = Array.from(document.forms).find((f) => { + const p = new URL(f.getAttribute('action') || location.href, location.href).pathname; + return p === actionPath || (leaf && p.split('/').pop() === leaf); + }); + if (!form) return { unevaluable: 'the scratch page has no form that posts to the save' }; + for (const [name, value] of fields) { + const el = form.elements.namedItem(name); + if (!el || typeof el.tagName !== 'string') { + return { unevaluable: `${name} is not one control in the form` }; + } + const kind = (el.type || '').toLowerCase(); + if (kind === 'checkbox' || kind === 'radio' || kind === 'file') { + return { unevaluable: `${name} is a ${kind} the API step doesn't fill` }; + } + if (el.tagName === 'SELECT' && !Array.from(el.options).some((o) => o.value === value)) { + return { unevaluable: `${name} has no option with value ${JSON.stringify(value)}` }; + } + if (typeof el.focus === 'function') el.focus(); + el.value = value; + el.dispatchEvent(new Event('input', { bubbles: true })); + el.dispatchEvent(new Event('change', { bubbles: true })); + if (typeof el.blur === 'function') el.blur(); + await sleep(0); + } + await sleep(settleMs); + const serialized = Array.from(new FormData(form)).map( + ([k, v]) => [k, typeof v === 'string' ? v : '']); + const invalid = form.noValidate ? [] : Array.from(form.elements) + .filter((e) => e.willValidate && !e.checkValidity()) + .map((e) => `${e.name || e.id}: ${e.validationMessage}`); + let reached = false, prevented = null, threw = null; + const last = (e) => { + if (e.target === form) { reached = true; prevented = e.defaultPrevented; e.preventDefault(); } + }; + const before = state.programmaticSubmits; + window.addEventListener('submit', last); + try { + if (!invalid.length) { + if (typeof form.requestSubmit === 'function') form.requestSubmit(); + else form.dispatchEvent(new Event('submit', { bubbles: true, cancelable: true })); + } + } catch (e) { threw = String(e && e.message || e); } + window.removeEventListener('submit', last); + await sleep(settleMs); + let rules = null, ruleKinds = false; + if (typeof window.validate === 'function' && window.constraints && typeof window.constraints === 'object') { + ruleKinds = true; + try { + const values = typeof window.validate.collectFormValues === 'function' + ? window.validate.collectFormValues(form) : Object.fromEntries(new FormData(form)); + rules = window.validate(values, window.constraints) || null; + } catch (e) { threw = threw || `validate.js: ${String(e && e.message || e)}`; } + } + return { serialized, invalid, reached, prevented, threw, rules, ruleKinds, + programmaticSubmits: state.programmaticSubmits - before, opened: state.opened.slice() }; +} +""" + + +@dataclass +class FormEvaluation: + """What the form's own script made of the run's values.""" + + #: ``valid``, ``invalid`` (the page would refuse these values) or + #: ``unevaluable`` (the script couldn't be run faithfully). + status: str + reason: str = "" + #: The body the browser would send, ``(name, value)`` in order. + serialized: Optional[list[tuple[str, str]]] = None + #: Requests the page tried to send and were blocked, ``METHOD /path``. + blocked: list[str] = field(default_factory=list) + seconds: float = 0.0 + + @property + def valid(self) -> bool: + return self.status == "valid" + + +def _tried(blocked: list[str]) -> str: + sent = sorted({b.removeprefix("after load: ") for b in blocked}) + return f"the form's script tried to send {', '.join(sent)} while it ran (blocked)" + + +def _summary(request: Any) -> str: + return f"{request.method} {up.urlsplit(request.url).path}" + + +def evaluate_form( + page: Any, + base_url: str, + form_path: str, + action_path: str, + fields: list[tuple[str, str]], + *, + timeout_s: float = 20.0, + settle_ms: int = 150, +) -> FormEvaluation: + """Run the form's script in a scratch copy of ``page``'s session. + + ``fields`` are the values the person would type or pick (the API step's + ``param`` and ``select_label`` fields). Never raises: a failure to run + the script is an ``unevaluable`` result. + """ + + started = time.monotonic() + blocked: list[str] = [] + errors: list[str] = [] + dialogs: list[str] = [] + phase = {"loaded": False} + origin = up.urlsplit(base_url) + context = None + scratch = None + own_context = False + + def route(r: Any) -> None: + req = r.request + same_origin = up.urlsplit(req.url).netloc == origin.netloc + script_sent = req.resource_type in ("xhr", "fetch", "eventsource", "websocket") + if req.method not in ("GET", "HEAD") or (phase["loaded"] and script_sent): + blocked.append(("after load: " if phase["loaded"] else "") + _summary(req)) + r.abort() + elif req.resource_type in ("image", "media", "font") or ( + not same_origin and req.resource_type not in ("script", "stylesheet") + ): + r.abort() + else: + r.continue_() + + def done(status: str, reason: str = "", serialized: Any = None) -> FormEvaluation: + return FormEvaluation( + status=status, + reason=reason, + serialized=serialized, + blocked=list(blocked), + seconds=round(time.monotonic() - started, 3), + ) + + try: + live = page.context + browser = getattr(live, "browser", None) + if browser is not None: + # A separate context keeps the run's own tabs, history and page + # count untouched; it shares the session through its cookies. + context = browser.new_context( + storage_state=live.storage_state(), + viewport=page.viewport_size or {"width": 1280, "height": 800}, + ) + own_context = True + else: + context = live + scratch = context.new_page() + # Page-level, so a scratch tab in the live context leaves the run's + # own pages alone. + scratch.add_init_script(_INSTRUMENT_JS) + scratch.route("**/*", route) + scratch.on("pageerror", lambda exc: errors.append(str(exc)[:200])) + + def on_dialog(d: Any) -> None: + dialogs.append(f"{d.type}: {d.message[:160]}") + try: + d.dismiss() + except Exception: # noqa: BLE001 - already handled + pass + + scratch.on("dialog", on_dialog) + resp = scratch.goto( + base_url + form_path, wait_until="load", timeout=timeout_s * 1000.0 + ) + if resp is None or resp.status != 200: + return done( + "unevaluable", + f"the form page returned {getattr(resp, 'status', 'nothing')} in the scratch page", + ) + if errors: + return done( + "unevaluable", + f"the form page raised an error as it loaded ({errors[0]})", + ) + phase["loaded"] = True + load_blocked = list(blocked) + result = scratch.evaluate( + _EVALUATE_JS, + { + "actionPath": action_path, + "leaf": action_path.rsplit("/", 1)[-1], + "fields": [[k, v] for k, v in fields], + "settleMs": settle_ms, + }, + ) + serialized = [(str(k), str(v)) for k, v in result.get("serialized") or []] + late = [b for b in blocked if b not in load_blocked] + if late: + return done("unevaluable", _tried(late), serialized) + if result.get("unevaluable"): + return done("unevaluable", result["unevaluable"]) + if errors: + return done( + "unevaluable", f"the form's script raised an error ({errors[0]})" + ) + if result.get("threw"): + return done( + "unevaluable", f"the form's script raised an error ({result['threw']})" + ) + if result.get("opened"): + return done("unevaluable", "the form's script opened a window while it ran") + if dialogs: + return done( + "invalid", + f"the form's script showed a dialog ({dialogs[0]})", + serialized, + ) + if result.get("invalid"): + return done( + "invalid", + "the browser's own checks refuse these values (" + + "; ".join(result["invalid"][:3]) + + ")", + serialized, + ) + if result.get("rules"): + names = ", ".join(sorted(result["rules"])[:5]) + return done( + "invalid", + f"the form's validation rules refuse these values ({names})", + serialized, + ) + if not result.get("reached"): + return done( + "unevaluable", + "the form's script stopped the submit before it could be seen", + serialized, + ) + if result.get("prevented") and not result.get("programmaticSubmits"): + return done( + "invalid", + "the form's script refused the submit for these values", + serialized, + ) + return done("valid", "", serialized) + except Exception as exc: # noqa: BLE001 - a failure to run is unevaluable + late = [b for b in blocked if b.startswith("after load: ")] + if late: + # For example the page let the browser's own submit through + # without our listener seeing it: the route blocked it. + return done("unevaluable", _tried(late)) + return done("unevaluable", f"the scratch page failed ({type(exc).__name__})") + finally: + try: + if scratch is not None: + scratch.close() + if own_context and context is not None: + context.close() + except Exception: # noqa: BLE001 - cleanup only + pass diff --git a/packages/flow/openadapt_flow/runtime/actuators/web_form.py b/packages/flow/openadapt_flow/runtime/actuators/web_form.py index 74dced3a5d..8638b11624 100644 --- a/packages/flow/openadapt_flow/runtime/actuators/web_form.py +++ b/packages/flow/openadapt_flow/runtime/actuators/web_form.py @@ -225,6 +225,21 @@ def unsupported_client_rules(rules: Optional[dict[str, Any]]) -> list[str]: return sorted(out) +def page_scripts( + html: str, action_path: str, sent: Iterable[str], rules: Optional[dict[str, Any]] +) -> list[str]: + """Everything the browser runs on this save that reading the form can't evaluate. + + :func:`form_scripts` plus the validate.js rule kinds outside + :data:`CLIENT_RULE_KINDS`, sorted. The API step runs these itself before + each save (:mod:`openadapt_flow.runtime.actuators.form_eval`). + """ + + found = form_scripts(html, action_path, sent) + found += [f"{rule} (validate.js)" for rule in unsupported_client_rules(rules)] + return sorted(set(found)) + + _EMAIL = re.compile(r"^[^\s@]+@[^\s@]+\.[^\s@]+$") @@ -418,6 +433,10 @@ def get(self, path_and_query: str) -> Response: ... def post_form(self, path: str, body: str, *, referer: str) -> Response: ... + # Optional: ``evaluate_form(form_path, action_path, fields) -> FormEvaluation`` + # runs the form's own script on the run's values (form_eval). A transport + # without it can't run a form that has script; that save stays on the GUI. + class PlaywrightSessionTransport: """The browser context's own request API: same cookies, same session.""" @@ -425,9 +444,24 @@ class PlaywrightSessionTransport: def __init__(self, page: Any, *, timeout_s: float = 30.0) -> None: parts = up.urlsplit(page.url) self.base_url = f"{parts.scheme}://{parts.netloc}" + self._page = page self._request = page.context.request self._timeout_ms = timeout_s * 1000.0 + def evaluate_form( + self, form_path: str, action_path: str, fields: list[tuple[str, str]] + ) -> Any: + from openadapt_flow.runtime.actuators.form_eval import evaluate_form + + return evaluate_form( + self._page, + self.base_url, + form_path, + action_path, + fields, + timeout_s=self._timeout_ms / 1000.0, + ) + def _wrap(self, resp: Any) -> Response: return _Resp( resp.status, resp.text(), {k.lower(): v for k, v in resp.headers.items()} @@ -649,25 +683,27 @@ def unavailable(reason: str) -> ApiActuationResult: return unavailable( "the form's validation rules changed since the recording" ) - unsupported = unsupported_client_rules(spec.client_rules) - if unsupported: - return unavailable( - "the form validates with rules the API step can't evaluate " - f"({', '.join(unsupported)})" - ) - scripts = form_scripts( + action_path = up.urlsplit(save_path).path + scripts = page_scripts( page.text, - up.urlsplit(save_path).path, + action_path, [f.name for f in spec.fields], + spec.client_rules, ) - if scripts: - return unavailable( - "the form runs script the API step can't run " - f"({', '.join(scripts[:5])})" - ) + if scripts != sorted(set(spec.client_scripts)): + if not spec.client_scripts: + return unavailable( + "the form runs script the API step wasn't proposed to run " + f"({', '.join(scripts[:5])})" + ) + return unavailable("the form's script changed since the recording") body, problem = self.build_body(spec, params, controls) if problem: return unavailable(problem) + if scripts: + problem = self._run_page_script(spec, form_path, action_path, body) + if problem: + return unavailable(problem) for check in spec.prechecks: query = [(k, _fill(v, params)) for k, v in check.query] qs = "&".join( @@ -702,6 +738,49 @@ def unavailable(reason: str) -> ApiActuationResult: ), ) + def _run_page_script( + self, + spec: Any, + form_path: str, + action_path: str, + body: list[tuple[str, str]], + ) -> Optional[str]: + """Run the form's own script on the run's values; why not to send, or None. + + The fields the person typed or picked go into a scratch copy of the + form (form_eval). The save is sent only when the page's script lets + the submit through and the browser would send exactly ``body``, apart + from the session values the scratch page was issued for itself. + """ + + evaluate = getattr(self.transport, "evaluate_form", None) + if evaluate is None: + return "the form runs script and this session can't run it" + typed = {f.name for f in spec.fields if f.source in ("param", "select_label")} + result = evaluate( + form_path, action_path, [(k, v) for k, v in body if k in typed] + ) + self.log.append( + { + "stage": "page_script", + "status": result.status, + "seconds": result.seconds, + "blocked": result.blocked, + } + ) + if not result.valid: + return f"{result.reason}; the page's own script decided" + session = {f.name for f in spec.fields if f.source == "session"} + want = sorted((k, v) for k, v in body if k not in session) + got = sorted((k, v) for k, v in result.serialized or [] if k not in session) + if want != got: + changed = sorted({k for k, _ in set(want) ^ set(got)}) + return ( + "the page would send other values than the API step built " + f"({', '.join(changed[:5])}); its script changes them" + ) + return None + def send(self, binding: Any, prepared: "PreparedSave") -> ApiActuationResult: """Send exactly ``prepared``, once. Never retried.""" diff --git a/packages/flow/tests/test_api_step_native_validation.py b/packages/flow/tests/test_api_step_native_validation.py index 6ca5eea5a8..9d270f987e 100644 --- a/packages/flow/tests/test_api_step_native_validation.py +++ b/packages/flow/tests/test_api_step_native_validation.py @@ -14,7 +14,7 @@ import pytest from openadapt_flow import network_capture as nc -from openadapt_flow.compiler.api_proposal import propose_api_step +from openadapt_flow.compiler.api_proposal import admit_api_step, propose_api_step from openadapt_flow.ir import WebFormField, WebFormSpec from openadapt_flow.runtime.actuators import ActuationStatus from openadapt_flow.runtime.actuators.web_form import ( @@ -26,7 +26,6 @@ from tests.test_api_tier_web_form import ( FORM_HTML, FakeTransport, - _admitted_binding, _recording, _workflow, ) @@ -139,17 +138,24 @@ def _rec_with_form(tmp_path, html): ), ], ) -def test_script_the_api_step_cant_run_keeps_the_save_on_the_gui(tmp_path, edit, named): +def test_script_on_the_form_is_recorded_and_must_be_run_before_the_save( + tmp_path, edit, named +): html = edit(FORM_HTML) - _wf, step, reasons = propose_api_step(_workflow(), _rec_with_form(tmp_path, html)) - assert step is None and named in " ".join(reasons) - # The same script on the live form makes an admitted API step unavailable. + wf, step, reasons = propose_api_step(_workflow(), _rec_with_form(tmp_path, html)) + assert step is not None and not reasons + scripts = step.api_binding.web_form.client_scripts + assert any(named in s for s in scripts) + wf, _ = admit_api_step(wf, admitted_by="reviewer") + binding = wf.steps[-1].api_binding + run = {"lname": "Marsh", "state_label": "California", "note": "Referred"} live = edit(FORM_HTML.replace("", "live-session-value")) - (tmp_path / "admitted").mkdir() - out = WebFormActuator(FakeTransport(form=live)).actuate( - _admitted_binding(tmp_path / "admitted"), - {"lname": "Marsh", "state_label": "California", "note": "Referred"}, - ) + # A session that can't run the page's script keeps the save on the GUI. + out = WebFormActuator(FakeTransport(form=live)).actuate(binding, run) + assert out.status is ActuationStatus.UNAVAILABLE + assert "can't run it" in out.reason or "rules changed" in out.reason + # So does a live form whose script is not the recorded script. + plain = FORM_HTML.replace("", "live-session-value") + out = WebFormActuator(FakeTransport(form=plain)).actuate(binding, run) assert out.status is ActuationStatus.UNAVAILABLE - # A rule kind the recording didn't have is first a changed rule set. - assert named in out.reason or "rules changed" in out.reason + assert "script changed" in out.reason or "rules changed" in out.reason diff --git a/packages/flow/tests/test_api_step_page_script.py b/packages/flow/tests/test_api_step_page_script.py new file mode 100644 index 0000000000..d83bc57c1a --- /dev/null +++ b/packages/flow/tests/test_api_step_page_script.py @@ -0,0 +1,367 @@ +"""The API step runs a form's own script before it sends the save. + +A form whose fields have handlers, whose ``onsubmit`` validates, or whose +validate.js rules include a custom kind (OpenEMR's ``pastDate``) used to keep +the save on the GUI. Now the runtime runs that script in a scratch copy of +the session, on the run's values, with every write blocked, and sends only +when the page would submit the same body. These tests use a real headless +Chromium against a local server. Synthetic values only. +""" + +from __future__ import annotations + +import threading +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from typing import Any + +import pytest + +from openadapt_flow.ir import WebFormField, WebFormSpec +from openadapt_flow.runtime.actuators.form_eval import FormEvaluation +from openadapt_flow.runtime.actuators.web_form import ( + PlaywrightSessionTransport, + WebFormActuator, +) + +playwright = pytest.importorskip("playwright.sync_api") + +FORM = """ +
+ + + + + +
+""" + +VALIDATE = 'onsubmit="return pastDate(this.dob.value)"' + + +class _App: + """A local app: serves pages by path and counts every write.""" + + def __init__(self) -> None: + self.pages: dict[str, str] = {"/": "home"} + self.writes: list[str] = [] + self.loads = 0 + app = self + + class Handler(BaseHTTPRequestHandler): + def log_message(self, *a: Any) -> None: + pass + + def do_GET(self) -> None: # noqa: N802 + path = self.path.split("?")[0] + if path in app.pages: + app.loads += 1 + body = app.pages[path].replace("{n}", str(app.loads)).encode() + self.send_response(200) + self.send_header("Content-Type", "text/html") + else: + body = b"no" + self.send_response(404) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def do_POST(self) -> None: # noqa: N802 + app.writes.append(self.path) + self.send_response(200) + self.send_header("Content-Length", "2") + self.end_headers() + self.wfile.write(b"ok") + + self.server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + threading.Thread(target=self.server.serve_forever, daemon=True).start() + self.base = f"http://127.0.0.1:{self.server.server_address[1]}" + + def form( + self, *, form_attrs: str = VALIDATE, lname_attrs: str = "", script: str = "" + ) -> None: + self.pages["/new.php"] = FORM.format( + form_attrs=form_attrs, lname_attrs=lname_attrs, script=script, n="{n}" + ) + + +@pytest.fixture(scope="module") +def browser(): + with playwright.sync_playwright() as pw: + b = pw.chromium.launch(headless=True) + yield b + b.close() + + +@pytest.fixture +def app_page(browser): + app = _App() + context = browser.new_context() + context.add_cookies([{"name": "sid", "value": "live", "url": app.base}]) + page = context.new_page() + page.goto(app.base + "/") + yield app, page + context.close() + app.server.shutdown() + + +GOOD = [("lname", "Marsh"), ("dob", "1979-06-14"), ("state", "CA")] + + +def _evaluate(app: _App, page: Any, fields=GOOD) -> FormEvaluation: + return PlaywrightSessionTransport(page).evaluate_form( + "/new.php", "/save.php", fields + ) + + +def test_values_the_page_accepts_are_valid_and_nothing_is_written(app_page): + app, page = app_page + app.form() + result = _evaluate(app, page) + assert result.status == "valid", result.reason + assert ("lname", "Marsh") in result.serialized and ( + "state", + "CA", + ) in result.serialized + assert app.writes == [] + # The run's own tab is untouched: same page, same URL, one tab. + assert page.url == app.base + "/" and len(page.context.pages) == 1 + + +def test_a_custom_rule_that_refuses_the_value_makes_it_invalid(app_page): + app, page = app_page + app.form() + result = _evaluate( + app, page, [("lname", "Marsh"), ("dob", "2999-01-01"), ("state", "CA")] + ) + assert result.status == "invalid" and "refused the submit" in result.reason + assert app.writes == [] + + +def test_a_handler_that_rewrites_a_value_shows_in_the_body_it_would_send(app_page): + app, page = app_page + app.form(lname_attrs='onchange="this.value = this.value.toUpperCase()"') + result = _evaluate(app, page) + assert result.status == "valid" + assert ("lname", "MARSH") in result.serialized + + +def test_script_that_tries_to_write_is_blocked_and_unevaluable(app_page): + app, page = app_page + app.form( + form_attrs="onsubmit=\"fetch('/audit.php', {method: 'POST', body: 'x'}); return true\"" + ) + result = _evaluate(app, page) + assert result.status == "unevaluable" and "POST /audit.php" in result.reason + assert app.writes == [] + + +def test_a_programmatic_submit_after_validation_counts_and_never_navigates(app_page): + app, page = app_page + app.form( + form_attrs='onsubmit="if (pastDate(this.dob.value)) this.submit(); return false"' + ) + result = _evaluate(app, page) + assert result.status == "valid", result.reason + assert app.writes == [] + + +@pytest.mark.parametrize( + "kw, status, reason", + [ + ( + {"script": "undefinedThing();"}, + "unevaluable", + "raised an error as it loaded", + ), + ( + { + "form_attrs": "onsubmit=\"alert('Date of birth is required'); return false\"" + }, + "invalid", + "showed a dialog", + ), + ( + { + "form_attrs": "", + "script": "document.forms[0].addEventListener('submit', e => e.stopPropagation())", + }, + "unevaluable", + "tried to send POST /save.php", + ), + ({"form_attrs": "", "lname_attrs": "required"}, "valid", ""), + ( + { + "form_attrs": "", + "script": "var constraints = {dob: {pastDate: true}};" + "function validate(values, c) { return pastDate(values.dob) ? undefined : {dob: ['must be past']}; }" + "validate.collectFormValues = f => Object.fromEntries(new FormData(f));", + }, + "valid", + "", + ), + ], +) +def test_what_the_page_does_decides(app_page, kw, status, reason): + app, page = app_page + app.form(**kw) + result = _evaluate(app, page) + assert result.status == status and reason in result.reason, result.reason + assert app.writes == [] + + +def test_native_constraints_and_validate_js_rules_refuse(app_page): + app, page = app_page + app.form(form_attrs="", lname_attrs="required") + assert ( + _evaluate( + app, page, [("lname", ""), ("dob", "1979-06-14"), ("state", "CA")] + ).status + == "invalid" + ) + app.form( + form_attrs="", + script="var constraints = {dob: {pastDate: true}};" + "function validate(values, c) { return pastDate(values.dob) ? undefined : {dob: ['must be past']}; }", + ) + result = _evaluate( + app, page, [("lname", "Marsh"), ("dob", "2999-01-01"), ("state", "CA")] + ) + assert result.status == "invalid" and "dob" in result.reason + + +def test_an_unknown_option_or_a_missing_form_is_unevaluable(app_page): + app, page = app_page + app.form() + assert _evaluate(app, page, [("state", "ZZ")]).status == "unevaluable" + assert ( + PlaywrightSessionTransport(page) + .evaluate_form("/new.php", "/other.php", GOOD) + .status + == "unevaluable" + ) + + +# -- the actuator: what it does with the page's verdict ------------------------ + + +class _Transport: + base_url = "http://app" + + def __init__(self, evaluation: FormEvaluation) -> None: + self.evaluation = evaluation + self.calls: list[Any] = [] + self.posts: list[str] = [] + + def get(self, path): + from openadapt_flow.runtime.actuators.web_form import _Resp + + return _Resp( + 200, FORM.format(form_attrs=VALIDATE, lname_attrs="", script="", n="1"), {} + ) + + def post_form(self, path, body, *, referer): + self.posts.append(body) + from openadapt_flow.runtime.actuators.web_form import _Resp + + return _Resp(302, "", {"location": "/view.php?id=7"}) + + def evaluate_form(self, form_path, action_path, fields): + self.calls.append(fields) + return self.evaluation + + +def _binding(): + from openadapt_flow.ir import ApiBinding + + spec = WebFormSpec( + form_path="/new.php", + fields=[ + WebFormField(name="csrf", source="session"), + WebFormField(name="lname", source="param", param="lname"), + WebFormField(name="dob", source="param", param="dob"), + WebFormField(name="state", source="select_label", param="state_label"), + ], + controls=["csrf", "dob", "lname", "state"], + client_scripts=["dob onchange", "form onsubmit"], + replaces_steps=["s0"], + folded_checks_digest="sha256:" + "0" * 64, + ) + return ApiBinding( + kind="web_form", method="POST", url_template="/save.php", web_form=spec + ) + + +RUN = {"lname": "Marsh", "dob": "1979-06-14", "state_label": "California"} + + +def _evaluation(status="valid", serialized=None, reason=""): + return FormEvaluation( + status=status, + reason=reason, + serialized=serialized + if serialized is not None + else [ + ("csrf", "tok-9"), + ("lname", "Marsh"), + ("dob", "1979-06-14"), + ("state", "CA"), + ], + ) + + +def test_the_actuator_sends_only_what_the_page_would_send(): + t = _Transport(_evaluation()) + out = WebFormActuator(t).actuate(_binding(), RUN) + assert out.status.value == "actuated", out.reason + # Only the typed and picked fields are put into the scratch form. + assert t.calls == [[("lname", "Marsh"), ("dob", "1979-06-14"), ("state", "CA")]] + assert len(t.posts) == 1 and "csrf=tok-1" in t.posts[0] + + +@pytest.mark.parametrize( + "evaluation, reason", + [ + ( + _evaluation("invalid", reason="the form's script refused the submit"), + "refused the submit", + ), + ( + _evaluation("unevaluable", reason="the scratch page failed"), + "scratch page failed", + ), + ( + _evaluation( + serialized=[ + ("csrf", "x"), + ("lname", "MARSH"), + ("dob", "1979-06-14"), + ("state", "CA"), + ] + ), + "its script changes them", + ), + ( + _evaluation( + serialized=[ + ("csrf", "x"), + ("lname", "Marsh"), + ("dob", "1979-06-14"), + ("state", "CA"), + ("extra", "1"), + ] + ), + "extra", + ), + ], +) +def test_the_actuator_leaves_the_save_to_the_gui_when_the_page_disagrees( + evaluation, reason +): + t = _Transport(evaluation) + out = WebFormActuator(t).actuate(_binding(), RUN) + assert out.status.value == "unavailable" and reason in out.reason + assert t.posts == [] From 8656dcf4e75d1cf26127256991d479969a860a63 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sat, 24 Oct 2026 23:53:51 -0400 Subject: [PATCH 02/15] docs: refresh llms-full.txt Co-Authored-By: Claude Opus 5.5 --- llms-full.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/llms-full.txt b/llms-full.txt index 02ba1a00d5..3e28afeddb 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -563,6 +563,7 @@ improvising or retrying when something is off. | Learn one program with branches from two or more recordings, or say which recordings are missing | Released | Synthetic | `openadapt-flow induce`, `disambiguate`; [CAPABILITIES.md (flow)](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/CAPABILITIES.md) | | Learn a guarded branch from several halts fixed the same way | Released | Synthetic | `openadapt-flow teach-branch`, `learning/branch_teach.py` | | Run a program over a worklist | Released | Real app | `openadapt-flow for-each` | +| Learn a worklist loop from recordings that search a record, open it, work it and go back to the list, even when the last row skips the return or rows differ (a field some rows lack, a write some rows get behind a screen check a person accepts). Each pass selects the row's own record by its values, verifies it, and checks that row's write. Certified and governed runs refuse it until qualification covers it | On main ([#1435](https://github.com/OpenAdaptAI/OpenAdapt/pull/1435)) | Synthetic (held-out rows run through the interpreter against an in-memory app) | `openadapt-flow induce`; `Step.target_param` | | Typed business decisions inside a run | Released | Synthetic | `business_decision` states, `openadapt-flow business-decisions` | | Watch many real runs in the background and mine candidate branches (shadow mode) | On main ([#1212](https://github.com/OpenAdaptAI/OpenAdapt/pull/1212)) | Synthetic | [SHADOW_MINING.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/capture/docs/SHADOW_MINING.md), [LEARN_FROM_MANY_RUNS.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/LEARN_FROM_MANY_RUNS.md) | | Fill a workflow's inputs from a referral PDF, fax image or email | Released | Synthetic | `openadapt-flow intake`, [INTAKE.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/INTAKE.md) | @@ -595,6 +596,7 @@ improvising or retrying when something is off. | Run a step as a direct API call (`ApiActuator`, `ApiBinding`) | Released, hand-authored bindings | Synthetic benchmarks | `runtime/actuators/api.py` | | Capture a recording's network traffic and compile a clean save into an API step, with the same record check | On main ([#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269)) | Synthetic patients on a local OpenEMR test copy | network capture, `ApiBinding` | | Fall back from the API step to page structure when the API is unavailable, before anything is sent | On main ([#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269)) | Synthetic | [#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269) | +| Run a form's own script before an API save instead of refusing the form: field handlers, `onsubmit`, submit listeners and custom validate.js rules (OpenEMR's `pastDate`) run on the run's values in a scratch copy of the session with every write blocked, and the save is sent only when the page lets the submit through and would send the same body | On main | Synthetic (headless Chromium against a local test page) | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#what-happens-at-run-time), `runtime/actuators/form_eval.py` | | Approve an API step's exact request under per-item approval: one digest binds the request bytes, the record and the record check, and a changed request is not sent | On main ([#1355](https://github.com/OpenAdaptAI/OpenAdapt/pull/1355)) | Synthetic | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#under-an-approval-mode) | | Run an API step in a durable run: checkpoint the span after the save is sent, never resend or redo a save that may have landed, and reconcile it by reading the record back | On main ([#1358](https://github.com/OpenAdaptAI/OpenAdapt/pull/1358)) | Synthetic | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#in-a-durable-run) | | Run an admitted API step under a governed run authorization and the Standard or Regulated profile: the authorization names the API step by digest, the person who admits it names the right-record parameter, and the outcome reads `VERIFIED` | On main ([#1361](https://github.com/OpenAdaptAI/OpenAdapt/pull/1361)) | Synthetic | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#under-governance) | From 2dd666d00fb27eea1c20925f74f1a03a949d78e1 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sat, 24 Oct 2026 23:48:32 -0400 Subject: [PATCH 03/15] flow: declare an API step's other writes as an ordered sequence Any other write sent while the replaced steps ran refused the API step. Each is now declared in the proposal (web_form.writes), in recorded order: - ambient: a script request from another document than the form, loaded before the form opened, to an endpoint the form page doesn't name (for example OpenEMR's tab shell posting execute_background_services.php on a timer). No replaced step caused it, so the API step doesn't send it. - step: a write a replaced step caused, rebuilt like the save and sent in its recorded place before it. It is read back by the record check of the step it was sent during (admission refuses a step write whose step has none; the binding's record check is the save's plus those), and a failure or an answer outside its recorded status stops the run with nothing after it sent and nothing retried. The approved digest covers every request. Anything else still refuses the proposal. Co-Authored-By: Claude Opus 5.5 --- CAPABILITIES.md | 1 + packages/flow/docs/API_TIER.md | 43 ++- .../openadapt_flow/compiler/api_proposal.py | 189 ++++++++++-- packages/flow/openadapt_flow/ir.py | 49 +++- .../runtime/actuators/web_form.py | 115 +++++++- .../openadapt_flow/runtime/authorization.py | 4 +- .../flow/openadapt_flow/runtime/replayer.py | 7 +- .../tests/test_api_step_write_sequence.py | 271 ++++++++++++++++++ 8 files changed, 644 insertions(+), 35 deletions(-) create mode 100644 packages/flow/tests/test_api_step_write_sequence.py diff --git a/CAPABILITIES.md b/CAPABILITIES.md index f7c2b78ff2..51c7db83d9 100644 --- a/CAPABILITIES.md +++ b/CAPABILITIES.md @@ -200,6 +200,7 @@ improvising or retrying when something is off. | Capture a recording's network traffic and compile a clean save into an API step, with the same record check | On main ([#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269)) | Synthetic patients on a local OpenEMR test copy | network capture, `ApiBinding` | | Fall back from the API step to page structure when the API is unavailable, before anything is sent | On main ([#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269)) | Synthetic | [#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269) | | Run a form's own script before an API save instead of refusing the form: field handlers, `onsubmit`, submit listeners and custom validate.js rules (OpenEMR's `pastDate`) run on the run's values in a scratch copy of the session with every write blocked, and the save is sent only when the page lets the submit through and would send the same body | On main | Synthetic (headless Chromium against a local test page) | [API_TIER.md](packages/flow/docs/API_TIER.md#what-happens-at-run-time), `runtime/actuators/form_eval.py` | +| Declare the other writes in an API step's span as an ordered sequence: the page's own background requests (another document's script, loaded before the form, to an endpoint the form doesn't name) are listed and not sent; a write a step caused is sent in its recorded place before the save, read back by that step's record check, and a failure stops the run with nothing after it sent and nothing retried | On main | Synthetic | [API_TIER.md](packages/flow/docs/API_TIER.md#other-writes-in-the-span) | | Approve an API step's exact request under per-item approval: one digest binds the request bytes, the record and the record check, and a changed request is not sent | On main ([#1355](https://github.com/OpenAdaptAI/OpenAdapt/pull/1355)) | Synthetic | [API_TIER.md](packages/flow/docs/API_TIER.md#under-an-approval-mode) | | Run an API step in a durable run: checkpoint the span after the save is sent, never resend or redo a save that may have landed, and reconcile it by reading the record back | On main ([#1358](https://github.com/OpenAdaptAI/OpenAdapt/pull/1358)) | Synthetic | [API_TIER.md](packages/flow/docs/API_TIER.md#in-a-durable-run) | | Run an admitted API step under a governed run authorization and the Standard or Regulated profile: the authorization names the API step by digest, the person who admits it names the right-record parameter, and the outcome reads `VERIFIED` | On main ([#1361](https://github.com/OpenAdaptAI/OpenAdapt/pull/1361)) | Synthetic | [API_TIER.md](packages/flow/docs/API_TIER.md#under-governance) | diff --git a/packages/flow/docs/API_TIER.md b/packages/flow/docs/API_TIER.md index 2eca30b092..23e3eff981 100644 --- a/packages/flow/docs/API_TIER.md +++ b/packages/flow/docs/API_TIER.md @@ -3,8 +3,11 @@ **Scope: a single HTML form submission.** An API step repeats one `application/x-www-form-urlencoded` POST of one HTML form, read again from the live page, in place of the clicks and keystrokes that filled it. It is -not a general API client: JSON or script-built requests, more than one -write and multi-page server state stay on the GUI. A form whose script +not a general API client: JSON or script-built requests and multi-page +server state stay on the GUI. Other writes during the replaced steps are a +declared sequence (below): the page's own background requests are listed +and left to the page, and a write a step caused is sent in its recorded +place before the save and read back. A form whose script validates or rewrites values is run, not refused: the API step runs that script on the run's values before each save (below), and the save stays on the GUI when the script can't be run. @@ -95,10 +98,10 @@ doesn't propose an API step in these cases: - the save sends a secret field; - the form page wasn't captured; - the save isn't the program's last step; -- the steps the API step would replace also sent another write (a POST, - PUT, PATCH or DELETE other than the save, such as a reservation, an - autosave or a background call). Admission and runs refuse such a binding - too; +- the steps the API step would replace also sent a write (a POST, PUT, + PATCH or DELETE other than the save) that is neither the page's own + request nor a write the API step can repeat (see "Other writes in the + span"); - the program has branches; - a URL value equals two inputs' values (ambiguous), or the save or a pre-check is on another origin than the form; @@ -115,6 +118,34 @@ it sends the save. On-screen checks of steps that made no request (layout and region checks) are screen-only. They are listed for the person who admits the step and aren't repeated. +## Other writes in the span + +Every other write sent while the replaced steps ran is declared in the +proposal (`web_form.writes`), in recorded order, with one of two roles: + +- **ambient**: the page sent it on its own. It is a script request (fetch or + XHR) from another document than the form, one that was loaded before the + form opened, to an endpoint the form page's source doesn't name. OpenEMR's + tab shell, for example, posts `execute_background_services.php` on a + timer while the patient form is open. No replaced step caused it, so the + API step neither sends nor waits for it: the live page goes on sending it, + as it does under the GUI steps. `api show` lists it with that evidence. +- **step**: a replaced step caused it. The API step sends it, rebuilt the + same way as the save (typed parameters by which step typed them, session + values and unchanged form defaults from the live form), in its recorded + place before the save. A write whose body isn't form-encoded, that carries + values in its URL or another value, that went to another origin, or that + wasn't accepted when recorded refuses the proposal. Each step write is + read back by the record check declared on the step it was sent during: + admission refuses one whose step has none, and the binding's record check + is the save's followed by those steps' checks. If a write fails or answers + outside its recorded status, the run stops there: nothing after it is + sent, nothing is retried or redone through the GUI, and the run reconciles + by reading the records back. Under approval, the digest the person + approves covers every request in the sequence. + +A write that fits neither role keeps the save on the GUI. + ## What happens at run time The API step uses the replaying browser's own session: its requests go diff --git a/packages/flow/openadapt_flow/compiler/api_proposal.py b/packages/flow/openadapt_flow/compiler/api_proposal.py index d85e5694e7..b6dce4b5b0 100644 --- a/packages/flow/openadapt_flow/compiler/api_proposal.py +++ b/packages/flow/openadapt_flow/compiler/api_proposal.py @@ -67,6 +67,7 @@ WebFormField, WebFormPrecheck, WebFormSpec, + WebFormWrite, Workflow, ) from openadapt_flow.runtime.actuators.web_form import ( @@ -74,6 +75,7 @@ folded_checks_digest, form_controls, page_scripts, + web_form_effects, ) @@ -246,6 +248,140 @@ def _success_pattern(line: dict[str, Any]) -> Optional[str]: return r".*?".join(pieces) +def _span_writes( + lines: list[dict[str, Any]], + post: dict[str, Any], + form_line: dict[str, Any], + folded: list[Step], + events: list[dict[str, Any]], + controls: dict[str, Any], + typed: dict[str, Optional[str]], + params: dict[str, str], +) -> tuple[list[WebFormWrite], list[str]]: + """Every other write sent while the replaced steps ran, each with its role. + + ``ambient`` when the page sent it on its own: a script request (fetch or + XHR) from another document than the form, one loaded before the form + opened, to an endpoint the form page's source doesn't name. ``step`` + otherwise: sent in its recorded place before the save, rebuilt from a + form-encoded body whose every field is a typed parameter (by provenance, + as for the save), a session value or an unchanged form default, and read + back by the record check of the step it was sent during. Returns the + problems that refuse the proposal. + """ + + form_t, post_t = float(form_line["t"]), float(post["t"]) + form_doc = _path(form_line["url"]) + form_html = form_line.get("response_text") or "" + loaded_before = { + ln["url"] + for ln in lines + if ln["method"] == "GET" + and ln.get("resource_type") == "document" + and float(ln["t"]) < form_t + } + stamps = [ + (float(events[i]["t"]), s.id) + for s in folded + if (i := _event_index(s)) is not None and i < len(events) + ] + out: list[WebFormWrite] = [] + problems: list[str] = [] + for ln in lines: + if ( + ln["method"] in ("GET", "HEAD", "OPTIONS") + or ln is post + or not form_t < float(ln["t"]) < post_t + ): + continue + method, path = ln["method"], _path(ln["url"]) + sender = ln.get("frame_url") or "" + leaf = path.rsplit("/", 1)[-1] + if ( + ln.get("resource_type") in ("fetch", "xhr") + and sender + and _path(sender) != form_doc + and sender in loaded_before + and leaf + and leaf not in form_html + ): + out.append( + WebFormWrite( + role="ambient", + method=method, + path=path, + sent_by=_path(sender), + evidence=( + f"sent by a script of {_path(sender)}, a page loaded before " + "the form opened; the form page doesn't name it" + ), + ) + ) + continue + during = [sid for t, sid in stamps if t <= float(ln["t"])] + from_step = during[-1] if during else None + what = f"{method} {path}" + if from_step is None: + problems.append( + f"{what} was sent before any replaced step; its cause is unknown" + ) + continue + if _origin(ln["url"]) != _origin(post["url"]): + problems.append(f"the steps also sent {what} to another origin") + continue + if up.urlsplit(ln["url"]).query: + problems.append(f"{from_step} also sent {what} with values in its URL") + continue + if ln.get("form") is None: + problems.append( + f"{from_step} also sent {what} with a body the API step can't rebuild " + f"({ln.get('body_omitted') or ln.get('request_type') or 'not form-encoded'})" + ) + continue + status = ln.get("status") + if not isinstance(status, int) or not 200 <= status < 400: + problems.append( + f"{from_step} also sent {what}, and it wasn't accepted ({status})" + ) + continue + fields: list[WebFormField] = [] + for name, value in ln["form"]: + ctl = controls.get(name) + param = typed.get(name) + if param is not None and params.get(param) == value: + fields.append(WebFormField(name=name, source="param", param=param)) + elif ( + value.startswith(" tuple[Workflow, Optional[Step], list[str]]: @@ -574,27 +710,11 @@ def carried(line: dict[str, Any]) -> int: ) ) - others = sorted( - { - f"{ln['method']} {_path(ln['url'])}" - for ln in lines - if ln["method"] not in ("GET", "HEAD", "OPTIONS") - and ln is not post - and form_t < float(ln["t"]) < float(post["t"]) - } + writes, write_problems = _span_writes( + lines, post, form_line, folded, events, controls, typed, params ) - if others: - # The steps the API step would replace also wrote (a reservation, an - # autosave, a background call): the one request can't stand in for - # them, and skipping them would change what the program does. - return ( - workflow, - None, - [ - "the steps the API step would replace also sent " - f"{', '.join(others)}; one request can't stand in for them" - ], - ) + if write_problems: + return workflow, None, write_problems spec = WebFormSpec( form_path=form_path, fields=fields, @@ -604,7 +724,7 @@ def carried(line: dict[str, Any]) -> int: prechecks=prechecks, success_pattern=_success_pattern(post), replaces_steps=[s.id for s in folded], - other_writes_seen=others, + writes=writes, screen_only_checks=screen_only, folded_checks_digest=folded_checks_digest(folded), ) @@ -666,6 +786,11 @@ def describe_api_step(step: Step) -> str: ) lines.append( f" record check after: {len(b.effects)} effect(s) copied from the save step" + + ( + " and the steps whose writes it sends" + if any(w.role == "step" for w in spec.writes) + else "" + ) ) for identity in b.identity: lines.append( @@ -685,6 +810,17 @@ def describe_api_step(step: Step) -> str: ) if spec.screen_only_checks: lines.append(f" screen-only checks, not repeated: {spec.screen_only_checks}") + for i, write in enumerate(spec.writes, 1): + if write.role == "ambient": + lines.append( + f" not sent (the page's own request): {write.method} {write.path}; " + f"{write.evidence}" + ) + else: + lines.append( + f" sent before the save, in this order: {write.method} {write.path} " + f"({len(write.fields)} field(s)), read back by {write.from_step}'s record check" + ) if spec.other_writes_seen: lines.append( f" NOT repeated (other writes seen during these steps): {spec.other_writes_seen}" @@ -791,13 +927,22 @@ def admit_api_step( f"{step.id} has no record check; an API step must be confirmed by one " "(add it, for example with record --verify or check mark, then admit)" ) + by_id = {s.id: s for s in workflow.steps} + for write in b.web_form.writes if b.web_form is not None else []: + cause = by_id.get(write.from_step or "") + if write.role == "step" and (cause is None or not cause.effects): + raise ApiProposalError( + f"{step.id}: {write.method} {write.path} is sent during " + f"{write.from_step}, which has no record check; each write the " + "API step sends must be read back (add one to that step, then admit)" + ) identities = [ web_form_identity(b, list(step.effects), key, param) for key, param in sorted((identity or {}).items()) ] clean = b.model_copy( update={ - "effects": list(step.effects), + "effects": web_form_effects(workflow.steps, step), "admission": None, "identity": identities or list(b.identity), } diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index 139b33a069..4996dcceed 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -610,6 +610,45 @@ class WebFormPrecheck(BaseModel): from_step: Optional[str] = None +class WebFormWrite(BaseModel): + """A request, other than the save, that wrote while the replaced steps ran. + + The API step's span is a declared, ordered sequence: these writes in the + order they were recorded, then the save. Each is one of two roles: + + - ``ambient``: the page sent it on its own. It came from a script of + another document than the form, one that was loaded before the form + opened (for example OpenEMR's tab shell, which posts + ``execute_background_services.php`` on a timer), and the form page's + source doesn't name it. No replaced step caused it, so the API step + neither sends nor waits for it; the live page goes on as it does under + the GUI steps. Listed for the person who admits the step. + - ``step``: a replaced step caused it. The API step sends it, rebuilt + from ``fields`` the same way as the save, in its recorded place before + the save. Its read-back is the record check declared on ``from_step``, + which the binding's record check includes; admission refuses a step + write whose step has none. A write that fails, or whose response is + outside ``expected_status``, stops the run with nothing after it sent, + nothing retried and nothing redone through the GUI, and the run + reconciles by reading the records back. + """ + + model_config = ConfigDict(extra="forbid") + + role: Literal["ambient", "step"] + method: str = Field(min_length=1) + path: str = Field(min_length=1) + #: Path of the document whose script or form sent it. + sent_by: str = "" + #: The replaced step during which it was sent (``step`` writes). + from_step: Optional[str] = None + #: How to rebuild the body (``step`` writes): same sources as the save. + fields: list[WebFormField] = Field(default_factory=list) + expected_status: list[int] = Field(default_factory=list) + #: Why it has this role, in words, from the capture. + evidence: str = "" + + class WebFormSpec(BaseModel): """How to repeat a recorded browser form save as one HTTP request. @@ -642,10 +681,14 @@ class WebFormSpec(BaseModel): success_pattern: Optional[str] = None #: GUI steps the API step stands in for, in order. The last one is the save. replaces_steps: list[str] = Field(min_length=1) - #: Other non-GET requests seen while the recorded steps ran. A proposal - #: with any is refused, and so are its admission and its run (a binding - #: proposed before that rule existed may still list some). + #: Other non-GET requests seen while the recorded steps ran that no + #: declared write accounts for. A binding with any is refused at + #: admission and at run time (bindings proposed before ``writes`` + #: existed may list some). other_writes_seen: list[str] = Field(default_factory=list) + #: Every other write seen while the replaced steps ran, in recorded + #: order, each with its role (WebFormWrite). The save comes after them. + writes: list[WebFormWrite] = Field(default_factory=list) #: On-screen checks of folded steps that caused no request (screen-only, #: not repeated), listed for the person admitting the step. screen_only_checks: list[str] = Field(default_factory=list) diff --git a/packages/flow/openadapt_flow/runtime/actuators/web_form.py b/packages/flow/openadapt_flow/runtime/actuators/web_form.py index 8638b11624..b1d7fde922 100644 --- a/packages/flow/openadapt_flow/runtime/actuators/web_form.py +++ b/packages/flow/openadapt_flow/runtime/actuators/web_form.py @@ -362,6 +362,32 @@ def option_for_label(control: dict[str, Any], label: str) -> Optional[str]: return hits[0] if len(hits) == 1 else None +def web_form_effects(steps: Any, save: Any) -> list[Any]: + """The record check an API step must carry: the save's, then each step write's. + + A ``step`` write (WebFormWrite) is read back by the record check declared + on the step it was sent during, so the binding's record check is the + save step's effects followed by those steps' effects, in write order, + each step once. + """ + + out = list(save.effects) + binding = getattr(save, "api_binding", None) + spec = getattr(binding, "web_form", None) + if spec is None: + return out + by_id = {s.id: s for s in steps} + seen: set[str] = set() + for write in spec.writes: + if write.role != "step" or not write.from_step or write.from_step in seen: + continue + seen.add(write.from_step) + step = by_id.get(write.from_step) + if step is not None: + out.extend(step.effects) + return out + + def folded_checks_digest(steps: Any) -> str: """SHA-256 over the complete definition of the steps an API step replaces. @@ -513,22 +539,45 @@ class PreparedSave: #: Fields whose value the server issued to this session (CSRF tokens). #: In the digest; never shown to the person. session_fields: frozenset[str] = frozenset() + #: The span's ``step`` writes, sent in this order before the save. + writes: tuple["PreparedWrite", ...] = () def encoded(self) -> str: return up.urlencode(list(self.body)) def payload_sha256(self) -> str: - payload = { + payload: dict[str, Any] = { "method": self.method.upper(), "path": self.path, "content_type": "application/x-www-form-urlencoded", "body": self.encoded(), "referer": self.referer, } + if self.writes: + # Every request the API step sends, in order. A save without step + # writes keeps the digest it always had. + payload["writes_before_save"] = [ + {"method": w.method.upper(), "path": w.path, "body": w.encoded()} + for w in self.writes + ] canonical = json.dumps(payload, sort_keys=True, separators=(",", ":")) return "sha256:" + hashlib.sha256(canonical.encode("utf-8")).hexdigest() +@dataclass(frozen=True) +class PreparedWrite: + """One ``step`` write of the span, built and checked, not yet sent.""" + + method: str + path: str + body: tuple[tuple[str, str], ...] + expected_status: tuple[int, ...] + from_step: str + + def encoded(self) -> str: + return up.urlencode(list(self.body)) + + def operation_digest( prepared: PreparedSave, *, @@ -582,6 +631,13 @@ def operation_rows( defaults += 1 for name, value in record: rows.append((f"Record check: {name}", value)) + for i, write in enumerate(prepared.writes, 1): + rows.append( + ( + f"Write {i} before the save", + f"{write.method.upper()} {write.path} ({len(write.body)} field(s), from {write.from_step})", + ) + ) rows.append( ( "Request", @@ -724,6 +780,25 @@ def unavailable(reason: str) -> ApiActuationResult: f"the check before saving ({check.from_step or check.path}) did not pass: " f"missing {missing!r}" ) + writes: list[PreparedWrite] = [] + for write in spec.writes: + if write.role != "step": + continue # the page's own request: not the API step's to send + sub = spec.model_copy(update={"fields": write.fields}) + wbody, problem = self.build_body( + sub, params, controls, enforce_rules=False + ) + if problem: + return unavailable(f"{write.method} {write.path}: {problem}") + writes.append( + PreparedWrite( + method=write.method, + path=fill_url(write.path, params), + body=tuple(wbody), + expected_status=tuple(write.expected_status), + from_step=write.from_step or "", + ) + ) except KeyError as exc: return unavailable(f"the run has no value for {exc}") except Exception as exc: # noqa: BLE001 - only reads were sent @@ -736,6 +811,7 @@ def unavailable(reason: str) -> ApiActuationResult: session_fields=frozenset( f.name for f in spec.fields if f.source == "session" ), + writes=tuple(writes), ) def _run_page_script( @@ -786,6 +862,43 @@ def send(self, binding: Any, prepared: "PreparedSave") -> ApiActuationResult: spec = binding.web_form summary = f"{binding.method} {binding.url_template}" + total = len(prepared.writes) + 1 + for i, write in enumerate(prepared.writes, 1): + what = f"{write.method} {write.path}" + done = f"; the {i - 1} write(s) before it were accepted" if i > 1 else "" + try: + wresp = self.transport.post_form( + write.path, write.encoded(), referer=prepared.referer + ) + except Exception as exc: # noqa: BLE001 - after dispatch began + self.log.append({"stage": "write", "n": i, "error": type(exc).__name__}) + return ApiActuationResult( + status=ActuationStatus.HALT, + substrate=self.substrate, + halt_kind=ApiHaltKind.DELIVERY_UNCERTAIN, + reason=( + f"write {i} of {total} ({what}) failed after it was sent " + f"({type(exc).__name__}){done}; it may have written, nothing " + "after it was sent and nothing is retried" + ), + request_summary=summary, + ) + accepted = wresp.status in (write.expected_status or tuple(range(200, 400))) + self.log.append( + {"stage": "write", "n": i, "status": wresp.status, "accepted": accepted} + ) + if not accepted: + return ApiActuationResult( + status=ActuationStatus.HALT, + substrate=self.substrate, + halt_kind=ApiHaltKind.RESPONSE_REJECTED, + reason=( + f"write {i} of {total} ({what}) -> {wresp.status}, outside its " + f"recorded response{done}; the save was not sent and nothing is retried" + ), + http_status=wresp.status, + request_summary=summary, + ) try: resp = self.transport.post_form( prepared.path, prepared.encoded(), referer=prepared.referer diff --git a/packages/flow/openadapt_flow/runtime/authorization.py b/packages/flow/openadapt_flow/runtime/authorization.py index 2cbd946232..807ead3e0b 100644 --- a/packages/flow/openadapt_flow/runtime/authorization.py +++ b/packages/flow/openadapt_flow/runtime/authorization.py @@ -363,6 +363,8 @@ def admitted_api_steps(workflow: Any) -> tuple[AdmittedApiStep, ...]: step's own, is listed. """ + from openadapt_flow.runtime.actuators.web_form import web_form_effects + rows: list[AdmittedApiStep] = [] for step in workflow.steps: binding = step.api_binding @@ -372,7 +374,7 @@ def admitted_api_steps(workflow: Any) -> tuple[AdmittedApiStep, ...]: or binding.web_form is None or not binding.admitted() or not binding.effects - or list(binding.effects) != list(step.effects) + or list(binding.effects) != web_form_effects(workflow.steps, step) ): continue rows.append( diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index e5df26bc3d..98439a05a9 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -2713,7 +2713,10 @@ def _web_form_span_at( the API step may run. None when no API step starts here. """ - from openadapt_flow.runtime.actuators.web_form import folded_checks_digest + from openadapt_flow.runtime.actuators.web_form import ( + folded_checks_digest, + web_form_effects, + ) first = workflow.steps[step_index].id for end_index in range(step_index, len(workflow.steps)): @@ -2760,7 +2763,7 @@ def _web_form_span_at( "step names no right-record contract (openadapt-flow api " "admit --identity)" ) - elif list(binding.effects) != list(save.effects): + elif list(binding.effects) != web_form_effects(workflow.steps, save): reason = ( "the save step's record check differs from the one admitted " "with the API step (openadapt-flow api admit)" diff --git a/packages/flow/tests/test_api_step_write_sequence.py b/packages/flow/tests/test_api_step_write_sequence.py new file mode 100644 index 0000000000..85e7d8ded0 --- /dev/null +++ b/packages/flow/tests/test_api_step_write_sequence.py @@ -0,0 +1,271 @@ +"""An API step's span as a declared sequence of writes, then the save. + +Other writes sent while the replaced steps ran used to refuse the API step. +Now each is declared with a role: ``ambient`` (the page sent it on its own, +from another document loaded before the form opened, to an endpoint the form +doesn't name; not sent by the API step) or ``step`` (a replaced step caused +it; sent in its recorded place before the save, read back by that step's +record check, and the run stops without retrying when it fails). Synthetic +values only. +""" + +from __future__ import annotations + +import json + +import pytest +import requests + +from openadapt_flow import network_capture as nc +from openadapt_flow.compiler.api_proposal import ( + ApiProposalError, + admit_api_step, + describe_api_step, + propose_api_step, +) +from openadapt_flow.runtime.actuators import ActuationStatus, ApiHaltKind +from openadapt_flow.runtime.actuators.web_form import ( + WebFormActuator, + _Resp, + web_form_effects, +) +from openadapt_flow.runtime.authorization import admitted_api_steps +from tests.test_api_tier_web_form import ( + RUN, + FakeTransport, + _effect, + _recording, + _workflow, +) + +SHELL = { + "t": -3.0, + "method": "GET", + "url": "http://app/main.php", + "status": 200, + "resource_type": "document", + "response_text": "", +} +BACKGROUND = { + "t": 3.4, + "method": "POST", + "url": "http://app/library/ajax/bg.php", + "status": None, + "failed": True, + "resource_type": "fetch", + "frame_url": "http://app/main.php", + "request_type": "multipart/form-data", + "body_omitted": "multipart", +} +DRAFT = { + "t": 4.2, + "method": "POST", + "url": "http://app/app/draft.php", + "status": 200, + "resource_type": "xhr", + "frame_url": "http://app/app/new.php", + "form": [["csrf_token_form", ""], ["form_lname", "Marsh"]], +} + + +def _rec(tmp_path, *extra): + rec = _recording(tmp_path) + rows = [json.loads(x) for x in (rec / nc.NETWORK_FILE).read_text().splitlines()] + body = sorted(rows[1:] + [dict(e) for e in extra], key=lambda r: r["t"]) + (rec / nc.NETWORK_FILE).write_text( + json.dumps({"schema": nc.SCHEMA, "exchanges": len(body)}) + + "\n" + + "".join(json.dumps(r) + "\n" for r in body) + ) + return rec + + +def _with_step_effect(wf, step_id="step_003"): + steps = [ + s.model_copy(update={"effects": [_effect()]}) if s.id == step_id else s + for s in wf.steps + ] + return wf.model_copy(update={"steps": steps}) + + +def test_a_request_the_page_sends_on_its_own_is_ambient_and_not_sent(tmp_path): + wf, step, reasons = propose_api_step(_workflow(), _rec(tmp_path, SHELL, BACKGROUND)) + assert reasons == [] and step is not None + (write,) = step.api_binding.web_form.writes + assert write.role == "ambient" and write.path == "/library/ajax/bg.php" + assert ( + write.sent_by == "/main.php" + and "loaded before the form opened" in write.evidence + ) + assert "not sent (the page's own request)" in describe_api_step(step) + wf, _ = admit_api_step(wf, admitted_by="reviewer") + t = FakeTransport() + out = WebFormActuator(t).actuate(wf.steps[-1].api_binding, RUN) + assert out.status is ActuationStatus.ACTUATED + assert [p for p, _ in t.posts] == ["/app/save.php"] + + +@pytest.mark.parametrize( + "edit, why", + [ + # Sent by the form's own document: a replaced step may have caused it. + ( + lambda b, s: (dict(b, frame_url="http://app/app/new.php"), s), + "can't rebuild", + ), + # The sending page wasn't open before the form: a step may have opened it. + (lambda b, s: (b, dict(s, t=0.5)), "can't rebuild"), + # Navigation or form posts are never ambient. + (lambda b, s: (dict(b, resource_type="document"), s), "can't rebuild"), + ], +) +def test_a_write_without_the_evidence_is_a_step_write(tmp_path, edit, why): + background, shell = edit(dict(BACKGROUND), dict(SHELL)) + _wf, step, reasons = propose_api_step( + _workflow(), _rec(tmp_path, shell, background) + ) + assert step is None and why in " ".join(reasons) + + +def test_an_endpoint_the_form_page_names_is_not_ambient(tmp_path): + rec = _rec(tmp_path, SHELL, BACKGROUND) + rows = [json.loads(x) for x in (rec / nc.NETWORK_FILE).read_text().splitlines()] + for row in rows[1:]: + if row.get("url") == "http://app/app/new.php": + row["response_text"] += "" + (rec / nc.NETWORK_FILE).write_text("".join(json.dumps(r) + "\n" for r in rows)) + _wf, step, reasons = propose_api_step(_workflow(), rec) + assert step is None and "bg.php" in " ".join(reasons) + + +def test_a_step_write_is_declared_in_order_and_needs_its_own_read_back(tmp_path): + wf, step, reasons = propose_api_step(_workflow(), _rec(tmp_path, DRAFT)) + assert reasons == [] and step is not None + (write,) = step.api_binding.web_form.writes + assert (write.role, write.from_step, write.expected_status) == ( + "step", + "step_003", + [200], + ) + assert [(f.name, f.source, f.param) for f in write.fields] == [ + ("csrf_token_form", "session", None), + ("form_lname", "param", "lname"), + ] + assert "sent before the save, in this order" in describe_api_step(step) + with pytest.raises(ApiProposalError, match="has no record check"): + admit_api_step(wf, admitted_by="reviewer") + wf = _with_step_effect(wf) + wf, _ = admit_api_step(wf, admitted_by="reviewer") + save = wf.steps[-1] + # The binding's record check reads back the save and the step's write. + assert save.api_binding.effects == web_form_effects(wf.steps, save) + assert len(save.api_binding.effects) == len(save.effects) + 1 + assert [row.step_id for row in admitted_api_steps(wf)] == [save.id] + + +def _admitted_sequence(tmp_path): + wf, _step, _ = propose_api_step(_workflow(), _rec(tmp_path, DRAFT)) + wf, _ = admit_api_step(_with_step_effect(wf), admitted_by="reviewer") + return wf.steps[-1].api_binding + + +class SequenceTransport(FakeTransport): + def __init__(self, write_status=200, raise_on_write=False): + super().__init__() + self.write_status, self.raise_on_write = write_status, raise_on_write + + def post_form(self, path, body, *, referer): + if path == "/app/draft.php": + if self.raise_on_write: + raise requests.exceptions.ConnectionError("reset after send") + self.posts.append((path, body)) + return _Resp(self.write_status, "", {}) + return super().post_form(path, body, referer=referer) + + +def test_the_sequence_sends_each_write_in_order_then_the_save(tmp_path): + t = SequenceTransport() + out = WebFormActuator(t).actuate(_admitted_sequence(tmp_path), RUN) + assert out.status is ActuationStatus.ACTUATED + assert [p for p, _ in t.posts] == ["/app/draft.php", "/app/save.php"] + assert "form_lname=Feld" in t.posts[0][1] and "live-session-value" in t.posts[0][1] + + +@pytest.mark.parametrize( + "transport, kind, said", + [ + ( + lambda: SequenceTransport(write_status=500), + ApiHaltKind.RESPONSE_REJECTED, + "the save was not sent", + ), + ( + lambda: SequenceTransport(raise_on_write=True), + ApiHaltKind.DELIVERY_UNCERTAIN, + "may have written", + ), + ], +) +def test_a_failed_write_stops_the_sequence_and_nothing_is_retried( + tmp_path, transport, kind, said +): + t = transport() + out = WebFormActuator(t).actuate(_admitted_sequence(tmp_path), RUN) + assert ( + out.status is ActuationStatus.HALT + and out.halt_kind is kind + and said in out.reason + ) + assert "/app/save.php" not in [p for p, _ in t.posts] + assert len([p for p, _ in t.posts if p == "/app/draft.php"]) <= 1 + + +def test_the_approved_digest_covers_every_write_and_keeps_the_old_one_without(tmp_path): + seq = WebFormActuator(SequenceTransport()).prepare( + _admitted_sequence(tmp_path), RUN + ) + assert len(seq.writes) == 1 + bare = seq.__class__( + method=seq.method, + path=seq.path, + body=seq.body, + referer=seq.referer, + session_fields=seq.session_fields, + ) + assert bare.payload_sha256() != seq.payload_sha256() + # Without writes the digest is the one the approval gate always used. + import hashlib + + canonical = json.dumps( + { + "method": bare.method.upper(), + "path": bare.path, + "content_type": "application/x-www-form-urlencoded", + "body": bare.encoded(), + "referer": bare.referer, + }, + sort_keys=True, + separators=(",", ":"), + ) + assert ( + bare.payload_sha256() + == "sha256:" + hashlib.sha256(canonical.encode()).hexdigest() + ) + + +@pytest.mark.parametrize( + "write, why", + [ + (dict(DRAFT, t=0.5), "before any replaced step"), + (dict(DRAFT, url="http://other/app/draft.php"), "another origin"), + (dict(DRAFT, url="http://app/app/draft.php?id=7"), "values in its URL"), + (dict(DRAFT, status=500), "wasn't accepted"), + (dict(DRAFT, form=[["form_lname", "Someone else"]]), "is not a typed value"), + (dict(DRAFT, form=None, body_omitted="multipart"), "can't rebuild"), + ], +) +def test_a_step_write_the_api_step_cant_repeat_refuses_the_proposal( + tmp_path, write, why +): + _wf, step, reasons = propose_api_step(_workflow(), _rec(tmp_path, write)) + assert step is None and why in " ".join(reasons) From fc54e47292130d1a9a559a2de43df462255f96a6 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sat, 24 Oct 2026 23:53:24 -0400 Subject: [PATCH 04/15] benchmark: OpenEMR referral harness for the admissible API step Interleaved browser and API arms on the API-tier fixture, the eight referrals plus a future date of birth (the form's pastDate rule must refuse it) and a lower-case first name, a per-item timeout, the test setup's own SQL read-back, and the whole saved patient_data row per run so the two arms' saves can be compared column by column. Co-Authored-By: Claude Opus 5.5 --- .../benchmark/openemr_api_tier/admissible.py | 321 ++++++++++++++++++ 1 file changed, 321 insertions(+) create mode 100644 packages/flow/benchmark/openemr_api_tier/admissible.py diff --git a/packages/flow/benchmark/openemr_api_tier/admissible.py b/packages/flow/benchmark/openemr_api_tier/admissible.py new file mode 100644 index 0000000000..183fcb4875 --- /dev/null +++ b/packages/flow/benchmark/openemr_api_tier/admissible.py @@ -0,0 +1,321 @@ +"""The OpenEMR referral's API step, admissible: page script run, background write declared. + +Until now the compiler refused an API step for this form for two reasons: the +form validates with script (``submitme(...)`` on submit, a custom validate.js +``pastDate`` rule on the date of birth, ``onchange`` on every field) and the +tab shell posts ``execute_background_services.php`` while the form is open. +With the API step running the page's own script before each save +(runtime/actuators/form_eval.py) and declaring the background post as the +page's own request (``web_form.writes``, role ``ambient``), the same +recording now gets an API step. This harness measures it on the local +OpenEMR test copy, against the same program on the GUI. + +Label every number exactly as: "plain replayer, no approval gate, no durable +run, no run gate; API step admitted by the harness (benchmark-fixture)". + +- Arms: ``browser`` (``api_steps=False``) and ``api`` (``api_steps=True``), + interleaved by block; each block resets the fixture and seeds the chart r6 + must not duplicate. +- Items: the eight referrals of ``common.REFERRALS`` (r6 must create no + chart), plus ``x1-future-dob`` (the form's ``pastDate`` rule must refuse + it: no chart) and ``x2-lowercase`` (a first name typed in lower case; + both arms must save the same thing). +- Read-back: the test setup's own SQL check (``common.record_check``), never + the arm under test, plus the whole new ``patient_data`` row, so the two + arms' saves can be compared column by column. +- Time: ``wall_s`` from a fresh browser context through sign-in, the entry and + the read-back; items per hour is 3600 over the mean ``wall_s`` of one + sequential worker. Each item has a hard timeout (``--item-timeout``). + + python -m benchmark.openemr_api_tier.admissible compile --recording --out + python -m benchmark.openemr_api_tier.admissible run --bundle --out [--blocks 4] + python -m benchmark.openemr_api_tier.admissible summarize --out +""" + +from __future__ import annotations + +import argparse +import csv +import json +import shutil +import signal +import statistics +import subprocess +import sys +import time +from pathlib import Path +from typing import Any + +from benchmark.openemr_api_tier.common import ( + _MA, + REFERRALS, + Referral, + load_demo, + record_check, + reset_and_seed, + spec_for, + sql_patients, +) +from benchmark.openemr_api_tier.feature_e2e import NO_MATCH_TEXT, _load1, wait_quiet +from benchmark.openemr_api_tier.spike_fixture import SpikeFixture + +LABEL = ( + "plain replayer, no approval gate, no durable run, no run gate; " + "API step admitted by the harness (benchmark-fixture)" +) +#: A date of birth in the future: the form's own pastDate rule refuses it. +FUTURE_DOB = Referral( + "x1-future-dob", + dict(title="Ms.", fname="Wren", lname="Halvorsen", dob="2031-02-03", sex="Female", + street="5 ORCHARD LANE", phone_home="555-0171", email="wren.halvorsen@example.invalid", **_MA), + mismatch=True, +) +#: A first name typed in lower case: whatever the page does with it, both +#: arms must save the same chart. +LOWERCASE = Referral( + "x2-lowercase", + dict(title="Mr.", fname="otto", lname="Brandvold", dob="1968-08-21", sex="Male", + street="21 KILN STREET", phone_home="555-0176", email="otto.brandvold@example.invalid", **_MA), +) +ITEMS = [*REFERRALS, FUTURE_DOB, LOWERCASE] +#: Columns that differ between any two saves of the same referral. +VOLATILE = {"id", "pid", "pubpid", "uuid", "date", "regdate", "created_by", "updated_by", + "last_updated", "created_at", "updated_at"} + +COLS = ["arm", "block", "pos", "ref", "expect_chart", "correct", "new_charts", "readback", + "engine_success", "outcome", "transaction_outcome", "save_actuation", "api_fallback", "error", "wall_s", "setup_s", "engine_s", "readback_s", + "load1", "gate_waited_s", "start_local", "engine_sha"] + + +def cmd_compile(a: argparse.Namespace) -> None: + from openadapt_flow.compiler.api_proposal import ( + admit_api_step, + describe_api_step, + propose_api_step, + ) + from openadapt_flow.ir import Postcondition, PostconditionKind, Workflow + + demo = load_demo() + rec, bundle = Path(a.recording), Path(a.out) + demo.compile_recording(rec, bundle) + first = json.loads((bundle / "api_proposal.json").read_text()) + wf = Workflow.load(bundle) + # The referral's business rule (as in feature_e2e): confirm a new chart + # only when OpenEMR's duplicate check finds no existing patient. + dialog = [s for s in wf.steps if s.anchor is not None and s.anchor.structural is not None + and s.anchor.structural.selector == "#create"] + assert len(dialog) == 1 + dialog[0].expect = list(dialog[0].expect) + [ + Postcondition(kind=PostconditionKind.TEXT_PRESENT, text=NO_MATCH_TEXT, timeout_s=10.0)] + wf, step, reasons = propose_api_step(wf, rec, replace=True) + if step is None: + raise SystemExit(f"no API step: {reasons}") + wf, _ = admit_api_step(wf, admitted_by="benchmark-fixture") + wf.save(bundle) + save = next(s for s in wf.steps if s.id == step.id) + out = {"first_compile": first, "proposed": step.id, "label": LABEL, + "client_scripts": save.api_binding.web_form.client_scripts, + "writes": [w.model_dump(mode="json") for w in save.api_binding.web_form.writes], + "describe": describe_api_step(save)} + (bundle / "admissible-compile.json").write_text(json.dumps(out, indent=1)) + print(out["describe"]) + + +def _columns(f: SpikeFixture) -> list[str]: + out = f._db("SELECT column_name FROM information_schema.columns WHERE table_schema='openemr' " + "AND table_name='patient_data' ORDER BY ordinal_position").decode() + return [c for c in out.split() if c] + + +def _rows(f: SpikeFixture, cols: list[str], ids: list[str]) -> list[dict[str, str]]: + if not ids: + return [] + sel = ",".join(f"`{c}`" for c in cols) + out = f._db(f"SELECT {sel} FROM openemr.patient_data WHERE id IN ({','.join(ids)}) ORDER BY id").decode() + return [{k: ("" if v == "NULL" else v) for k, v in zip(cols, line.split("\t"))} + for line in out.splitlines()] + + +class _Timeout(Exception): + pass + + +def _alarm(_signum: int, _frame: Any) -> None: + raise _Timeout("item timeout") + + +def run_one(env: dict[str, Any], arm: str, ref: Referral, bundle: Path, out: Path, timeout_s: int) -> dict[str, Any]: + from openadapt_flow.benchmark.openemr_local import OpenEMRPatientOracle + from openadapt_flow.ir import Workflow + from openadapt_flow.runtime import Replayer + + demo, f, browser, ocr = env["demo"], env["f"], env["browser"], env["ocr"] + spec = spec_for(ref) + out.mkdir(parents=True) + before_ids = {r["id"] for r in sql_patients(f)} + oracle = OpenEMRPatientOracle(f.api_base_url, f.token_session("oracle"), spec) + res: dict[str, Any] = {"arm": arm, "ref": ref.key, "expect_chart": not ref.mismatch, "error": None} + t0 = time.time() + ctx = browser.new_context(viewport={"width": 1280, "height": 800}, device_scale_factor=1) + signal.signal(signal.SIGALRM, _alarm) + signal.alarm(timeout_s) + t_setup = t_task = None + try: + page = ctx.new_page() + page.goto(f.ui_base_url) + backend = demo._BasePlaywrightBackend(page) + demo._preauthenticate_browser(backend, f) + t_setup = time.time() + ocr.clear_ocr_memo() + replayer = Replayer(backend, effect_verifier=oracle.verifier, api_steps=(arm == "api")) + rep = replayer.run(Workflow.load(bundle), params=spec.params(), bundle_dir=bundle, + run_dir=out / "engine") + t_task = time.time() + raw = rep.model_dump(mode="json") + last = raw["results"][-1] if raw["results"] else {} + res.update(engine_success=raw.get("success"), outcome=str(raw.get("execution_outcome")), + transaction_outcome=raw.get("transaction_outcome"), save_actuation=last.get("actuation"), + api_fallback=next((r["api_fallback"] for r in raw["results"] if r.get("api_fallback")), None), + step_errors=[(r["step_id"], (r.get("error") or "")[:300]) for r in raw["results"] if r.get("error")]) + except _Timeout: + res["error"] = f"timeout after {timeout_s}s" + except Exception as exc: # noqa: BLE001 + res["error"] = f"{type(exc).__name__}: {exc}"[:500] + finally: + signal.alarm(0) + t_setup = t_setup or time.time() + t_task = t_task or time.time() + time.sleep(demo.ORACLE_SETTLE_S) + res.update(record_check(f, ref, before_ids)) + new_ids = [r["id"] for r in sql_patients(f) if r["id"] not in before_ids] + res["new_rows"] = _rows(f, env["cols"], new_ids) + t_end = time.time() + try: + ctx.close() + except Exception: # noqa: BLE001 + pass + res.update(wall_s=round(t_end - t0, 3), setup_s=round(t_setup - t0, 3), engine_s=round(t_task - t_setup, 3), + readback_s=round(t_end - t_task, 3)) + (out / "result.json").write_text(json.dumps(res, indent=1, default=str)) + shutil.rmtree(out / "engine" / "steps", ignore_errors=True) + return res + + +def cmd_run(a: argparse.Namespace) -> None: + from playwright.sync_api import sync_playwright + + import openadapt_flow.vision.ocr # noqa: F401 + + ocr = sys.modules["openadapt_flow.vision.ocr"] + out, bundle = Path(a.out), Path(a.bundle) + out.mkdir(parents=True, exist_ok=True) + sha = subprocess.run(["git", "rev-parse", "HEAD"], capture_output=True, text=True).stdout.strip()[:9] + demo, f = load_demo(), SpikeFixture() + pw = sync_playwright().start() + browser = pw.chromium.launch(headless=True) + ocr._get_engine() + ctx = browser.new_context(viewport={"width": 1280, "height": 800}) + page = ctx.new_page() + page.goto(f.ui_base_url) + ocr.ocr(page.screenshot(type="png")) + ctx.close() + env = {"demo": demo, "f": f, "browser": browser, "ocr": ocr, "cols": _columns(f)} + (out / "run.json").write_text(json.dumps({"label": LABEL, "engine_sha": sha, "bundle": str(bundle), + "blocks": a.blocks, "items": [r.key for r in ITEMS]}, indent=1)) + csv_path = out / "runs.csv" + new = not csv_path.exists() + try: + for b in range(a.blocks): + for arm in (["browser", "api"] if b % 2 == 0 else ["api", "browser"]): + meta = reset_and_seed(f) | {"arm": arm, "block": b + 1, "started": time.strftime("%Y-%m-%d %H:%M:%S")} + (out / f"block-{b + 1:02d}-{arm}.json").write_text(json.dumps(meta, indent=1)) + for pos, ref in enumerate(ITEMS): + gate = wait_quiet() + start = time.strftime("%H:%M:%S") + res = run_one(env, arm, ref, bundle, out / f"b{b + 1:02d}-{arm}-{pos + 1:02d}-{ref.key}", + a.item_timeout) + row = {k: res.get(k) for k in COLS} | {"block": b + 1, "pos": pos + 1, "load1": _load1(), + "gate_waited_s": gate["gate_waited_s"], + "start_local": start, "engine_sha": sha} + with open(csv_path, "a", newline="") as fh: + w = csv.DictWriter(fh, fieldnames=COLS) + if new: + w.writeheader() + new = False + w.writerow(row) + print(json.dumps({k: row[k] for k in ("arm", "block", "ref", "correct", "readback", + "save_actuation", "api_fallback", "wall_s", + "error")}), flush=True) + finally: + browser.close() + pw.stop() + + +def _same_save(a_rows: list[dict[str, str]], b_rows: list[dict[str, str]]) -> list[str]: + if len(a_rows) != len(b_rows): + return [f"charts: {len(a_rows)} vs {len(b_rows)}"] + diffs = [] + for x, y in zip(a_rows, b_rows): + for k in sorted(set(x) | set(y)): + if k not in VOLATILE and x.get(k) != y.get(k): + diffs.append(f"{k}: {x.get(k)!r} vs {y.get(k)!r}") + return diffs + + +def cmd_summarize(a: argparse.Namespace) -> None: + out = Path(a.out) + results = [json.loads(p.read_text()) for p in sorted(out.glob("b*/result.json"))] + summary: dict[str, Any] = {"label": LABEL, "arms": {}} + for arm in ("browser", "api"): + rs = [r for r in results if r["arm"] == arm] + if not rs: + continue + valid = [r for r in rs if r["expect_chart"]] + walls = [r["wall_s"] for r in rs] + summary["arms"][arm] = { + "items": len(rs), + "correct": sum(bool(r["correct"]) for r in rs), + "wrong_writes": sum(r["new_charts"] > 0 and not r["correct"] for r in rs), + "false_stops": sum(not r["correct"] and r["new_charts"] == 0 for r in valid), + "api_saves": sum(r.get("save_actuation") == "api" for r in rs), + "fallbacks": sorted({r["api_fallback"] for r in rs if r.get("api_fallback") + and r["api_fallback"] != "API steps are off for this run"}), + "errors": [r["error"] for r in rs if r.get("error")], + "median_wall_s": round(statistics.median(walls), 2), + "mean_wall_s": round(statistics.mean(walls), 2), + "items_per_hour": round(3600 / statistics.mean(walls), 1), + "median_engine_s": round(statistics.median(r["engine_s"] for r in rs), 2), + } + # Equivalence: the API arm's chart against the browser arm's, per referral. + by = {} + for r in results: + by.setdefault((r["ref"], r["arm"]), []).append(r) + eq = {} + for ref in sorted({r["ref"] for r in results}): + pairs = list(zip(by.get((ref, "browser"), []), by.get((ref, "api"), []))) + eq[ref] = [_same_save(x.get("new_rows") or [], y.get("new_rows") or []) for x, y in pairs] + summary["equivalence"] = {ref: {"pairs": len(v), "identical": sum(not d for d in v), + "diffs": sorted({x for d in v for x in d})[:10]} for ref, v in eq.items()} + (out / "summary.json").write_text(json.dumps(summary, indent=1)) + print(json.dumps(summary, indent=1)) + + +def main() -> None: + ap = argparse.ArgumentParser() + sub = ap.add_subparsers(dest="cmd", required=True) + c = sub.add_parser("compile") + c.add_argument("--recording", required=True) + c.add_argument("--out", required=True) + r = sub.add_parser("run") + r.add_argument("--bundle", required=True) + r.add_argument("--out", required=True) + r.add_argument("--blocks", type=int, default=4) + r.add_argument("--item-timeout", type=int, default=240) + s = sub.add_parser("summarize") + s.add_argument("--out", required=True) + a = ap.parse_args() + {"compile": cmd_compile, "run": cmd_run, "summarize": cmd_summarize}[a.cmd](a) + + +if __name__ == "__main__": + main() From 6b5d1c0b68f9afd9f2728b5e74397491cf0694a9 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 00:08:18 -0400 Subject: [PATCH 05/15] docs: refresh llms-full.txt Co-Authored-By: Claude Opus 5.5 --- llms-full.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/llms-full.txt b/llms-full.txt index 3e28afeddb..e8faf5873b 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -597,6 +597,7 @@ improvising or retrying when something is off. | Capture a recording's network traffic and compile a clean save into an API step, with the same record check | On main ([#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269)) | Synthetic patients on a local OpenEMR test copy | network capture, `ApiBinding` | | Fall back from the API step to page structure when the API is unavailable, before anything is sent | On main ([#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269)) | Synthetic | [#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269) | | Run a form's own script before an API save instead of refusing the form: field handlers, `onsubmit`, submit listeners and custom validate.js rules (OpenEMR's `pastDate`) run on the run's values in a scratch copy of the session with every write blocked, and the save is sent only when the page lets the submit through and would send the same body | On main | Synthetic (headless Chromium against a local test page) | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#what-happens-at-run-time), `runtime/actuators/form_eval.py` | +| Declare the other writes in an API step's span as an ordered sequence: the page's own background requests (another document's script, loaded before the form, to an endpoint the form doesn't name) are listed and not sent; a write a step caused is sent in its recorded place before the save, read back by that step's record check, and a failure stops the run with nothing after it sent and nothing retried | On main | Synthetic | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#other-writes-in-the-span) | | Approve an API step's exact request under per-item approval: one digest binds the request bytes, the record and the record check, and a changed request is not sent | On main ([#1355](https://github.com/OpenAdaptAI/OpenAdapt/pull/1355)) | Synthetic | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#under-an-approval-mode) | | Run an API step in a durable run: checkpoint the span after the save is sent, never resend or redo a save that may have landed, and reconcile it by reading the record back | On main ([#1358](https://github.com/OpenAdaptAI/OpenAdapt/pull/1358)) | Synthetic | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#in-a-durable-run) | | Run an admitted API step under a governed run authorization and the Standard or Regulated profile: the authorization names the API step by digest, the person who admits it names the right-record parameter, and the outcome reads `VERIFIED` | On main ([#1361](https://github.com/OpenAdaptAI/OpenAdapt/pull/1361)) | Synthetic | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#under-governance) | From b3aa3b1efc09b58f4fed8303a52b045e032eb7ed Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 01:26:52 -0400 Subject: [PATCH 06/15] flow: review fixes for running a form's script before an API save - Compare the body the page submits, taken when it submits: a last submit listener, the form's own submit(), or the browser's POST the route blocks. - Click the control the person clicked (recorded as submitter), and require the page to submit or make a request that click made in the recording; with no recorded control, refuse a form whose buttons have click handlers. - Run the page's script whenever the page has any: client_scripts now holds every script URL, inline script, and each handler with a digest of its source (form, sent fields, buttons). - In the scratch context, block every other origin and every request once the page has loaded; service workers are blocked; no scratch tab in the live context. - API_TIER.md no longer claims the two paths are proven to agree. Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/API_TIER.md | 66 ++-- .../openadapt_flow/compiler/api_proposal.py | 55 +++ packages/flow/openadapt_flow/ir.py | 7 + .../runtime/actuators/form_eval.py | 351 +++++++++++------- .../runtime/actuators/web_form.py | 89 ++++- .../tests/test_api_step_native_validation.py | 6 +- .../flow/tests/test_api_step_page_script.py | 105 +++++- packages/flow/tests/test_api_tier_web_form.py | 15 + 8 files changed, 515 insertions(+), 179 deletions(-) diff --git a/packages/flow/docs/API_TIER.md b/packages/flow/docs/API_TIER.md index 2eca30b092..33bd8654b0 100644 --- a/packages/flow/docs/API_TIER.md +++ b/packages/flow/docs/API_TIER.md @@ -127,30 +127,41 @@ Before it sends the save, the API step: 1. reads the form page again in this session; 2. refuses if the form's fields or its client-side validation rules (for example, OpenEMR's `constraints` object) differ from the recording; -3. refuses if the form's script isn't the script recorded with the - proposal (`client_scripts`: an `on*` handler on the form, such as - `onsubmit`, or on a field it sends, such as `onchange`; a page script - listening for the submit; a validate.js rule kind other than `presence`, - `length`, `format`, `numericality` and `email`, such as OpenEMR's - `pastDate`); +3. refuses if the page's script isn't the script recorded with the + proposal (`client_scripts`): every script the page loads, by URL; + whether it has inline script; each `on*` handler of the form, of a field + it sends and of the form's buttons, with a digest of its source; a page + script listening for submit; and validate.js rule kinds other than + `presence`, `length`, `format`, `numericality` and `email`, such as + OpenEMR's `pastDate`; 4. builds the body and checks it the way the browser would before it submits: native HTML constraints (`required`, `minlength`, `maxlength`, `pattern`, `min`, `max`, and the `email`, `url`, `number` and `date` input types) and the page's validate.js rules; -5. when the form has script, **runs it** (`runtime/actuators/form_eval.py`): - it opens the form page in a scratch copy of the live session (a separate - browser context with the session's cookies), blocks every request that - isn't a GET and every script-sent request once the page has loaded, puts - the run's typed and picked values into the form with the same `input`, - `change` and `blur` events a browser fires, reads the body the browser - would send, runs the browser's own constraint checks, submits the form - through its own handlers with a final listener that records whether - they let the submit through and then stops it, and runs the page's - validate.js `constraints` when it declares them. The save goes ahead only - when the page let the submit through, raised no error, showed no dialog, - tried to send nothing, and would send exactly the body the API step - built (session values aside). Anything else is a reason, and the GUI - steps run; +5. when the page has any script, **runs it** (`runtime/actuators/form_eval.py`): + - it opens the form page in a scratch copy of the live session: a + separate browser context with the session's cookies and no service + workers. While the page loads, only same-origin GET requests go + through; once it has loaded, every request is blocked, so nothing the + page does there can write or reach the session; + - it puts the run's typed and picked values into the form with the same + `input`, `change` and `blur` events a browser fires; + - it starts the submit as the person did: it clicks the control the + person clicked (`submitter`, from the recording). When the recording + names none it submits the form, and a form whose buttons have click + handlers is refused; + - the page must then submit the form, or make a request the person's + click made in the recording (`submitter_requests`; on OpenEMR, the + duplicate search). The body compared is the one the page submits, + taken at the moment it submits (a last `submit` listener, the form's + own `submit()`, or the browser's POST, which the route blocks); when + the page goes on to a recorded request instead, it is the form as the + page left it. Declared validate.js `constraints` run too; + - the save goes ahead only when the page submitted or went on as + recorded, raised no error, showed no dialog, tried to send nothing + else, and the body equals the one the API step built (session values + aside). Any difference, and anything else, is a reason, and the GUI + steps run; 6. runs the pre-checks. Each of these refusals means the save wasn't sent, so the GUI steps run. @@ -378,13 +389,14 @@ recording. form's own script for the rest. A `pattern` is evaluated with Python's regular expressions, which differ from the browser's in rare syntax; one that doesn't compile keeps the save on the GUI. -- Running the form's script covers what the script does on `input`, - `change`, `blur` and `submit`. Script that validates on a click handler - of the submit button only, or asynchronously after the submit event, - isn't seen; qualification on test cases, both paths, is what shows the - two paths agree. A form whose script needs the server while it validates - (a script-sent request after load) is blocked and stays on the GUI. Each - save with script costs one extra load of the form page. +- Running the form's script covers what it does on `input`, `change`, + `blur`, the recorded click and `submit`, within a short settle time. + Script that validates later (after a timer or a server reply) isn't seen: + a form whose script needs the server while it validates is blocked and + stays on the GUI. Running it is evidence for one run's values, not a + proof that the two paths always agree; admitting the API step stays a + person's decision. Each save with script costs one extra load of the + form page. - Multi-step server state (wizards, uploads, locks held between pages) needs the whole request sequence, and isn't proposed. diff --git a/packages/flow/openadapt_flow/compiler/api_proposal.py b/packages/flow/openadapt_flow/compiler/api_proposal.py index d85e5694e7..7a8a4f3444 100644 --- a/packages/flow/openadapt_flow/compiler/api_proposal.py +++ b/packages/flow/openadapt_flow/compiler/api_proposal.py @@ -246,6 +246,56 @@ def _success_pattern(line: dict[str, Any]) -> Optional[str]: return r".*?".join(pieces) +def _in_page(selector: str, html: str) -> bool: + """Whether a recorded ``#id`` or ``[name="..."]`` selector names an element of ``html``.""" + + m = re.fullmatch(r"#([A-Za-z][\w:.-]*)", selector) + if m: + return ( + re.search(r"""\bid\s*=\s*["']%s["']""" % re.escape(m.group(1)), html) + is not None + ) + m = re.fullmatch(r"""\[name=["']?([^"'\]]+)["']?\]""", selector) + if m: + return ( + re.search(r"""\bname\s*=\s*["']%s["']""" % re.escape(m.group(1)), html) + is not None + ) + return False + + +def _submitter( + folded: list[Step], + events: list[dict[str, Any]], + lines: list[dict[str, Any]], + form_html: str, +) -> tuple[Optional[str], list[str]]: + """The last folded click on a control of the form page, and what it requested.""" + + for step in reversed(folded): + if step.action not in (ActionKind.CLICK, ActionKind.DOUBLE_CLICK): + continue + i = _event_index(step) + selector = None + if step.anchor is not None and step.anchor.structural is not None: + selector = step.anchor.structural.selector + elif i is not None and i < len(events): + selector = (events[i].get("structural") or {}).get("selector") + if not selector or not _in_page(selector, form_html) or i is None: + continue + lo = float(events[i]["t"]) if i < len(events) else float("-inf") + hi = float(events[i + 1]["t"]) if i + 1 < len(events) else float("inf") + made = sorted( + { + f"{ln['method']} {_path(ln['url'])}" + for ln in lines + if lo <= float(ln["t"]) < hi + } + ) + return selector, made + return None, [] + + def propose_api_step( workflow: Workflow, recording_dir: Path | str, *, replace: bool = False ) -> tuple[Workflow, Optional[Step], list[str]]: @@ -595,12 +645,17 @@ def carried(line: dict[str, Any]) -> int: f"{', '.join(others)}; one request can't stand in for them" ], ) + submitter, submitter_requests = _submitter( + folded, events, lines, form_line["response_text"] + ) spec = WebFormSpec( form_path=form_path, fields=fields, controls=sorted(controls), client_rules=client_rules(form_line["response_text"]), client_scripts=client_scripts, + submitter=submitter, + submitter_requests=submitter_requests, prechecks=prechecks, success_pattern=_success_pattern(post), replaces_steps=[s.id for s in folded], diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index 139b33a069..fa1ec74f9b 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -637,6 +637,13 @@ class WebFormSpec(BaseModel): #: it can't run the script, the GUI steps run. The live form must list the #: same script, or the API step doesn't run. client_scripts: list[str] = Field(default_factory=list) + #: The control in the form the person clicked to start the save (a CSS + #: selector from the recording), and the requests that click made + #: (``METHOD /path``). Running the page's script clicks it, as the person + #: did, and the page must then submit the form or make one of those + #: requests. + submitter: Optional[str] = None + submitter_requests: list[str] = Field(default_factory=list) prechecks: list[WebFormPrecheck] = Field(default_factory=list) #: Regular expression the save response must match to count as accepted. success_pattern: Optional[str] = None diff --git a/packages/flow/openadapt_flow/runtime/actuators/form_eval.py b/packages/flow/openadapt_flow/runtime/actuators/form_eval.py index a73c2f67fd..f9618e4e1e 100644 --- a/packages/flow/openadapt_flow/runtime/actuators/form_eval.py +++ b/packages/flow/openadapt_flow/runtime/actuators/form_eval.py @@ -2,29 +2,37 @@ The ``web_form`` API step sends one request in place of the clicks that fill and submit a form. Most forms run script on the way: field ``onchange`` -handlers, an ``onsubmit`` validator, a page listener for ``submit``, custom -validate.js rules such as OpenEMR's ``pastDate``. The API step can't read -what that script does, so it runs it, before each save: +handlers, an ``onsubmit`` validator, a click handler on the save button, a +page listener for ``submit``, custom validate.js rules such as OpenEMR's +``pastDate``. The API step can't read what that script does, so it runs it, +before each save: 1. open the form page in a scratch copy of the live session (a separate - browser context with the session's cookies, or a scratch tab when the - browser can't make one); -2. block every request that isn't a GET for the whole visit, and every - script-sent request once the page has loaded, so nothing the page does - here can write; + browser context with the session's cookies, service workers blocked); +2. while the page loads, let only same-origin GET requests through (no + other origin at all); once it has loaded, block every request, so + nothing the page does here can write or reach the session; 3. put the run's values into the fields the person typed or picked, the way a browser does (``input`` and ``change`` events, then ``blur``); -4. read back the body the browser would send (``FormData``), check the - browser's own constraints, and submit the form with a listener that - records whether the page's own handlers let the submit through and then - stops it; where the page declares validate.js ``constraints``, run them - too. +4. start the submit the way the person did: click the recorded control + (``submitter``), or, when the recording names none, submit the form; a + form whose buttons have click handlers and no recorded control is + refused; +5. see what the page does: it submits the form (the submit event reaches a + last listener, ``form.submit()`` is called, or the browser's own POST to + the save reaches the blocked route), or it makes a request the person's + click made in the recording (``submitter_requests``, such as OpenEMR's + duplicate search). The body compared is the one the page submits, taken + at the moment it submits; a request from the recording counts with the + form as the page left it. Where the page declares validate.js + ``constraints``, they run too. -The result is ``valid`` only when nothing failed, nothing tried to write, -the page raised no error, asked no question, and its handlers let the submit -through. Anything else is a reason for the GUI steps to run instead. -Comparing the body the page would send with the one the API step built is -the caller's job (:meth:`WebFormActuator.prepare`). Model-free. +The result is ``valid`` only when the page submitted or went on as +recorded, raised no error, showed no dialog, and tried nothing else. +Anything else is a reason for the GUI steps to run instead. Comparing the +body with the one the API step built is the caller's job +(:meth:`WebFormActuator.prepare`); any difference keeps the save on the GUI. +Model-free. """ from __future__ import annotations @@ -34,20 +42,33 @@ from dataclasses import dataclass, field from typing import Any, Optional -#: Before any page script: a form's own ``submit()`` (which skips the submit -#: event) is counted instead of navigating, and window.open opens nothing. +#: Before any page script: what a form's own ``submit()`` would send is +#: recorded instead of navigating, window.open opens nothing, and every +#: click listener's target is noted. _INSTRUMENT_JS = """ (() => { - const state = { programmaticSubmits: 0, opened: [] }; + const pairs = (f, s) => Array.from(s ? new FormData(f, s) : new FormData(f)).map( + ([k, v]) => [k, typeof v === 'string' ? v : '']); + const state = { programmatic: [], opened: [], clickTargets: [], pairs }; Object.defineProperty(window, '__openadaptFormEval', { value: state }); - HTMLFormElement.prototype.submit = function () { state.programmaticSubmits += 1; }; + HTMLFormElement.prototype.submit = function () { + try { state.programmatic.push(pairs(this)); } catch (e) { state.programmatic.push(null); } + }; window.open = function (url) { state.opened.push(String(url || '')); return null; }; + const add = EventTarget.prototype.addEventListener; + EventTarget.prototype.addEventListener = function (type, fn, opts) { + if (['click', 'mousedown', 'mouseup', 'pointerdown', 'pointerup'].includes(type)) { + state.clickTargets.push(this); + } + return add.call(this, type, fn, opts); + }; })(); """ _EVALUATE_JS = """ -async ({ actionPath, leaf, fields, settleMs }) => { - const state = window.__openadaptFormEval || { programmaticSubmits: 0, opened: [] }; +async ({ actionPath, leaf, fields, settleMs, submitter }) => { + const state = window.__openadaptFormEval; + if (!state) return { unevaluable: 'the scratch page could not be instrumented' }; const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); const form = Array.from(document.forms).find((f) => { const p = new URL(f.getAttribute('action') || location.href, location.href).pathname; @@ -74,36 +95,48 @@ await sleep(0); } await sleep(settleMs); - const serialized = Array.from(new FormData(form)).map( - ([k, v]) => [k, typeof v === 'string' ? v : '']); const invalid = form.noValidate ? [] : Array.from(form.elements) .filter((e) => e.willValidate && !e.checkValidity()) .map((e) => `${e.name || e.id}: ${e.validationMessage}`); - let reached = false, prevented = null, threw = null; + const buttons = Array.from(form.querySelectorAll( + 'button, input[type=submit], input[type=button], input[type=image]')); + const listened = new Set(state.clickTargets); + const buttonScript = buttons.some((b) => b.hasAttribute('onclick') || listened.has(b)); + let reached = false, prevented = null, submitted = null, threw = null, clicked = false; const last = (e) => { - if (e.target === form) { reached = true; prevented = e.defaultPrevented; e.preventDefault(); } + if (e.target !== form) return; + reached = true; + prevented = e.defaultPrevented; + try { submitted = state.pairs(form, e.submitter || null); } catch (x) { submitted = state.pairs(form); } + e.preventDefault(); }; - const before = state.programmaticSubmits; + const before = state.programmatic.length; window.addEventListener('submit', last); try { - if (!invalid.length) { - if (typeof form.requestSubmit === 'function') form.requestSubmit(); - else form.dispatchEvent(new Event('submit', { bubbles: true, cancelable: true })); + if (submitter) { + const el = document.querySelector(submitter); + if (!el) return { unevaluable: `the scratch page has no ${submitter} to click` }; + clicked = true; + el.click(); + } else if (buttonScript) { + return { unevaluable: 'a button of the form runs script on click and the recording names no control to click' }; + } else if (!invalid.length) { + form.requestSubmit(); } } catch (e) { threw = String(e && e.message || e); } - window.removeEventListener('submit', last); await sleep(settleMs); - let rules = null, ruleKinds = false; + window.removeEventListener('submit', last); + let rules = null; if (typeof window.validate === 'function' && window.constraints && typeof window.constraints === 'object') { - ruleKinds = true; try { const values = typeof window.validate.collectFormValues === 'function' ? window.validate.collectFormValues(form) : Object.fromEntries(new FormData(form)); rules = window.validate(values, window.constraints) || null; } catch (e) { threw = threw || `validate.js: ${String(e && e.message || e)}`; } } - return { serialized, invalid, reached, prevented, threw, rules, ruleKinds, - programmaticSubmits: state.programmaticSubmits - before, opened: state.opened.slice() }; + return { invalid, reached, prevented, submitted, threw, rules, clicked, + programmatic: state.programmatic.slice(before), after: state.pairs(form), + opened: state.opened.slice() }; } """ @@ -116,8 +149,12 @@ class FormEvaluation: #: ``unevaluable`` (the script couldn't be run faithfully). status: str reason: str = "" - #: The body the browser would send, ``(name, value)`` in order. + #: The body the page submits, ``(name, value)`` in order: taken when it + #: submits, or the form as the page left it when it went on to a request + #: from the recording. serialized: Optional[list[tuple[str, str]]] = None + #: How the page went on: ``submit`` or the recorded request it made. + via: str = "" #: Requests the page tried to send and were blocked, ``METHOD /path``. blocked: list[str] = field(default_factory=list) seconds: float = 0.0 @@ -127,13 +164,8 @@ def valid(self) -> bool: return self.status == "valid" -def _tried(blocked: list[str]) -> str: - sent = sorted({b.removeprefix("after load: ") for b in blocked}) - return f"the form's script tried to send {', '.join(sent)} while it ran (blocked)" - - -def _summary(request: Any) -> str: - return f"{request.method} {up.urlsplit(request.url).path}" +def _summary(method: str, url: str) -> str: + return f"{method} {up.urlsplit(url).path}" def evaluate_form( @@ -143,8 +175,10 @@ def evaluate_form( action_path: str, fields: list[tuple[str, str]], *, + submitter: Optional[str] = None, + submitter_requests: tuple[str, ...] = (), timeout_s: float = 20.0, - settle_ms: int = 150, + settle_ms: int = 250, ) -> FormEvaluation: """Run the form's script in a scratch copy of ``page``'s session. @@ -154,54 +188,70 @@ def evaluate_form( """ started = time.monotonic() - blocked: list[str] = [] + late: list[dict[str, Any]] = [] errors: list[str] = [] dialogs: list[str] = [] phase = {"loaded": False} - origin = up.urlsplit(base_url) + origin = up.urlsplit(base_url).netloc context = None scratch = None - own_context = False def route(r: Any) -> None: req = r.request - same_origin = up.urlsplit(req.url).netloc == origin.netloc - script_sent = req.resource_type in ("xhr", "fetch", "eventsource", "websocket") - if req.method not in ("GET", "HEAD") or (phase["loaded"] and script_sent): - blocked.append(("after load: " if phase["loaded"] else "") + _summary(req)) + same_origin = up.urlsplit(req.url).netloc == origin + if phase["loaded"]: + # Once the page has loaded, nothing leaves: no write, no read + # that could reach the session, no other origin. + try: + body = req.post_data + except Exception: # noqa: BLE001 - a binary body has no text + body = None + late.append( + { + "what": _summary(req.method, req.url), + "method": req.method, + "path": up.urlsplit(req.url).path, + "body": body, + "content_type": (req.headers or {}).get("content-type", ""), + } + ) r.abort() - elif req.resource_type in ("image", "media", "font") or ( - not same_origin and req.resource_type not in ("script", "stylesheet") + elif ( + req.method not in ("GET", "HEAD") + or not same_origin + or req.resource_type in ("image", "media", "font") ): r.abort() else: r.continue_() - def done(status: str, reason: str = "", serialized: Any = None) -> FormEvaluation: + def done( + status: str, reason: str = "", serialized: Any = None, via: str = "" + ) -> FormEvaluation: return FormEvaluation( status=status, reason=reason, serialized=serialized, - blocked=list(blocked), + via=via, + blocked=[b["what"] for b in late], seconds=round(time.monotonic() - started, 3), ) try: live = page.context browser = getattr(live, "browser", None) - if browser is not None: - # A separate context keeps the run's own tabs, history and page - # count untouched; it shares the session through its cookies. - context = browser.new_context( - storage_state=live.storage_state(), - viewport=page.viewport_size or {"width": 1280, "height": 800}, + if browser is None: + # Without a separate context the scratch page would share the + # run's tabs and storage; don't run the page's script there. + return done( + "unevaluable", "this session can't open a separate scratch context" ) - own_context = True - else: - context = live + context = browser.new_context( + storage_state=live.storage_state(), + viewport=page.viewport_size or {"width": 1280, "height": 800}, + service_workers="block", + ) scratch = context.new_page() - # Page-level, so a scratch tab in the live context leaves the run's - # own pages alone. scratch.add_init_script(_INSTRUMENT_JS) scratch.route("**/*", route) scratch.on("pageerror", lambda exc: errors.append(str(exc)[:200])) @@ -228,78 +278,113 @@ def on_dialog(d: Any) -> None: f"the form page raised an error as it loaded ({errors[0]})", ) phase["loaded"] = True - load_blocked = list(blocked) - result = scratch.evaluate( - _EVALUATE_JS, - { - "actionPath": action_path, - "leaf": action_path.rsplit("/", 1)[-1], - "fields": [[k, v] for k, v in fields], - "settleMs": settle_ms, - }, - ) - serialized = [(str(k), str(v)) for k, v in result.get("serialized") or []] - late = [b for b in blocked if b not in load_blocked] - if late: - return done("unevaluable", _tried(late), serialized) - if result.get("unevaluable"): - return done("unevaluable", result["unevaluable"]) - if errors: - return done( - "unevaluable", f"the form's script raised an error ({errors[0]})" - ) - if result.get("threw"): - return done( - "unevaluable", f"the form's script raised an error ({result['threw']})" - ) - if result.get("opened"): - return done("unevaluable", "the form's script opened a window while it ran") - if dialogs: - return done( - "invalid", - f"the form's script showed a dialog ({dialogs[0]})", - serialized, - ) - if result.get("invalid"): - return done( - "invalid", - "the browser's own checks refuse these values (" - + "; ".join(result["invalid"][:3]) - + ")", - serialized, - ) - if result.get("rules"): - names = ", ".join(sorted(result["rules"])[:5]) - return done( - "invalid", - f"the form's validation rules refuse these values ({names})", - serialized, - ) - if not result.get("reached"): - return done( - "unevaluable", - "the form's script stopped the submit before it could be seen", - serialized, - ) - if result.get("prevented") and not result.get("programmaticSubmits"): - return done( - "invalid", - "the form's script refused the submit for these values", - serialized, + try: + result = scratch.evaluate( + _EVALUATE_JS, + { + "actionPath": action_path, + "leaf": action_path.rsplit("/", 1)[-1], + "fields": [[k, v] for k, v in fields], + "settleMs": settle_ms, + "submitter": submitter, + }, ) - return done("valid", "", serialized) + except Exception as exc: # noqa: BLE001 - e.g. the page navigated away + result = {"navigated": type(exc).__name__} + return _verdict( + result, late, errors, dialogs, action_path, set(submitter_requests), done + ) except Exception as exc: # noqa: BLE001 - a failure to run is unevaluable - late = [b for b in blocked if b.startswith("after load: ")] - if late: - # For example the page let the browser's own submit through - # without our listener seeing it: the route blocked it. - return done("unevaluable", _tried(late)) return done("unevaluable", f"the scratch page failed ({type(exc).__name__})") finally: try: if scratch is not None: scratch.close() - if own_context and context is not None: + if context is not None: context.close() except Exception: # noqa: BLE001 - cleanup only pass + + +def _pairs(items: Any) -> list[tuple[str, str]]: + return [(str(k), str(v)) for k, v in items or []] + + +def _verdict( + result: dict[str, Any], + late: list[dict[str, Any]], + errors: list[str], + dialogs: list[str], + action_path: str, + expected: set[str], + done: Any, +) -> FormEvaluation: + """Decide from what the page did; any doubt is a reason for the GUI.""" + + if result.get("unevaluable"): + return done("unevaluable", result["unevaluable"]) + saves = [b for b in late if b["method"] == "POST" and b["path"] == action_path] + recorded = [b for b in late if b["what"] in expected and b not in saves] + others = [b for b in late if b not in saves and b not in recorded] + opened_all = list(result.get("opened") or []) + opened = [u for u in opened_all if f"GET {up.urlsplit(u).path}" not in expected] + if others: + sent = sorted({b["what"] for b in others}) + return done( + "unevaluable", + f"the form's script tried to send {', '.join(sent)} while it ran (blocked)", + ) + if errors: + return done("unevaluable", f"the form's script raised an error ({errors[0]})") + if result.get("threw"): + return done( + "unevaluable", f"the form's script raised an error ({result['threw']})" + ) + if opened: + return done("unevaluable", "the form's script opened a window while it ran") + if dialogs: + return done("invalid", f"the form's script showed a dialog ({dialogs[0]})") + if result.get("rules"): + names = ", ".join(sorted(result["rules"])[:5]) + return done( + "invalid", f"the form's validation rules refuse these values ({names})" + ) + # What the page submits, at the moment it submits. + if saves: + save = saves[0] + if save["body"] is None or "x-www-form-urlencoded" not in save["content_type"]: + return done( + "unevaluable", "the page submitted a body the API step can't compare" + ) + body = up.parse_qsl(save["body"], keep_blank_values=True) + return done("valid", "", body, "submit") + if result.get("reached") and not result.get("prevented"): + return done("valid", "", _pairs(result.get("submitted")), "submit") + if result.get("programmatic"): + body = result["programmatic"][0] + if body is None: + return done("unevaluable", "the page's own submit could not be read") + return done("valid", "", _pairs(body), "submit") + if recorded or len(opened_all) > len(opened): + via = recorded[0]["what"] if recorded else "a window from the recording" + return done("valid", "", _pairs(result.get("after")), via) + if result.get("navigated"): + return done("unevaluable", "the page navigated while its script ran") + if result.get("invalid"): + return done( + "invalid", + "the browser's own checks refuse these values (" + + "; ".join(result["invalid"][:3]) + + ")", + ) + if result.get("prevented"): + return done("invalid", "the form's script refused the submit for these values") + if result.get("clicked"): + return done( + "invalid", + "after the recorded click the page neither submitted nor went on as " + "recorded (its script may have refused these values)", + ) + return done( + "unevaluable", "the form's script stopped the submit before it could be seen" + ) diff --git a/packages/flow/openadapt_flow/runtime/actuators/web_form.py b/packages/flow/openadapt_flow/runtime/actuators/web_form.py index 8638b11624..d6f3966a71 100644 --- a/packages/flow/openadapt_flow/runtime/actuators/web_form.py +++ b/packages/flow/openadapt_flow/runtime/actuators/web_form.py @@ -77,6 +77,8 @@ def handle_starttag(self, tag: str, attrs: list[tuple[str, Optional[str]]]) -> N "action": a.get("action", ""), "controls": {}, "handlers": sorted(k for k in a if k.startswith("on")), + "handler_src": {k: v for k, v in a.items() if k.startswith("on")}, + "buttons": [], "novalidate": "novalidate" in a, } self.forms.append(self._form) @@ -84,8 +86,21 @@ def handle_starttag(self, tag: str, attrs: list[tuple[str, Optional[str]]]) -> N if self._form is None: return name = a.get("name") + kind = a.get("type", "text" if tag == "input" else "submit").lower() + if tag == "button" or ( + tag == "input" and kind in ("submit", "button", "image") + ): + # A control that can start the submit: its click handlers run + # before the form's own. + self._form["buttons"].append( + { + "id": a.get("id"), + "name": name, + "type": kind, + "handler_src": {k: v for k, v in a.items() if k.startswith("on")}, + } + ) if tag == "input" and name: - kind = a.get("type", "text").lower() if kind in ("submit", "button", "image", "reset", "file"): return if kind in ("checkbox", "radio") and "checked" not in a: @@ -162,6 +177,7 @@ def _native_rules(a: dict[str, str]) -> dict[str, Any]: "maxlength": a.get("maxlength"), "disabled": "disabled" in a, "handlers": sorted(k for k in a if k.startswith("on")), + "handler_src": {k: v for k, v in a.items() if k.startswith("on")}, } @@ -225,17 +241,51 @@ def unsupported_client_rules(rules: Optional[dict[str, Any]]) -> list[str]: return sorted(out) +_SCRIPT_SRC = re.compile(r"""]*?\bsrc\s*=\s*["']([^"']+)["']""", re.I) +_INLINE_SCRIPT = re.compile(r"]*\bsrc\s*=)[^>]*>\s*\S", re.I) + + +def _src_digest(source: str) -> str: + return hashlib.sha256(source.encode("utf-8")).hexdigest()[:12] + + def page_scripts( html: str, action_path: str, sent: Iterable[str], rules: Optional[dict[str, Any]] ) -> list[str]: - """Everything the browser runs on this save that reading the form can't evaluate. - - :func:`form_scripts` plus the validate.js rule kinds outside - :data:`CLIENT_RULE_KINDS`, sorted. The API step runs these itself before - each save (:mod:`openadapt_flow.runtime.actuators.form_eval`). + """Every script the browser may run on this save, fingerprinted, sorted. + + The form's own ``on*`` handlers, the ``on*`` handlers of the fields the + save sends and of the form's buttons (each with a digest of its source), + a page script listening for submit, every script the page loads (by + URL), whether it has inline script, and the validate.js rule kinds + outside :data:`CLIENT_RULE_KINDS`. Any entry means the API step runs the + page's script before each save + (:mod:`openadapt_flow.runtime.actuators.form_eval`), and a live page whose + list differs doesn't run the API step. """ - found = form_scripts(html, action_path, sent) + found: list[str] = [] + form = _form(html, action_path) + if form is not None: + found += [f"form {h} {_src_digest(v)}" for h, v in form["handler_src"].items()] + sent_names = set(sent) + for name, control in form["controls"].items(): + if name in sent_names: + found += [ + f"{name} {h} {_src_digest(v)}" + for h, v in (control.get("handler_src") or {}).items() + ] + for button in form["buttons"]: + label = button["id"] or button["name"] or button["type"] + found += [ + f"button {label} {h} {_src_digest(v)}" + for h, v in button["handler_src"].items() + ] + if _SUBMIT_LISTENER.search(html): + found.append("a page script listening for submit") + found += [f"script {src}" for src in _SCRIPT_SRC.findall(html)] + if _INLINE_SCRIPT.search(html): + found.append("inline script") found += [f"{rule} (validate.js)" for rule in unsupported_client_rules(rules)] return sorted(set(found)) @@ -449,7 +499,13 @@ def __init__(self, page: Any, *, timeout_s: float = 30.0) -> None: self._timeout_ms = timeout_s * 1000.0 def evaluate_form( - self, form_path: str, action_path: str, fields: list[tuple[str, str]] + self, + form_path: str, + action_path: str, + fields: list[tuple[str, str]], + *, + submitter: Optional[str] = None, + submitter_requests: tuple[str, ...] = (), ) -> Any: from openadapt_flow.runtime.actuators.form_eval import evaluate_form @@ -459,6 +515,8 @@ def evaluate_form( form_path, action_path, fields, + submitter=submitter, + submitter_requests=submitter_requests, timeout_s=self._timeout_ms / 1000.0, ) @@ -748,9 +806,11 @@ def _run_page_script( """Run the form's own script on the run's values; why not to send, or None. The fields the person typed or picked go into a scratch copy of the - form (form_eval). The save is sent only when the page's script lets - the submit through and the browser would send exactly ``body``, apart - from the session values the scratch page was issued for itself. + form, and the recorded control is clicked (form_eval). The save is + sent only when the page submits, or goes on to a request the click + made in the recording, and the body it submits is exactly ``body`` + apart from the session values the scratch page was issued for + itself. Any difference keeps the save on the GUI. """ evaluate = getattr(self.transport, "evaluate_form", None) @@ -758,12 +818,17 @@ def _run_page_script( return "the form runs script and this session can't run it" typed = {f.name for f in spec.fields if f.source in ("param", "select_label")} result = evaluate( - form_path, action_path, [(k, v) for k, v in body if k in typed] + form_path, + action_path, + [(k, v) for k, v in body if k in typed], + submitter=spec.submitter, + submitter_requests=tuple(spec.submitter_requests), ) self.log.append( { "stage": "page_script", "status": result.status, + "via": result.via, "seconds": result.seconds, "blocked": result.blocked, } diff --git a/packages/flow/tests/test_api_step_native_validation.py b/packages/flow/tests/test_api_step_native_validation.py index 9d270f987e..f080415a46 100644 --- a/packages/flow/tests/test_api_step_native_validation.py +++ b/packages/flow/tests/test_api_step_native_validation.py @@ -151,7 +151,11 @@ def test_script_on_the_form_is_recorded_and_must_be_run_before_the_save( run = {"lname": "Marsh", "state_label": "California", "note": "Referred"} live = edit(FORM_HTML.replace("", "live-session-value")) # A session that can't run the page's script keeps the save on the GUI. - out = WebFormActuator(FakeTransport(form=live)).actuate(binding, run) + + class NoScriptTransport(FakeTransport): + evaluate_form = None + + out = WebFormActuator(NoScriptTransport(form=live)).actuate(binding, run) assert out.status is ActuationStatus.UNAVAILABLE assert "can't run it" in out.reason or "rules changed" in out.reason # So does a live form whose script is not the recorded script. diff --git a/packages/flow/tests/test_api_step_page_script.py b/packages/flow/tests/test_api_step_page_script.py index d83bc57c1a..603f92bb31 100644 --- a/packages/flow/tests/test_api_step_page_script.py +++ b/packages/flow/tests/test_api_step_page_script.py @@ -21,6 +21,7 @@ from openadapt_flow.runtime.actuators.web_form import ( PlaywrightSessionTransport, WebFormActuator, + page_scripts, ) playwright = pytest.importorskip("playwright.sync_api") @@ -111,9 +112,9 @@ def app_page(browser): GOOD = [("lname", "Marsh"), ("dob", "1979-06-14"), ("state", "CA")] -def _evaluate(app: _App, page: Any, fields=GOOD) -> FormEvaluation: +def _evaluate(app: _App, page: Any, fields=GOOD, **kw: Any) -> FormEvaluation: return PlaywrightSessionTransport(page).evaluate_form( - "/new.php", "/save.php", fields + "/new.php", "/save.php", fields, **kw ) @@ -189,8 +190,8 @@ def test_a_programmatic_submit_after_validation_counts_and_never_navigates(app_p "form_attrs": "", "script": "document.forms[0].addEventListener('submit', e => e.stopPropagation())", }, - "unevaluable", - "tried to send POST /save.php", + "valid", + "", ), ({"form_attrs": "", "lname_attrs": "required"}, "valid", ""), ( @@ -269,8 +270,9 @@ def post_form(self, path, body, *, referer): return _Resp(302, "", {"location": "/view.php?id=7"}) - def evaluate_form(self, form_path, action_path, fields): + def evaluate_form(self, form_path, action_path, fields, **kw): self.calls.append(fields) + self.kw = kw return self.evaluation @@ -286,7 +288,13 @@ def _binding(): WebFormField(name="state", source="select_label", param="state_label"), ], controls=["csrf", "dob", "lname", "state"], - client_scripts=["dob onchange", "form onsubmit"], + client_scripts=page_scripts( + FORM.format(form_attrs=VALIDATE, lname_attrs="", script="", n="1"), + "/save.php", + ["csrf", "lname", "dob", "state"], + None, + ), + submitter="button[type=submit]", replaces_steps=["s0"], folded_checks_digest="sha256:" + "0" * 64, ) @@ -319,6 +327,7 @@ def test_the_actuator_sends_only_what_the_page_would_send(): assert out.status.value == "actuated", out.reason # Only the typed and picked fields are put into the scratch form. assert t.calls == [[("lname", "Marsh"), ("dob", "1979-06-14"), ("state", "CA")]] + assert t.kw["submitter"] == "button[type=submit]" assert len(t.posts) == 1 and "csrf=tok-1" in t.posts[0] @@ -365,3 +374,87 @@ def test_the_actuator_leaves_the_save_to_the_gui_when_the_page_disagrees( out = WebFormActuator(t).actuate(_binding(), RUN) assert out.status.value == "unavailable" and reason in out.reason assert t.posts == [] + + +# -- review cases: submit-time body, click handlers, blocking --------------------- + + +def test_an_onsubmit_rewrite_is_in_the_body_compared(app_page): + app, page = app_page + app.form( + form_attrs='onsubmit="this.lname.value = this.lname.value.toUpperCase(); return true"' + ) + result = _evaluate(app, page) + assert result.status == "valid" and result.via == "submit" + assert ("lname", "MARSH") in result.serialized + + +def test_a_click_handler_on_a_button_needs_the_recorded_control(app_page): + app, page = app_page + app.form( + form_attrs="", + script="document.querySelector('button').addEventListener('click', " + "e => { if (!pastDate(document.forms[0].dob.value)) e.preventDefault(); })", + ) + # No recorded control: refused, the GUI runs. + assert _evaluate(app, page).status == "unevaluable" + # The recorded control is clicked, as the person did. + assert _evaluate(app, page, submitter="button").status == "valid" + bad = [("lname", "Marsh"), ("dob", "2999-01-01"), ("state", "CA")] + assert _evaluate(app, page, bad, submitter="button").status == "invalid" + assert app.writes == [] + + +def test_a_click_that_goes_on_to_the_recorded_request_counts(app_page): + app, page = app_page + app.pages["/dup.php"] = "No matches" + app.form( + form_attrs="", + lname_attrs="", + script="function check() { if (pastDate(document.forms[0].dob.value)) {" + " const f = document.createElement('iframe'); f.src = '/dup.php?lname=x';" + " document.body.appendChild(f); } }", + ) + page_html = app.pages["/new.php"].replace( + '', + '', + ) + app.pages["/new.php"] = page_html + loads = app.loads + ok = _evaluate(app, page, submitter="#create", submitter_requests=("GET /dup.php",)) + assert ok.status == "valid" and ok.via == "GET /dup.php" + # The duplicate search was blocked: only the form page itself loaded. + assert app.loads == loads + 1 + bad = [("lname", "Marsh"), ("dob", "2999-01-01"), ("state", "CA")] + refused = _evaluate( + app, page, bad, submitter="#create", submitter_requests=("GET /dup.php",) + ) + assert refused.status == "invalid" and "neither submitted" in refused.reason + # A request the recording didn't make is refused. + other = _evaluate( + app, page, submitter="#create", submitter_requests=("GET /else.php",) + ) + assert other.status == "unevaluable" and "GET /dup.php" in other.reason + + +def test_no_request_leaves_after_the_page_has_loaded(app_page): + app, page = app_page + app.pages["/lookup.php"] = "ok" + app.form( + script="document.forms[0].dob.addEventListener('change', () => { new Image().src = '/lookup.php?q=1'; });" + ) + loads = app.loads + result = _evaluate(app, page) + assert result.status == "unevaluable" and "GET /lookup.php" in result.reason + assert app.loads == loads + 1 # the form page only + + +def test_the_script_fingerprint_covers_handler_source_and_script_urls(): + html = FORM.format(form_attrs=VALIDATE, lname_attrs="", script="", n="1").replace( + "", +} +BACKGROUND = { + "t": 3.4, + "method": "POST", + "url": "http://app/library/ajax/bg.php", + "status": None, + "failed": True, + "resource_type": "fetch", + "frame_url": "http://app/main.php", + "request_type": "multipart/form-data", + "body_omitted": "multipart", +} +DRAFT = { + "t": 4.2, + "method": "POST", + "url": "http://app/app/draft.php", + "status": 200, + "resource_type": "xhr", + "frame_url": "http://app/app/new.php", + "form": [["csrf_token_form", ""], ["form_lname", "Marsh"]], +} + + +def _rec(tmp_path, *extra): + rec = _recording(tmp_path) + rows = [json.loads(x) for x in (rec / nc.NETWORK_FILE).read_text().splitlines()] + body = sorted(rows[1:] + [dict(e) for e in extra], key=lambda r: r["t"]) + (rec / nc.NETWORK_FILE).write_text( + json.dumps({"schema": nc.SCHEMA, "exchanges": len(body)}) + + "\n" + + "".join(json.dumps(r) + "\n" for r in body) + ) + return rec + + +def _with_step_effect(wf, step_id="step_003"): + steps = [ + s.model_copy(update={"effects": [_effect()]}) if s.id == step_id else s + for s in wf.steps + ] + return wf.model_copy(update={"steps": steps}) + + +def test_a_request_the_page_sends_on_its_own_is_ambient_and_not_sent(tmp_path): + wf, step, reasons = propose_api_step(_workflow(), _rec(tmp_path, SHELL, BACKGROUND)) + assert reasons == [] and step is not None + (write,) = step.api_binding.web_form.writes + assert write.role == "ambient" and write.path == "/library/ajax/bg.php" + assert ( + write.sent_by == "/main.php" + and "loaded before the form opened" in write.evidence + ) + assert "not sent (the page's own request)" in describe_api_step(step) + wf, _ = admit_api_step(wf, admitted_by="reviewer") + t = FakeTransport() + out = WebFormActuator(t).actuate(wf.steps[-1].api_binding, RUN) + assert out.status is ActuationStatus.ACTUATED + assert [p for p, _ in t.posts] == ["/app/save.php"] + + +@pytest.mark.parametrize( + "edit, why", + [ + # Sent by the form's own document: a replaced step may have caused it. + ( + lambda b, s: (dict(b, frame_url="http://app/app/new.php"), s), + "can't rebuild", + ), + # The sending page wasn't open before the form: a step may have opened it. + (lambda b, s: (b, dict(s, t=0.5)), "can't rebuild"), + # Navigation or form posts are never ambient. + (lambda b, s: (dict(b, resource_type="document"), s), "can't rebuild"), + ], +) +def test_a_write_without_the_evidence_is_a_step_write(tmp_path, edit, why): + background, shell = edit(dict(BACKGROUND), dict(SHELL)) + _wf, step, reasons = propose_api_step( + _workflow(), _rec(tmp_path, shell, background) + ) + assert step is None and why in " ".join(reasons) + + +def test_an_endpoint_the_form_page_names_is_not_ambient(tmp_path): + rec = _rec(tmp_path, SHELL, BACKGROUND) + rows = [json.loads(x) for x in (rec / nc.NETWORK_FILE).read_text().splitlines()] + for row in rows[1:]: + if row.get("url") == "http://app/app/new.php": + row["response_text"] += "" + (rec / nc.NETWORK_FILE).write_text("".join(json.dumps(r) + "\n" for r in rows)) + _wf, step, reasons = propose_api_step(_workflow(), rec) + assert step is None and "bg.php" in " ".join(reasons) + + +def test_a_step_write_is_declared_in_order_and_needs_its_own_read_back(tmp_path): + wf, step, reasons = propose_api_step(_workflow(), _rec(tmp_path, DRAFT)) + assert reasons == [] and step is not None + (write,) = step.api_binding.web_form.writes + assert (write.role, write.from_step, write.expected_status) == ( + "step", + "step_003", + [200], + ) + assert [(f.name, f.source, f.param) for f in write.fields] == [ + ("csrf_token_form", "session", None), + ("form_lname", "param", "lname"), + ] + assert "sent before the save, in this order" in describe_api_step(step) + with pytest.raises(ApiProposalError, match="has no record check"): + admit_api_step(wf, admitted_by="reviewer") + wf = _with_step_effect(wf) + wf, _ = admit_api_step(wf, admitted_by="reviewer") + save = wf.steps[-1] + # The binding's record check reads back the save and the step's write. + assert save.api_binding.effects == web_form_effects(wf.steps, save) + assert len(save.api_binding.effects) == len(save.effects) + 1 + assert [row.step_id for row in admitted_api_steps(wf)] == [save.id] + + +def _admitted_sequence(tmp_path): + wf, _step, _ = propose_api_step(_workflow(), _rec(tmp_path, DRAFT)) + wf, _ = admit_api_step(_with_step_effect(wf), admitted_by="reviewer") + return wf.steps[-1].api_binding + + +class SequenceTransport(FakeTransport): + def __init__(self, write_status=200, raise_on_write=False): + super().__init__() + self.write_status, self.raise_on_write = write_status, raise_on_write + + def post_form(self, path, body, *, referer): + if path == "/app/draft.php": + if self.raise_on_write: + raise requests.exceptions.ConnectionError("reset after send") + self.posts.append((path, body)) + return _Resp(self.write_status, "", {}) + return super().post_form(path, body, referer=referer) + + +def test_the_sequence_sends_each_write_in_order_then_the_save(tmp_path): + t = SequenceTransport() + out = WebFormActuator(t).actuate(_admitted_sequence(tmp_path), RUN) + assert out.status is ActuationStatus.ACTUATED + assert [p for p, _ in t.posts] == ["/app/draft.php", "/app/save.php"] + assert "form_lname=Feld" in t.posts[0][1] and "live-session-value" in t.posts[0][1] + + +@pytest.mark.parametrize( + "transport, kind, said", + [ + ( + lambda: SequenceTransport(write_status=500), + ApiHaltKind.RESPONSE_REJECTED, + "the save was not sent", + ), + ( + lambda: SequenceTransport(raise_on_write=True), + ApiHaltKind.DELIVERY_UNCERTAIN, + "may have written", + ), + ], +) +def test_a_failed_write_stops_the_sequence_and_nothing_is_retried( + tmp_path, transport, kind, said +): + t = transport() + out = WebFormActuator(t).actuate(_admitted_sequence(tmp_path), RUN) + assert ( + out.status is ActuationStatus.HALT + and out.halt_kind is kind + and said in out.reason + ) + assert "/app/save.php" not in [p for p, _ in t.posts] + assert len([p for p, _ in t.posts if p == "/app/draft.php"]) <= 1 + + +def test_the_approved_digest_covers_every_write_and_keeps_the_old_one_without(tmp_path): + seq = WebFormActuator(SequenceTransport()).prepare( + _admitted_sequence(tmp_path), RUN + ) + assert len(seq.writes) == 1 + bare = seq.__class__( + method=seq.method, + path=seq.path, + body=seq.body, + referer=seq.referer, + session_fields=seq.session_fields, + ) + assert bare.payload_sha256() != seq.payload_sha256() + # Without writes the digest is the one the approval gate always used. + import hashlib + + canonical = json.dumps( + { + "method": bare.method.upper(), + "path": bare.path, + "content_type": "application/x-www-form-urlencoded", + "body": bare.encoded(), + "referer": bare.referer, + }, + sort_keys=True, + separators=(",", ":"), + ) + assert ( + bare.payload_sha256() + == "sha256:" + hashlib.sha256(canonical.encode()).hexdigest() + ) + + +@pytest.mark.parametrize( + "write, why", + [ + (dict(DRAFT, t=0.5), "before any replaced step"), + (dict(DRAFT, url="http://other/app/draft.php"), "another origin"), + (dict(DRAFT, url="http://app/app/draft.php?id=7"), "values in its URL"), + (dict(DRAFT, status=500), "wasn't accepted"), + (dict(DRAFT, form=[["form_lname", "Someone else"]]), "is not a typed value"), + (dict(DRAFT, form=None, body_omitted="multipart"), "can't rebuild"), + ], +) +def test_a_step_write_the_api_step_cant_repeat_refuses_the_proposal( + tmp_path, write, why +): + _wf, step, reasons = propose_api_step(_workflow(), _rec(tmp_path, write)) + assert step is None and why in " ".join(reasons) From d772011c26b27a3e00c3af44fff199dbe88958b2 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sat, 24 Oct 2026 23:53:24 -0400 Subject: [PATCH 09/15] benchmark: OpenEMR referral harness for the admissible API step Interleaved browser and API arms on the API-tier fixture, the eight referrals plus a future date of birth (the form's pastDate rule must refuse it) and a lower-case first name, a per-item timeout, the test setup's own SQL read-back, and the whole saved patient_data row per run so the two arms' saves can be compared column by column. Co-Authored-By: Claude Opus 5.5 --- .../benchmark/openemr_api_tier/admissible.py | 321 ++++++++++++++++++ 1 file changed, 321 insertions(+) create mode 100644 packages/flow/benchmark/openemr_api_tier/admissible.py diff --git a/packages/flow/benchmark/openemr_api_tier/admissible.py b/packages/flow/benchmark/openemr_api_tier/admissible.py new file mode 100644 index 0000000000..183fcb4875 --- /dev/null +++ b/packages/flow/benchmark/openemr_api_tier/admissible.py @@ -0,0 +1,321 @@ +"""The OpenEMR referral's API step, admissible: page script run, background write declared. + +Until now the compiler refused an API step for this form for two reasons: the +form validates with script (``submitme(...)`` on submit, a custom validate.js +``pastDate`` rule on the date of birth, ``onchange`` on every field) and the +tab shell posts ``execute_background_services.php`` while the form is open. +With the API step running the page's own script before each save +(runtime/actuators/form_eval.py) and declaring the background post as the +page's own request (``web_form.writes``, role ``ambient``), the same +recording now gets an API step. This harness measures it on the local +OpenEMR test copy, against the same program on the GUI. + +Label every number exactly as: "plain replayer, no approval gate, no durable +run, no run gate; API step admitted by the harness (benchmark-fixture)". + +- Arms: ``browser`` (``api_steps=False``) and ``api`` (``api_steps=True``), + interleaved by block; each block resets the fixture and seeds the chart r6 + must not duplicate. +- Items: the eight referrals of ``common.REFERRALS`` (r6 must create no + chart), plus ``x1-future-dob`` (the form's ``pastDate`` rule must refuse + it: no chart) and ``x2-lowercase`` (a first name typed in lower case; + both arms must save the same thing). +- Read-back: the test setup's own SQL check (``common.record_check``), never + the arm under test, plus the whole new ``patient_data`` row, so the two + arms' saves can be compared column by column. +- Time: ``wall_s`` from a fresh browser context through sign-in, the entry and + the read-back; items per hour is 3600 over the mean ``wall_s`` of one + sequential worker. Each item has a hard timeout (``--item-timeout``). + + python -m benchmark.openemr_api_tier.admissible compile --recording --out + python -m benchmark.openemr_api_tier.admissible run --bundle --out [--blocks 4] + python -m benchmark.openemr_api_tier.admissible summarize --out +""" + +from __future__ import annotations + +import argparse +import csv +import json +import shutil +import signal +import statistics +import subprocess +import sys +import time +from pathlib import Path +from typing import Any + +from benchmark.openemr_api_tier.common import ( + _MA, + REFERRALS, + Referral, + load_demo, + record_check, + reset_and_seed, + spec_for, + sql_patients, +) +from benchmark.openemr_api_tier.feature_e2e import NO_MATCH_TEXT, _load1, wait_quiet +from benchmark.openemr_api_tier.spike_fixture import SpikeFixture + +LABEL = ( + "plain replayer, no approval gate, no durable run, no run gate; " + "API step admitted by the harness (benchmark-fixture)" +) +#: A date of birth in the future: the form's own pastDate rule refuses it. +FUTURE_DOB = Referral( + "x1-future-dob", + dict(title="Ms.", fname="Wren", lname="Halvorsen", dob="2031-02-03", sex="Female", + street="5 ORCHARD LANE", phone_home="555-0171", email="wren.halvorsen@example.invalid", **_MA), + mismatch=True, +) +#: A first name typed in lower case: whatever the page does with it, both +#: arms must save the same chart. +LOWERCASE = Referral( + "x2-lowercase", + dict(title="Mr.", fname="otto", lname="Brandvold", dob="1968-08-21", sex="Male", + street="21 KILN STREET", phone_home="555-0176", email="otto.brandvold@example.invalid", **_MA), +) +ITEMS = [*REFERRALS, FUTURE_DOB, LOWERCASE] +#: Columns that differ between any two saves of the same referral. +VOLATILE = {"id", "pid", "pubpid", "uuid", "date", "regdate", "created_by", "updated_by", + "last_updated", "created_at", "updated_at"} + +COLS = ["arm", "block", "pos", "ref", "expect_chart", "correct", "new_charts", "readback", + "engine_success", "outcome", "transaction_outcome", "save_actuation", "api_fallback", "error", "wall_s", "setup_s", "engine_s", "readback_s", + "load1", "gate_waited_s", "start_local", "engine_sha"] + + +def cmd_compile(a: argparse.Namespace) -> None: + from openadapt_flow.compiler.api_proposal import ( + admit_api_step, + describe_api_step, + propose_api_step, + ) + from openadapt_flow.ir import Postcondition, PostconditionKind, Workflow + + demo = load_demo() + rec, bundle = Path(a.recording), Path(a.out) + demo.compile_recording(rec, bundle) + first = json.loads((bundle / "api_proposal.json").read_text()) + wf = Workflow.load(bundle) + # The referral's business rule (as in feature_e2e): confirm a new chart + # only when OpenEMR's duplicate check finds no existing patient. + dialog = [s for s in wf.steps if s.anchor is not None and s.anchor.structural is not None + and s.anchor.structural.selector == "#create"] + assert len(dialog) == 1 + dialog[0].expect = list(dialog[0].expect) + [ + Postcondition(kind=PostconditionKind.TEXT_PRESENT, text=NO_MATCH_TEXT, timeout_s=10.0)] + wf, step, reasons = propose_api_step(wf, rec, replace=True) + if step is None: + raise SystemExit(f"no API step: {reasons}") + wf, _ = admit_api_step(wf, admitted_by="benchmark-fixture") + wf.save(bundle) + save = next(s for s in wf.steps if s.id == step.id) + out = {"first_compile": first, "proposed": step.id, "label": LABEL, + "client_scripts": save.api_binding.web_form.client_scripts, + "writes": [w.model_dump(mode="json") for w in save.api_binding.web_form.writes], + "describe": describe_api_step(save)} + (bundle / "admissible-compile.json").write_text(json.dumps(out, indent=1)) + print(out["describe"]) + + +def _columns(f: SpikeFixture) -> list[str]: + out = f._db("SELECT column_name FROM information_schema.columns WHERE table_schema='openemr' " + "AND table_name='patient_data' ORDER BY ordinal_position").decode() + return [c for c in out.split() if c] + + +def _rows(f: SpikeFixture, cols: list[str], ids: list[str]) -> list[dict[str, str]]: + if not ids: + return [] + sel = ",".join(f"`{c}`" for c in cols) + out = f._db(f"SELECT {sel} FROM openemr.patient_data WHERE id IN ({','.join(ids)}) ORDER BY id").decode() + return [{k: ("" if v == "NULL" else v) for k, v in zip(cols, line.split("\t"))} + for line in out.splitlines()] + + +class _Timeout(Exception): + pass + + +def _alarm(_signum: int, _frame: Any) -> None: + raise _Timeout("item timeout") + + +def run_one(env: dict[str, Any], arm: str, ref: Referral, bundle: Path, out: Path, timeout_s: int) -> dict[str, Any]: + from openadapt_flow.benchmark.openemr_local import OpenEMRPatientOracle + from openadapt_flow.ir import Workflow + from openadapt_flow.runtime import Replayer + + demo, f, browser, ocr = env["demo"], env["f"], env["browser"], env["ocr"] + spec = spec_for(ref) + out.mkdir(parents=True) + before_ids = {r["id"] for r in sql_patients(f)} + oracle = OpenEMRPatientOracle(f.api_base_url, f.token_session("oracle"), spec) + res: dict[str, Any] = {"arm": arm, "ref": ref.key, "expect_chart": not ref.mismatch, "error": None} + t0 = time.time() + ctx = browser.new_context(viewport={"width": 1280, "height": 800}, device_scale_factor=1) + signal.signal(signal.SIGALRM, _alarm) + signal.alarm(timeout_s) + t_setup = t_task = None + try: + page = ctx.new_page() + page.goto(f.ui_base_url) + backend = demo._BasePlaywrightBackend(page) + demo._preauthenticate_browser(backend, f) + t_setup = time.time() + ocr.clear_ocr_memo() + replayer = Replayer(backend, effect_verifier=oracle.verifier, api_steps=(arm == "api")) + rep = replayer.run(Workflow.load(bundle), params=spec.params(), bundle_dir=bundle, + run_dir=out / "engine") + t_task = time.time() + raw = rep.model_dump(mode="json") + last = raw["results"][-1] if raw["results"] else {} + res.update(engine_success=raw.get("success"), outcome=str(raw.get("execution_outcome")), + transaction_outcome=raw.get("transaction_outcome"), save_actuation=last.get("actuation"), + api_fallback=next((r["api_fallback"] for r in raw["results"] if r.get("api_fallback")), None), + step_errors=[(r["step_id"], (r.get("error") or "")[:300]) for r in raw["results"] if r.get("error")]) + except _Timeout: + res["error"] = f"timeout after {timeout_s}s" + except Exception as exc: # noqa: BLE001 + res["error"] = f"{type(exc).__name__}: {exc}"[:500] + finally: + signal.alarm(0) + t_setup = t_setup or time.time() + t_task = t_task or time.time() + time.sleep(demo.ORACLE_SETTLE_S) + res.update(record_check(f, ref, before_ids)) + new_ids = [r["id"] for r in sql_patients(f) if r["id"] not in before_ids] + res["new_rows"] = _rows(f, env["cols"], new_ids) + t_end = time.time() + try: + ctx.close() + except Exception: # noqa: BLE001 + pass + res.update(wall_s=round(t_end - t0, 3), setup_s=round(t_setup - t0, 3), engine_s=round(t_task - t_setup, 3), + readback_s=round(t_end - t_task, 3)) + (out / "result.json").write_text(json.dumps(res, indent=1, default=str)) + shutil.rmtree(out / "engine" / "steps", ignore_errors=True) + return res + + +def cmd_run(a: argparse.Namespace) -> None: + from playwright.sync_api import sync_playwright + + import openadapt_flow.vision.ocr # noqa: F401 + + ocr = sys.modules["openadapt_flow.vision.ocr"] + out, bundle = Path(a.out), Path(a.bundle) + out.mkdir(parents=True, exist_ok=True) + sha = subprocess.run(["git", "rev-parse", "HEAD"], capture_output=True, text=True).stdout.strip()[:9] + demo, f = load_demo(), SpikeFixture() + pw = sync_playwright().start() + browser = pw.chromium.launch(headless=True) + ocr._get_engine() + ctx = browser.new_context(viewport={"width": 1280, "height": 800}) + page = ctx.new_page() + page.goto(f.ui_base_url) + ocr.ocr(page.screenshot(type="png")) + ctx.close() + env = {"demo": demo, "f": f, "browser": browser, "ocr": ocr, "cols": _columns(f)} + (out / "run.json").write_text(json.dumps({"label": LABEL, "engine_sha": sha, "bundle": str(bundle), + "blocks": a.blocks, "items": [r.key for r in ITEMS]}, indent=1)) + csv_path = out / "runs.csv" + new = not csv_path.exists() + try: + for b in range(a.blocks): + for arm in (["browser", "api"] if b % 2 == 0 else ["api", "browser"]): + meta = reset_and_seed(f) | {"arm": arm, "block": b + 1, "started": time.strftime("%Y-%m-%d %H:%M:%S")} + (out / f"block-{b + 1:02d}-{arm}.json").write_text(json.dumps(meta, indent=1)) + for pos, ref in enumerate(ITEMS): + gate = wait_quiet() + start = time.strftime("%H:%M:%S") + res = run_one(env, arm, ref, bundle, out / f"b{b + 1:02d}-{arm}-{pos + 1:02d}-{ref.key}", + a.item_timeout) + row = {k: res.get(k) for k in COLS} | {"block": b + 1, "pos": pos + 1, "load1": _load1(), + "gate_waited_s": gate["gate_waited_s"], + "start_local": start, "engine_sha": sha} + with open(csv_path, "a", newline="") as fh: + w = csv.DictWriter(fh, fieldnames=COLS) + if new: + w.writeheader() + new = False + w.writerow(row) + print(json.dumps({k: row[k] for k in ("arm", "block", "ref", "correct", "readback", + "save_actuation", "api_fallback", "wall_s", + "error")}), flush=True) + finally: + browser.close() + pw.stop() + + +def _same_save(a_rows: list[dict[str, str]], b_rows: list[dict[str, str]]) -> list[str]: + if len(a_rows) != len(b_rows): + return [f"charts: {len(a_rows)} vs {len(b_rows)}"] + diffs = [] + for x, y in zip(a_rows, b_rows): + for k in sorted(set(x) | set(y)): + if k not in VOLATILE and x.get(k) != y.get(k): + diffs.append(f"{k}: {x.get(k)!r} vs {y.get(k)!r}") + return diffs + + +def cmd_summarize(a: argparse.Namespace) -> None: + out = Path(a.out) + results = [json.loads(p.read_text()) for p in sorted(out.glob("b*/result.json"))] + summary: dict[str, Any] = {"label": LABEL, "arms": {}} + for arm in ("browser", "api"): + rs = [r for r in results if r["arm"] == arm] + if not rs: + continue + valid = [r for r in rs if r["expect_chart"]] + walls = [r["wall_s"] for r in rs] + summary["arms"][arm] = { + "items": len(rs), + "correct": sum(bool(r["correct"]) for r in rs), + "wrong_writes": sum(r["new_charts"] > 0 and not r["correct"] for r in rs), + "false_stops": sum(not r["correct"] and r["new_charts"] == 0 for r in valid), + "api_saves": sum(r.get("save_actuation") == "api" for r in rs), + "fallbacks": sorted({r["api_fallback"] for r in rs if r.get("api_fallback") + and r["api_fallback"] != "API steps are off for this run"}), + "errors": [r["error"] for r in rs if r.get("error")], + "median_wall_s": round(statistics.median(walls), 2), + "mean_wall_s": round(statistics.mean(walls), 2), + "items_per_hour": round(3600 / statistics.mean(walls), 1), + "median_engine_s": round(statistics.median(r["engine_s"] for r in rs), 2), + } + # Equivalence: the API arm's chart against the browser arm's, per referral. + by = {} + for r in results: + by.setdefault((r["ref"], r["arm"]), []).append(r) + eq = {} + for ref in sorted({r["ref"] for r in results}): + pairs = list(zip(by.get((ref, "browser"), []), by.get((ref, "api"), []))) + eq[ref] = [_same_save(x.get("new_rows") or [], y.get("new_rows") or []) for x, y in pairs] + summary["equivalence"] = {ref: {"pairs": len(v), "identical": sum(not d for d in v), + "diffs": sorted({x for d in v for x in d})[:10]} for ref, v in eq.items()} + (out / "summary.json").write_text(json.dumps(summary, indent=1)) + print(json.dumps(summary, indent=1)) + + +def main() -> None: + ap = argparse.ArgumentParser() + sub = ap.add_subparsers(dest="cmd", required=True) + c = sub.add_parser("compile") + c.add_argument("--recording", required=True) + c.add_argument("--out", required=True) + r = sub.add_parser("run") + r.add_argument("--bundle", required=True) + r.add_argument("--out", required=True) + r.add_argument("--blocks", type=int, default=4) + r.add_argument("--item-timeout", type=int, default=240) + s = sub.add_parser("summarize") + s.add_argument("--out", required=True) + a = ap.parse_args() + {"compile": cmd_compile, "run": cmd_run, "summarize": cmd_summarize}[a.cmd](a) + + +if __name__ == "__main__": + main() From 8d4754a0e252eaa39f17fd1b78b29faf02274f84 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 00:08:18 -0400 Subject: [PATCH 10/15] docs: refresh llms-full.txt Co-Authored-By: Claude Opus 5.5 --- llms-full.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/llms-full.txt b/llms-full.txt index 3e28afeddb..e8faf5873b 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -597,6 +597,7 @@ improvising or retrying when something is off. | Capture a recording's network traffic and compile a clean save into an API step, with the same record check | On main ([#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269)) | Synthetic patients on a local OpenEMR test copy | network capture, `ApiBinding` | | Fall back from the API step to page structure when the API is unavailable, before anything is sent | On main ([#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269)) | Synthetic | [#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269) | | Run a form's own script before an API save instead of refusing the form: field handlers, `onsubmit`, submit listeners and custom validate.js rules (OpenEMR's `pastDate`) run on the run's values in a scratch copy of the session with every write blocked, and the save is sent only when the page lets the submit through and would send the same body | On main | Synthetic (headless Chromium against a local test page) | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#what-happens-at-run-time), `runtime/actuators/form_eval.py` | +| Declare the other writes in an API step's span as an ordered sequence: the page's own background requests (another document's script, loaded before the form, to an endpoint the form doesn't name) are listed and not sent; a write a step caused is sent in its recorded place before the save, read back by that step's record check, and a failure stops the run with nothing after it sent and nothing retried | On main | Synthetic | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#other-writes-in-the-span) | | Approve an API step's exact request under per-item approval: one digest binds the request bytes, the record and the record check, and a changed request is not sent | On main ([#1355](https://github.com/OpenAdaptAI/OpenAdapt/pull/1355)) | Synthetic | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#under-an-approval-mode) | | Run an API step in a durable run: checkpoint the span after the save is sent, never resend or redo a save that may have landed, and reconcile it by reading the record back | On main ([#1358](https://github.com/OpenAdaptAI/OpenAdapt/pull/1358)) | Synthetic | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#in-a-durable-run) | | Run an admitted API step under a governed run authorization and the Standard or Regulated profile: the authorization names the API step by digest, the person who admits it names the right-record parameter, and the outcome reads `VERIFIED` | On main ([#1361](https://github.com/OpenAdaptAI/OpenAdapt/pull/1361)) | Synthetic | [API_TIER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/API_TIER.md#under-governance) | From ba02103245df29a4758f907cc92d91a12d624b63 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 01:16:50 -0400 Subject: [PATCH 11/15] flow: write-ahead checkpoint before an API span's first request A durable first leg now persists, before the span's first request leaves, the folded steps' checkpoints and a delivery-uncertain pause at the save. A crash anywhere in the sequence of writes then resumes at the save, which is only reconciled, never sent again. The save's own outcome replaces that pause, or, once its checkpoint is persisted, the pause is consumed (only if it is still exactly the one written). A resumed leg keeps the continuation guard's delivery_started phase as its write-ahead. api admit lists the requests the API step leaves to the page before the person types admit. Co-Authored-By: Claude Opus 5.5 --- packages/flow/openadapt_flow/cli_api.py | 19 +- .../runtime/durable/controller.py | 23 +++ .../flow/openadapt_flow/runtime/replayer.py | 166 ++++++++++++++++-- packages/flow/tests/test_api_step_durable.py | 73 ++++++++ .../tests/test_api_step_write_sequence.py | 22 +++ 5 files changed, 283 insertions(+), 20 deletions(-) diff --git a/packages/flow/openadapt_flow/cli_api.py b/packages/flow/openadapt_flow/cli_api.py index b62a068250..51abfc878a 100644 --- a/packages/flow/openadapt_flow/cli_api.py +++ b/packages/flow/openadapt_flow/cli_api.py @@ -196,13 +196,28 @@ def cmd_api_admit( except (ApiProposalError, ValueError) as exc: print(f"api admit: {exc}", file=sys.stderr) return 2 + ambient = [] for step in updated.steps: if step.id in steps: print(describe_api_step(step)) print() + spec = step.api_binding.web_form if step.api_binding else None + ambient += [w for w in (spec.writes if spec else []) if w.role == "ambient"] + if ambient: + # The person decides that these are the page's own requests: the API + # step won't send them, and admitting says so. + print("The API step will NOT send these requests; they are the page's own:") + for write in ambient: + print(f" {write.method} {write.path}: {write.evidence}") + print() answer = ask( - f"Type '{ADMIT_WORD}' to let these requests save in place of the GUI steps " - "(the record check still confirms every save): " + f"Type '{ADMIT_WORD}' to let these requests save in place of the GUI steps" + + ( + f" and leave the {len(ambient)} request(s) above to the page" + if ambient + else "" + ) + + " (the record check still confirms every save): " ) if answer.strip().lower() != ADMIT_WORD: print("Not admitted. Nothing changed.") diff --git a/packages/flow/openadapt_flow/runtime/durable/controller.py b/packages/flow/openadapt_flow/runtime/durable/controller.py index c760326f18..69e7d85b90 100644 --- a/packages/flow/openadapt_flow/runtime/durable/controller.py +++ b/packages/flow/openadapt_flow/runtime/durable/controller.py @@ -1013,6 +1013,29 @@ def record_program_halt( pass self._sync_authority() + def pause_digest(self) -> str: + """Digest of the pause on disk now (of None when there is none).""" + + return self.store.model_digest(self.store.read_pending()) + + def consume_pause(self, digest: str) -> None: + """Remove the pause on disk only if it is exactly ``digest``. + + A durable API span writes a delivery-uncertain pause before its first + request (its write-ahead). Once the save's checkpoint is persisted, that + pause has done its job. Any other pause (a deferred record check, a + person's decision, another leg's) is left alone. + """ + + current = self.store.read_pending() + if current is None or self.store.model_digest(current) != digest: + return + if self._active_pause_digest != digest: + return + self.store.cas_pending(digest, None) + self._active_pause_digest = self.store.model_digest(None) + self._sync_authority() + def complete(self) -> None: """Consume only the exact pause that this successful leg admitted.""" diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index 98439a05a9..ae5b190218 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -1140,6 +1140,11 @@ def __init__( self.api_steps_off_reason: Optional[str] = None self.web_form_actuator = web_form_actuator self._web_form_span_active = False + #: Set by a durable _run_web_form_span: persists the span's write-ahead + #: checkpoint right before its first request is sent. + self._web_form_write_ahead: Optional[Callable[[], None]] = None + self._web_form_folded_checkpointed = False + self._web_form_write_ahead_pause = "" self._web_form_fallback_reason: Optional[str] = None # (step id, reason) set when a closed-loop scroll used up its budget # because the next step's target never resolved. That step then @@ -2450,6 +2455,7 @@ def acknowledge_delivery(self_nonlocal) -> None: bundle_dir=bundle_dir, run_dir=run_dir, new_crops=new_crops, + durable_run=durable_run, ) if batch is None: why_not = self._web_form_fallback_reason @@ -2475,6 +2481,11 @@ def acknowledge_delivery(self_nonlocal) -> None: # so a resume runs the whole span again. save_result = batch[-1] checkpoint_folded = save_result.ok or save_result.delivery_attempted + # The write-ahead checkpoint (before the first request) already + # holds the folded steps. + already = len(batch) > 1 and getattr( + self, "_web_form_folded_checkpointed", False + ) for offset, result in enumerate(batch): folded = offset < len(batch) - 1 stop_run = self._record_linear_result( @@ -2485,11 +2496,22 @@ def acknowledge_delivery(self_nonlocal) -> None: step=workflow.steps[step_index + offset], params=params, durable_run=( - None if folded and not checkpoint_folded else durable_run + None + if folded and (already or not checkpoint_folded) + else durable_run ), ) if stop_run: break + if ( + already + and durable_run is not None + and save_result.ok + and not stop_run + ): + # The save's own checkpoint is persisted: the write-ahead + # pause has done its job (unless something replaced it). + durable_run.consume_pause(self._web_form_write_ahead_pause) if stop_run: break @@ -2935,6 +2957,7 @@ def _run_web_form_span( bundle_dir: Path, run_dir: Path, new_crops: dict[str, bytes], + durable_run: Optional[Any] = None, ) -> Optional[list[StepResult]]: """Run the API step for steps ``start_index..end_index``. @@ -2946,23 +2969,6 @@ def _run_web_form_span( """ save = workflow.steps[end_index] - self._web_form_span_active = True - try: - result = self._run_step( - save, - workflow=workflow, - step_index=end_index, - params=params, - bundle_dir=bundle_dir, - run_dir=run_dir, - new_crops=new_crops, - ) - finally: - self._web_form_span_active = False - self._web_form_approval.clear() - if result.api_fallback is not None: - self._web_form_fallback_reason = result.api_fallback - return None router = getattr(self, "_surface_router", None) surface = router.active_id if router is not None else None folded = [ @@ -2980,6 +2986,81 @@ def _run_web_form_span( ) for s in workflow.steps[start_index:end_index] ] + # A resumed leg already has its write-ahead: the continuation guard + # moves the attempt to delivery_started before the first request, and + # a restart then reconciles. A first leg gets one here. + if durable_run is not None and self._durable_continuation_guard is None: + + def write_ahead() -> None: + # What a resume reads if the process stops while the span's + # requests are going out: the folded steps are done, and the + # save is paused as delivery-uncertain. A resume then starts at + # the save and only reconciles it; it never sends a write of + # the sequence again. The save's own outcome replaces this + # pause, and the run's completion consumes it. + observations = router.observations if router is not None else None + self._web_form_folded_checkpointed = True + for offset, item in enumerate(folded): + durable_run.record( + start_index + offset, + workflow.steps[start_index + offset], + item, + dict(params), + workflow=workflow, + active_surface=surface, + surface_observations=observations, + ) + provisional = StepResult( + step_id=save.id, + intent=save.intent, + ok=False, + safety_halt=True, + failure_category="safety_halt", + actuation="api", + delivery_attempted=True, + surface=surface, + risk=save.risk, + risk_explanation=save.risk_explanation, + risk_review_required=save.risk_review_required, + error=( + f"Step '{save.id}' ({save.intent}): the API step began " + "sending its requests and the run stopped before it " + "finished; they may have written. Resume never sends " + "them again; reconcile by reading the records back" + ), + ) + durable_run.record( + end_index, + save, + provisional, + dict(params), + workflow=workflow, + active_surface=surface, + surface_observations=observations, + ) + self._web_form_write_ahead_pause = durable_run.pause_digest() + + self._web_form_write_ahead = write_ahead + self._web_form_folded_checkpointed = False + self._web_form_write_ahead_pause = "" + self._web_form_span_active = True + try: + result = self._run_step( + save, + workflow=workflow, + step_index=end_index, + params=params, + bundle_dir=bundle_dir, + run_dir=run_dir, + new_crops=new_crops, + ) + finally: + self._web_form_span_active = False + self._web_form_write_ahead = None + self._web_form_approval.clear() + if result.api_fallback is not None: + self._web_form_fallback_reason = result.api_fallback + return None return [*folded, result] def _lookup_capability_refusal( @@ -9573,6 +9654,55 @@ def _try_api_tier( # request delivery, not against state captured earlier during # authorization or verifier setup. start_state = self._structural_state() + # A durable API span persists its write-ahead checkpoint (the folded + # steps, delivery attempted) before its first request leaves, so a + # crash anywhere in its sequence of writes resumes at the save, which + # is only ever reconciled, never sent again (_run_web_form_span). + write_ahead = getattr(self, "_web_form_write_ahead", None) + if ( + write_ahead is not None + and send_prepared is None + and callable(getattr(api_actuator, "prepare", None)) + and callable(getattr(api_actuator, "send", None)) + ): + # Prepare first (reads only), so a save that can't be sent falls + # back to the GUI without a write-ahead checkpoint. + from openadapt_flow.runtime.actuators.api import ( + ApiActuationResult as _Receipt, + ) + + try: + early = api_actuator.prepare(binding, runtime_params_for_gui(params)) + except Exception as exc: # noqa: BLE001 - reads only, nothing sent + early = None + result.ok = False + result.error = ( + f"Step '{step.id}' ({step.intent}): preparing the API request " + f"failed ({type(exc).__name__}); nothing was sent" + ) + return True + if isinstance(early, _Receipt): + # UNAVAILABLE: nothing was sent, as on the actuate path below. + result.effect_results.append(f"[api] {early.reason}") + del result.effect_contract_hashes[api_hash_start:] + if binding.on_unavailable == "halt": + self._set_api_unavailable_refusal(step, result, reason=early.reason) + return True + return False + send_prepared = early + if write_ahead is not None and send_prepared is not None: + try: + write_ahead() + except Exception as exc: # noqa: BLE001 - nothing has been sent + result.ok = False + result.safety_halt = True + result.failure_category = "safety_halt" + result.error = ( + f"Step '{step.id}' ({step.intent}): the durable run could not " + f"persist its write-ahead checkpoint ({type(exc).__name__}); " + "nothing was sent" + ) + return True # This transition is the explicit delivery boundary. Set it BEFORE the # call because an exception/timeout may occur after the request left the # process but before the actuator can return a receipt-like outcome. diff --git a/packages/flow/tests/test_api_step_durable.py b/packages/flow/tests/test_api_step_durable.py index 7e7d9ebf27..f70743a07e 100644 --- a/packages/flow/tests/test_api_step_durable.py +++ b/packages/flow/tests/test_api_step_durable.py @@ -379,3 +379,76 @@ def test_an_identity_checked_save_with_a_right_record_contract_runs_durably(tmp_ assert [c.actuation for c in fx.store().checkpoints()] == ["api_folded", "api"] finally: fx.stop() + + +class Crash(BaseException): + """The process dies: nothing after this point runs in this leg.""" + + +class WritesOnceThenDies(ScriptedActuator): + """The span's first write lands, then the process dies before the save. + + Prepares and sends like the live-session actuator, so the replayer writes + the span's write-ahead checkpoint between the two. + """ + + def prepare(self, binding, params): + return object() + + def send(self, binding, prepared): + return self.actuate(binding, {}) + + def actuate(self, binding, params): + self.calls += 1 + requests.post( + f"{self.sor_url}/api/encounter", + json={**TARGET, "note": "write 1"}, + timeout=5, + ) + raise Crash() + + +def test_a_crash_between_the_first_write_and_the_save_is_reconciled_not_resent(fx): + first = WritesOnceThenDies(fx.url, ActuationStatus.ACTUATED) + with pytest.raises(Crash): + fx.run(first) + assert first.calls == 1 and [r["note"] for r in fx.records()] == ["write 1"] + + # The write-ahead checkpoint was persisted before the first request: the + # folded steps are done and the save is paused as delivery-uncertain. + store = fx.store() + assert [c.actuation for c in store.checkpoints()] == ["api_folded"] + pending = store.read_pending() + assert pending is not None and pending.step_id == "step_001" + assert pending.resume_from_index == 1 and pending.delivery_uncertainty is not None + assert "began sending its requests" in pending.reason + + # An ordinary resume can't repeat a delivery that may have landed. + second = ScriptedActuator(fx.url, ActuationStatus.ACTUATED) + with pytest.raises(ApprovalRequired, match="may already have actuated"): + resume(fx.run_dir, fx.replayer(second), approval=fx.approval()) + # Nor can one that authorizes an uncertain retry: nothing is sent again. + resumed = resume( + fx.run_dir, + fx.replayer(second), + approval=fx.approval(authorize_uncertain_retry=True), + ) + assert second.calls == 0 and fx.backend.actions == [] + assert ( + resumed.success is False and "never sends it again" in resumed.results[-1].error + ) + assert [r["note"] for r in fx.records()] == ["write 1"] + + +class PreparesThenSaves(ScriptedActuator): + def prepare(self, binding, params): + return object() + + def send(self, binding, prepared): + return self.actuate(binding, {}) + + +def test_a_finished_durable_span_leaves_no_provisional_pause(fx): + report = fx.run(PreparesThenSaves(fx.url, ActuationStatus.ACTUATED)) + assert report.success is True + assert fx.store().read_pending() is None diff --git a/packages/flow/tests/test_api_step_write_sequence.py b/packages/flow/tests/test_api_step_write_sequence.py index 85e7d8ded0..7f323b1752 100644 --- a/packages/flow/tests/test_api_step_write_sequence.py +++ b/packages/flow/tests/test_api_step_write_sequence.py @@ -269,3 +269,25 @@ def test_a_step_write_the_api_step_cant_repeat_refuses_the_proposal( ): _wf, step, reasons = propose_api_step(_workflow(), _rec(tmp_path, write)) assert step is None and why in " ".join(reasons) + + +def test_api_admit_shows_the_requests_left_to_the_page_before_the_person_types( + tmp_path, capsys +): + import argparse + + from openadapt_flow.cli_api import cmd_api_admit + from openadapt_flow.ir import Workflow + + wf, _step, _ = propose_api_step(_workflow(), _rec(tmp_path, SHELL, BACKGROUND)) + bundle = tmp_path / "bundle" + wf.save(bundle) + prompts: list[str] = [] + args = argparse.Namespace(bundle=str(bundle), step=None, by="reviewer", identity=[]) + code = cmd_api_admit( + args, ask=lambda p: prompts.append(p) or "admit", interactive=lambda: True + ) + out = capsys.readouterr().out + assert code == 0 and Workflow.load(bundle).steps[-1].api_binding.admitted() + assert "will NOT send these requests" in out and "POST /library/ajax/bg.php" in out + assert "leave the 1 request(s) above to the page" in prompts[0] From 67b7c3b6ea1da241418cfa91b0dcefd0f64d3066 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 01:51:23 -0400 Subject: [PATCH 12/15] flow: a step write is sent only if the page's own script sends it Before each save, the page's script runs on the run's values in the scratch page; each declared step write must be one it sent there (blocked), with the same body, or the GUI steps run. A span with step writes and no page script to check them against stays on the GUI. Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/API_TIER.md | 18 ++++- .../runtime/actuators/form_eval.py | 29 +++++++- .../runtime/actuators/web_form.py | 72 +++++++++++++------ .../tests/test_api_step_write_sequence.py | 28 +++++++- 4 files changed, 123 insertions(+), 24 deletions(-) diff --git a/packages/flow/docs/API_TIER.md b/packages/flow/docs/API_TIER.md index 16a1b7eb30..56b3503957 100644 --- a/packages/flow/docs/API_TIER.md +++ b/packages/flow/docs/API_TIER.md @@ -135,7 +135,10 @@ proposal (`web_form.writes`), in recorded order, with one of two roles: values and unchanged form defaults from the live form), in its recorded place before the save. A write whose body isn't form-encoded, that carries values in its URL or another value, that went to another origin, or that - wasn't accepted when recorded refuses the proposal. Each step write is + wasn't accepted when recorded refuses the proposal. Before each save the + page's own script must send each step write itself, with the same body, + when it runs on the run's values in the scratch page (the request is + blocked there); otherwise the GUI steps run. Each step write is read back by the record check declared on the step it was sent during: admission refuses one whose step has none, and the binding's record check is the save's followed by those steps' checks. If a write fails or answers @@ -146,6 +149,19 @@ proposal (`web_form.writes`), in recorded order, with one of two roles: A write that fits neither role keeps the save on the GUI. +In a durable run, the span persists a write-ahead checkpoint before its +first request leaves: the folded steps' checkpoints and a +delivery-uncertain pause at the save. If the process stops anywhere in the +sequence, a resume starts at the save, which is only reconciled (the +attended reconcile action reads the records back) and never sent again, +and no write of the sequence is resent. The save's own outcome replaces +that pause; once its checkpoint is persisted, the pause is removed (only if +it is still exactly the one written). A resumed leg uses the continuation +guard's `delivery_started` phase as its write-ahead. + +`api admit` lists each request the API step leaves to the page, with the +evidence, before the person types `admit`. + ## What happens at run time The API step uses the replaying browser's own session: its requests go diff --git a/packages/flow/openadapt_flow/runtime/actuators/form_eval.py b/packages/flow/openadapt_flow/runtime/actuators/form_eval.py index f9618e4e1e..8003307099 100644 --- a/packages/flow/openadapt_flow/runtime/actuators/form_eval.py +++ b/packages/flow/openadapt_flow/runtime/actuators/form_eval.py @@ -158,6 +158,11 @@ class FormEvaluation: #: Requests the page tried to send and were blocked, ``METHOD /path``. blocked: list[str] = field(default_factory=list) seconds: float = 0.0 + #: Declared step writes the page's script sent (and the route blocked): + #: ``(METHOD /path, body pairs or None)``. + writes: list[tuple[str, Optional[list[tuple[str, str]]]]] = field( + default_factory=list + ) @property def valid(self) -> bool: @@ -177,6 +182,7 @@ def evaluate_form( *, submitter: Optional[str] = None, submitter_requests: tuple[str, ...] = (), + step_writes: tuple[str, ...] = (), timeout_s: float = 20.0, settle_ms: int = 250, ) -> FormEvaluation: @@ -291,9 +297,28 @@ def on_dialog(d: Any) -> None: ) except Exception as exc: # noqa: BLE001 - e.g. the page navigated away result = {"navigated": type(exc).__name__} - return _verdict( - result, late, errors, dialogs, action_path, set(submitter_requests), done + verdict = _verdict( + result, + [b for b in late if b["what"] not in step_writes], + errors, + dialogs, + action_path, + set(submitter_requests), + done, ) + # The step writes the page's script sent (blocked), with their bodies. + verdict.writes = [ + ( + b["what"], + up.parse_qsl(b["body"], keep_blank_values=True) + if b["body"] is not None + and "x-www-form-urlencoded" in b["content_type"] + else None, + ) + for b in late + if b["what"] in step_writes + ] + return verdict except Exception as exc: # noqa: BLE001 - a failure to run is unevaluable return done("unevaluable", f"the scratch page failed ({type(exc).__name__})") finally: diff --git a/packages/flow/openadapt_flow/runtime/actuators/web_form.py b/packages/flow/openadapt_flow/runtime/actuators/web_form.py index 18567b581a..00e8fa31a1 100644 --- a/packages/flow/openadapt_flow/runtime/actuators/web_form.py +++ b/packages/flow/openadapt_flow/runtime/actuators/web_form.py @@ -532,6 +532,7 @@ def evaluate_form( *, submitter: Optional[str] = None, submitter_requests: tuple[str, ...] = (), + step_writes: tuple[str, ...] = (), ) -> Any: from openadapt_flow.runtime.actuators.form_eval import evaluate_form @@ -543,6 +544,7 @@ def evaluate_form( fields, submitter=submitter, submitter_requests=submitter_requests, + step_writes=step_writes, timeout_s=self._timeout_ms / 1000.0, ) @@ -814,8 +816,34 @@ def unavailable(reason: str) -> ApiActuationResult: body, problem = self.build_body(spec, params, controls) if problem: return unavailable(problem) + writes: list[PreparedWrite] = [] + for write in spec.writes: + if write.role != "step": + continue # the page's own request: not the API step's to send + sub = spec.model_copy(update={"fields": write.fields}) + wbody, problem = self.build_body( + sub, params, controls, enforce_rules=False + ) + if problem: + return unavailable(f"{write.method} {write.path}: {problem}") + writes.append( + PreparedWrite( + method=write.method, + path=fill_url(write.path, params), + body=tuple(wbody), + expected_status=tuple(write.expected_status), + from_step=write.from_step or "", + ) + ) + if writes and not scripts: + return unavailable( + "the API step would send writes the page's script sends, but " + "the page has no script to check them against" + ) if scripts: - problem = self._run_page_script(spec, form_path, action_path, body) + problem = self._run_page_script( + spec, form_path, action_path, body, writes + ) if problem: return unavailable(problem) for check in spec.prechecks: @@ -838,25 +866,6 @@ def unavailable(reason: str) -> ApiActuationResult: f"the check before saving ({check.from_step or check.path}) did not pass: " f"missing {missing!r}" ) - writes: list[PreparedWrite] = [] - for write in spec.writes: - if write.role != "step": - continue # the page's own request: not the API step's to send - sub = spec.model_copy(update={"fields": write.fields}) - wbody, problem = self.build_body( - sub, params, controls, enforce_rules=False - ) - if problem: - return unavailable(f"{write.method} {write.path}: {problem}") - writes.append( - PreparedWrite( - method=write.method, - path=fill_url(write.path, params), - body=tuple(wbody), - expected_status=tuple(write.expected_status), - from_step=write.from_step or "", - ) - ) except KeyError as exc: return unavailable(f"the run has no value for {exc}") except Exception as exc: # noqa: BLE001 - only reads were sent @@ -878,6 +887,7 @@ def _run_page_script( form_path: str, action_path: str, body: list[tuple[str, str]], + writes: list["PreparedWrite"] = (), # type: ignore[assignment] ) -> Optional[str]: """Run the form's own script on the run's values; why not to send, or None. @@ -899,6 +909,7 @@ def _run_page_script( [(k, v) for k, v in body if k in typed], submitter=spec.submitter, submitter_requests=tuple(spec.submitter_requests), + step_writes=tuple(f"{w.method} {up.urlsplit(w.path).path}" for w in writes), ) self.log.append( { @@ -920,6 +931,27 @@ def _run_page_script( "the page would send other values than the API step built " f"({', '.join(changed[:5])}); its script changes them" ) + # Each write the API step sends before the save must be one the + # page's own script sent in the scratch run, with the same body. + sent = list(getattr(result, "writes", None) or []) + for write in writes: + what = f"{write.method} {up.urlsplit(write.path).path}" + want_w = sorted((k, v) for k, v in write.body if k not in session) + match = next( + ( + b + for w, b in sent + if w == what + and b is not None + and sorted((k, v) for k, v in b if k not in session) == want_w + ), + None, + ) + if match is None: + return ( + f"the page's script didn't send {what} with the body the API step " + "built when it ran on these values" + ) return None def send(self, binding: Any, prepared: "PreparedSave") -> ApiActuationResult: diff --git a/packages/flow/tests/test_api_step_write_sequence.py b/packages/flow/tests/test_api_step_write_sequence.py index 7f323b1752..eea2249a15 100644 --- a/packages/flow/tests/test_api_step_write_sequence.py +++ b/packages/flow/tests/test_api_step_write_sequence.py @@ -170,9 +170,27 @@ def _admitted_sequence(tmp_path): class SequenceTransport(FakeTransport): - def __init__(self, write_status=200, raise_on_write=False): + def __init__(self, write_status=200, raise_on_write=False, page_sends=True): super().__init__() self.write_status, self.raise_on_write = write_status, raise_on_write + self.page_sends = page_sends + + def evaluate_form(self, form_path, action_path, fields, **kw): + # The page's own script posts the draft as the person types (blocked + # in the scratch run); the actuator compares it with the one it built. + result = super().evaluate_form(form_path, action_path, fields, **kw) + self.step_writes = kw.get("step_writes") + if self.page_sends: + result.writes = [ + ( + "POST /app/draft.php", + [ + ("csrf_token_form", "scratch-token"), + ("form_lname", dict(fields)["form_lname"]), + ], + ) + ] + return result def post_form(self, path, body, *, referer): if path == "/app/draft.php": @@ -291,3 +309,11 @@ def test_api_admit_shows_the_requests_left_to_the_page_before_the_person_types( assert code == 0 and Workflow.load(bundle).steps[-1].api_binding.admitted() assert "will NOT send these requests" in out and "POST /library/ajax/bg.php" in out assert "leave the 1 request(s) above to the page" in prompts[0] + + +def test_a_step_write_the_page_wouldnt_send_keeps_the_save_on_the_gui(tmp_path): + t = SequenceTransport(page_sends=False) + out = WebFormActuator(t).actuate(_admitted_sequence(tmp_path), RUN) + assert out.status is ActuationStatus.UNAVAILABLE + assert "didn't send POST /app/draft.php" in out.reason + assert t.posts == [] and t.step_writes == ("POST /app/draft.php",) From 10f2f0978036bce327643d291c184208fda26642 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 02:24:35 -0400 Subject: [PATCH 13/15] flow: run the form's script with real input, and require a real submit Second review of the scratch run: - route every page of the scratch context (a popup is unevaluable and is closed), close every WebSocket, stub EventSource, RTCPeerConnection, sendBeacon and window.open; - type the values with real key events so key handlers run (fields not on screen get the same events from script), and click with real, trusted clicks; - a recorded read now completes, the recorded confirm control is clicked in whichever frame has it, and the page must then submit; a read alone no longer counts; - a page that submits more than once is unevaluable; after load, an image request that isn't a static file counts as the page sending. Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/API_TIER.md | 53 +- .../openadapt_flow/compiler/api_proposal.py | 21 +- packages/flow/openadapt_flow/ir.py | 5 + .../runtime/actuators/form_eval.py | 493 +++++++++++------- .../runtime/actuators/web_form.py | 14 +- .../flow/tests/test_api_step_page_script.py | 128 ++++- 6 files changed, 479 insertions(+), 235 deletions(-) diff --git a/packages/flow/docs/API_TIER.md b/packages/flow/docs/API_TIER.md index 33bd8654b0..1cbeffd980 100644 --- a/packages/flow/docs/API_TIER.md +++ b/packages/flow/docs/API_TIER.md @@ -138,30 +138,39 @@ Before it sends the save, the API step: submits: native HTML constraints (`required`, `minlength`, `maxlength`, `pattern`, `min`, `max`, and the `email`, `url`, `number` and `date` input types) and the page's validate.js rules; -5. when the page has any script, **runs it** (`runtime/actuators/form_eval.py`): +5. when the page has any script, **runs it** (`runtime/actuators/form_eval.py`), + the way the person's steps would: - it opens the form page in a scratch copy of the live session: a separate browser context with the session's cookies and no service - workers. While the page loads, only same-origin GET requests go - through; once it has loaded, every request is blocked, so nothing the - page does there can write or reach the session; - - it puts the run's typed and picked values into the form with the same - `input`, `change` and `blur` events a browser fires; - - it starts the submit as the person did: it clicks the control the - person clicked (`submitter`, from the recording). When the recording - names none it submits the form, and a form whose buttons have click - handlers is refused; - - the page must then submit the form, or make a request the person's - click made in the recording (`submitter_requests`; on OpenEMR, the - duplicate search). The body compared is the one the page submits, - taken at the moment it submits (a last `submit` listener, the form's - own `submit()`, or the browser's POST, which the route blocks); when - the page goes on to a recorded request instead, it is the form as the - page left it. Declared validate.js `constraints` run too; - - the save goes ahead only when the page submitted or went on as - recorded, raised no error, showed no dialog, tried to send nothing - else, and the body equals the one the API step built (session values - aside). Any difference, and anything else, is a reason, and the GUI - steps run; + workers. Every request of every page in it goes through one route, + every WebSocket is closed, and EventSource, RTCPeerConnection, + `sendBeacon` and `window.open` do nothing. While the page loads, only + same-origin GET requests go through. Once it has loaded, everything is + blocked except same-origin scripts and stylesheets and the reads the + person's click made in the recording (`submitter_requests`; on OpenEMR, + the duplicate search), so nothing the page does there can write. A + popup or new tab makes the run unevaluable; + - it types the run's typed and picked values with real key events, so + key handlers (`onkeyup`, `onchange`) run as they do for the person. A + field that isn't on screen when the form opens (on a tab the person + opened first) gets the same events key by key from script; its + handlers run, but those events aren't trusted; + - it clicks the control the person clicked (`submitter`) with a real, + trusted click. When the recording names none it submits the form, and + a form whose buttons have click handlers is refused. When the click + shows a dialog from the recorded reads, it clicks the control the + person clicked to confirm (`confirm`; on OpenEMR, `#confirmCreate`), + in whichever frame has it; + - the page must then submit the form, exactly once: a last `submit` + listener sees it go through, the form's own `submit()` is called, or + the browser's POST reaches the route (blocked). The body compared is + the one the page submits, taken when it submits. Declared validate.js + `constraints` run too; + - the save goes ahead only when the page submitted once, raised no + error, showed no dialog, opened no window or socket, tried to send + nothing else, and the body equals the one the API step built (session + values aside). Any difference, and anything else, is a reason, and the + GUI steps run; 6. runs the pre-checks. Each of these refusals means the save wasn't sent, so the GUI steps run. diff --git a/packages/flow/openadapt_flow/compiler/api_proposal.py b/packages/flow/openadapt_flow/compiler/api_proposal.py index 5e690c606c..3363a4ccfc 100644 --- a/packages/flow/openadapt_flow/compiler/api_proposal.py +++ b/packages/flow/openadapt_flow/compiler/api_proposal.py @@ -272,6 +272,17 @@ def _in_page(selector: str, html: str) -> bool: return bool(kind) and kind.group(1).lower() in ("submit", "button", "image") +def _selector(step: Step, events: list[dict[str, Any]]) -> Optional[str]: + """The CSS selector a recorded click targeted, when it has one.""" + + if step.anchor is not None and step.anchor.structural is not None: + return step.anchor.structural.selector + i = _event_index(step) + if i is not None and i < len(events): + return (events[i].get("structural") or {}).get("selector") + return None + + def _submitter( folded: list[Step], events: list[dict[str, Any]], @@ -291,11 +302,7 @@ def _submitter( if step.action not in (ActionKind.CLICK, ActionKind.DOUBLE_CLICK): continue i = _event_index(step) - selector = None - if step.anchor is not None and step.anchor.structural is not None: - selector = step.anchor.structural.selector - elif i is not None and i < len(events): - selector = (events[i].get("structural") or {}).get("selector") + selector = _selector(step, events) if not selector or not _in_page(selector, form_html) or i is None: continue lo = float(events[i - 1]["t"]) if 0 < i <= len(events) else float("-inf") @@ -663,6 +670,9 @@ def carried(line: dict[str, Any]) -> int: submitter, submitter_requests = _submitter( folded, events, lines, form_line["response_text"], post ) + confirm = _selector(save, events) if submitter is not None else None + if confirm == submitter: + confirm = None spec = WebFormSpec( form_path=form_path, fields=fields, @@ -671,6 +681,7 @@ def carried(line: dict[str, Any]) -> int: client_scripts=client_scripts, submitter=submitter, submitter_requests=submitter_requests, + confirm=confirm, prechecks=prechecks, success_pattern=_success_pattern(post), replaces_steps=[s.id for s in folded], diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index 640be0d375..8c00447ea5 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -710,6 +710,11 @@ class WebFormSpec(BaseModel): #: requests. submitter: Optional[str] = None submitter_requests: list[str] = Field(default_factory=list) + #: The control the person clicked to confirm the save when it isn't the + #: submitter (for example OpenEMR's ``#confirmCreate`` in the duplicate + #: check dialog). Running the page's script clicks it too, in whichever + #: frame has it, and the page must then submit the form. + confirm: Optional[str] = None prechecks: list[WebFormPrecheck] = Field(default_factory=list) #: Regular expression the save response must match to count as accepted. success_pattern: Optional[str] = None diff --git a/packages/flow/openadapt_flow/runtime/actuators/form_eval.py b/packages/flow/openadapt_flow/runtime/actuators/form_eval.py index f9618e4e1e..6577c8d237 100644 --- a/packages/flow/openadapt_flow/runtime/actuators/form_eval.py +++ b/packages/flow/openadapt_flow/runtime/actuators/form_eval.py @@ -1,34 +1,39 @@ """Run a form's own script on the run's values, with every write blocked. The ``web_form`` API step sends one request in place of the clicks that fill -and submit a form. Most forms run script on the way: field ``onchange`` -handlers, an ``onsubmit`` validator, a click handler on the save button, a -page listener for ``submit``, custom validate.js rules such as OpenEMR's -``pastDate``. The API step can't read what that script does, so it runs it, -before each save: - -1. open the form page in a scratch copy of the live session (a separate - browser context with the session's cookies, service workers blocked); +and submit a form. Most forms run script on the way: key and change +handlers on the fields, an ``onsubmit`` validator, a click handler on the +save button, a page listener for ``submit``, custom validate.js rules such +as OpenEMR's ``pastDate``. The API step can't read what that script does, +so it runs it, before each save, the way the person's steps would: + +1. open the form page in a scratch copy of the live session: a separate + browser context with the session's cookies, no service workers, every + request of every page in it routed, every WebSocket closed, and + EventSource and RTCPeerConnection stubbed out; 2. while the page loads, let only same-origin GET requests through (no - other origin at all); once it has loaded, block every request, so - nothing the page does here can write or reach the session; -3. put the run's values into the fields the person typed or picked, the way - a browser does (``input`` and ``change`` events, then ``blur``); -4. start the submit the way the person did: click the recorded control - (``submitter``), or, when the recording names none, submit the form; a - form whose buttons have click handlers and no recorded control is - refused; -5. see what the page does: it submits the form (the submit event reaches a - last listener, ``form.submit()`` is called, or the browser's own POST to - the save reaches the blocked route), or it makes a request the person's - click made in the recording (``submitter_requests``, such as OpenEMR's - duplicate search). The body compared is the one the page submits, taken - at the moment it submits; a request from the recording counts with the - form as the page left it. Where the page declares validate.js - ``constraints``, they run too. - -The result is ``valid`` only when the page submitted or went on as -recorded, raised no error, showed no dialog, and tried nothing else. + other origin at all). Once it has loaded, block every request except + same-origin scripts and stylesheets and the reads the person's click + made in the recording (``submitter_requests``, such as OpenEMR's + duplicate search), so nothing the page does can write; +3. type the run's values into the fields the person typed or picked, with + real key events (``keydown``, ``keyup``, ``input``), so key handlers run + as they do for the person. A field that isn't on screen when the form + opens (on a tab the person opened first) gets the same events key by + key from script: its handlers run, but those events aren't trusted; +4. click the control the person clicked (``submitter``) with a real + click; when the recording names none, submit the form, and refuse a form + whose buttons have click handlers. When the page then shows the recorded + reads (for example a confirm dialog), click the control the person + clicked to confirm (``confirm``), in whichever frame has it; +5. the page must then submit the form: a last ``submit`` listener sees it + go through, the form's own ``submit()`` is called, or the browser's POST + reaches the route (blocked). The body compared is the one the page + submits, taken when it submits. Declared validate.js ``constraints`` + run too. + +The result is ``valid`` only when the page submitted exactly once, raised no +error, showed no dialog, opened no window or socket, and tried nothing else. Anything else is a reason for the GUI steps to run instead. Comparing the body with the one the API step built is the caller's job (:meth:`WebFormActuator.prepare`); any difference keeps the save on the GUI. @@ -37,24 +42,36 @@ from __future__ import annotations +import json +import re import time import urllib.parse as up from dataclasses import dataclass, field from typing import Any, Optional #: Before any page script: what a form's own ``submit()`` would send is -#: recorded instead of navigating, window.open opens nothing, and every -#: click listener's target is noted. +#: recorded instead of navigating; window.open, WebSocket, EventSource and +#: RTCPeerConnection do nothing but say they were used; every click +#: listener's target is noted. _INSTRUMENT_JS = """ (() => { const pairs = (f, s) => Array.from(s ? new FormData(f, s) : new FormData(f)).map( ([k, v]) => [k, typeof v === 'string' ? v : '']); - const state = { programmatic: [], opened: [], clickTargets: [], pairs }; + const state = { programmatic: [], channels: [], clickTargets: [], pairs, + submits: [], reached: 0, prevented: 0 }; Object.defineProperty(window, '__openadaptFormEval', { value: state }); HTMLFormElement.prototype.submit = function () { try { state.programmatic.push(pairs(this)); } catch (e) { state.programmatic.push(null); } }; - window.open = function (url) { state.opened.push(String(url || '')); return null; }; + window.open = function (url) { state.channels.push('window.open ' + String(url || '')); return null; }; + for (const name of ['WebSocket', 'EventSource', 'RTCPeerConnection', 'webkitRTCPeerConnection']) { + if (name in window) { + window[name] = function () { state.channels.push(name); throw new Error(name + ' is blocked'); }; + } + } + if (navigator.sendBeacon) { + navigator.sendBeacon = function (url) { state.channels.push('sendBeacon ' + String(url)); return false; }; + } const add = EventTarget.prototype.addEventListener; EventTarget.prototype.addEventListener = function (type, fn, opts) { if (['click', 'mousedown', 'mouseup', 'pointerdown', 'pointerup'].includes(type)) { @@ -65,81 +82,67 @@ })(); """ -_EVALUATE_JS = """ -async ({ actionPath, leaf, fields, settleMs, submitter }) => { +#: Finds the form, marks it for the locators, and adds the last submit +#: listener (it records each submit that gets through, then stops it). +_SETUP_JS = """ +({ actionPath, leaf }) => { const state = window.__openadaptFormEval; if (!state) return { unevaluable: 'the scratch page could not be instrumented' }; - const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); const form = Array.from(document.forms).find((f) => { const p = new URL(f.getAttribute('action') || location.href, location.href).pathname; return p === actionPath || (leaf && p.split('/').pop() === leaf); }); if (!form) return { unevaluable: 'the scratch page has no form that posts to the save' }; - for (const [name, value] of fields) { - const el = form.elements.namedItem(name); - if (!el || typeof el.tagName !== 'string') { - return { unevaluable: `${name} is not one control in the form` }; - } - const kind = (el.type || '').toLowerCase(); - if (kind === 'checkbox' || kind === 'radio' || kind === 'file') { - return { unevaluable: `${name} is a ${kind} the API step doesn't fill` }; - } - if (el.tagName === 'SELECT' && !Array.from(el.options).some((o) => o.value === value)) { - return { unevaluable: `${name} has no option with value ${JSON.stringify(value)}` }; - } - if (typeof el.focus === 'function') el.focus(); - el.value = value; - el.dispatchEvent(new Event('input', { bubbles: true })); - el.dispatchEvent(new Event('change', { bubbles: true })); - if (typeof el.blur === 'function') el.blur(); - await sleep(0); - } - await sleep(settleMs); + form.setAttribute('data-openadapt-eval', '1'); + window.addEventListener('submit', (e) => { + if (e.target !== form) return; + state.reached += 1; + if (e.defaultPrevented) { state.prevented += 1; return; } + try { state.submits.push(state.pairs(form, e.submitter || null)); } catch (x) { state.submits.push(state.pairs(form)); } + e.preventDefault(); + }); + return {}; +} +""" + +_PRESUBMIT_JS = """ +() => { + const state = window.__openadaptFormEval; + const form = document.querySelector('form[data-openadapt-eval]'); const invalid = form.noValidate ? [] : Array.from(form.elements) .filter((e) => e.willValidate && !e.checkValidity()) .map((e) => `${e.name || e.id}: ${e.validationMessage}`); + const listened = new Set(state.clickTargets); const buttons = Array.from(form.querySelectorAll( 'button, input[type=submit], input[type=button], input[type=image]')); - const listened = new Set(state.clickTargets); - const buttonScript = buttons.some((b) => b.hasAttribute('onclick') || listened.has(b)); - let reached = false, prevented = null, submitted = null, threw = null, clicked = false; - const last = (e) => { - if (e.target !== form) return; - reached = true; - prevented = e.defaultPrevented; - try { submitted = state.pairs(form, e.submitter || null); } catch (x) { submitted = state.pairs(form); } - e.preventDefault(); - }; - const before = state.programmatic.length; - window.addEventListener('submit', last); - try { - if (submitter) { - const el = document.querySelector(submitter); - if (!el) return { unevaluable: `the scratch page has no ${submitter} to click` }; - clicked = true; - el.click(); - } else if (buttonScript) { - return { unevaluable: 'a button of the form runs script on click and the recording names no control to click' }; - } else if (!invalid.length) { - form.requestSubmit(); - } - } catch (e) { threw = String(e && e.message || e); } - await sleep(settleMs); - window.removeEventListener('submit', last); - let rules = null; - if (typeof window.validate === 'function' && window.constraints && typeof window.constraints === 'object') { + return { invalid, buttonScript: buttons.some((b) => b.hasAttribute('onclick') || listened.has(b)) }; +} +""" + +_COLLECT_JS = """ +() => { + const state = window.__openadaptFormEval; + const form = document.querySelector('form[data-openadapt-eval]'); + let rules = null, threw = null; + if (form && typeof window.validate === 'function' && window.constraints && typeof window.constraints === 'object') { try { const values = typeof window.validate.collectFormValues === 'function' ? window.validate.collectFormValues(form) : Object.fromEntries(new FormData(form)); rules = window.validate(values, window.constraints) || null; - } catch (e) { threw = threw || `validate.js: ${String(e && e.message || e)}`; } + } catch (e) { threw = `validate.js: ${String(e && e.message || e)}`; } } - return { invalid, reached, prevented, submitted, threw, rules, clicked, - programmatic: state.programmatic.slice(before), after: state.pairs(form), - opened: state.opened.slice() }; + return { submits: state.submits, programmatic: state.programmatic, reached: state.reached, + prevented: state.prevented, channels: state.channels, rules, threw }; } """ +#: Files a page loads as pictures or fonts; any other image request after +#: load (``new Image().src = '/track.php?...'``) counts as the page sending. +_STATIC = re.compile(r"\.(png|jpe?g|gif|svg|webp|ico|bmp|woff2?|ttf|otf|eot)$", re.I) + +#: How long a real click or keystroke may wait for its element. +_ACT_TIMEOUT_MS = 5000 + @dataclass class FormEvaluation: @@ -149,11 +152,10 @@ class FormEvaluation: #: ``unevaluable`` (the script couldn't be run faithfully). status: str reason: str = "" - #: The body the page submits, ``(name, value)`` in order: taken when it - #: submits, or the form as the page left it when it went on to a request - #: from the recording. + #: The body the page submits, ``(name, value)`` in order, taken when it + #: submits. serialized: Optional[list[tuple[str, str]]] = None - #: How the page went on: ``submit`` or the recorded request it made. + #: How the page submitted: ``listener``, ``form.submit()`` or ``POST``. via: str = "" #: Requests the page tried to send and were blocked, ``METHOD /path``. blocked: list[str] = field(default_factory=list) @@ -168,6 +170,10 @@ def _summary(method: str, url: str) -> str: return f"{method} {up.urlsplit(url).path}" +def _control(scratch: Any, name: str) -> Any: + return scratch.locator(f"form[data-openadapt-eval] [name={json.dumps(name)}]").first + + def evaluate_form( page: Any, base_url: str, @@ -177,8 +183,9 @@ def evaluate_form( *, submitter: Optional[str] = None, submitter_requests: tuple[str, ...] = (), + confirm: Optional[str] = None, timeout_s: float = 20.0, - settle_ms: int = 250, + settle_ms: int = 300, ) -> FormEvaluation: """Run the form's script in a scratch copy of ``page``'s session. @@ -191,39 +198,50 @@ def evaluate_form( late: list[dict[str, Any]] = [] errors: list[str] = [] dialogs: list[str] = [] + stray: list[str] = [] phase = {"loaded": False} origin = up.urlsplit(base_url).netloc + reads = {r.split(" ", 1)[1] for r in submitter_requests if r.startswith("GET ")} context = None - scratch = None def route(r: Any) -> None: req = r.request - same_origin = up.urlsplit(req.url).netloc == origin - if phase["loaded"]: - # Once the page has loaded, nothing leaves: no write, no read - # that could reach the session, no other origin. - try: - body = req.post_data - except Exception: # noqa: BLE001 - a binary body has no text - body = None - late.append( - { - "what": _summary(req.method, req.url), - "method": req.method, - "path": up.urlsplit(req.url).path, - "body": body, - "content_type": (req.headers or {}).get("content-type", ""), - } - ) - r.abort() - elif ( - req.method not in ("GET", "HEAD") - or not same_origin - or req.resource_type in ("image", "media", "font") + parts = up.urlsplit(req.url) + same_origin = parts.netloc == origin + if req.resource_type in ("image", "media", "font") and ( + not phase["loaded"] or _STATIC.search(parts.path) ): - r.abort() - else: + r.abort() # never needed to run the form's script + return + if not phase["loaded"]: + if req.method in ("GET", "HEAD") and same_origin: + r.continue_() + else: + r.abort() + return + if ( + same_origin + and req.method == "GET" + and (req.resource_type in ("script", "stylesheet") or parts.path in reads) + ): + # Static code, or a read the person's click made in the + # recording (the API step makes it too, as a pre-check). r.continue_() + return + try: + body = req.post_data + except Exception: # noqa: BLE001 - a binary body has no text + body = None + late.append( + { + "what": _summary(req.method, req.url), + "method": req.method, + "path": parts.path, + "body": body, + "content_type": (req.headers or {}).get("content-type", ""), + } + ) + r.abort() def done( status: str, reason: str = "", serialized: Any = None, via: str = "" @@ -241,8 +259,6 @@ def done( live = page.context browser = getattr(live, "browser", None) if browser is None: - # Without a separate context the scratch page would share the - # run's tabs and storage; don't run the page's script there. return done( "unevaluable", "this session can't open a separate scratch context" ) @@ -251,9 +267,24 @@ def done( viewport=page.viewport_size or {"width": 1280, "height": 800}, service_workers="block", ) + # Every page of the scratch context goes through the route, and no + # socket leaves it. + context.route("**/*", route) + context.route_web_socket( + "**", lambda ws: (stray.append(f"WebSocket {ws.url}"), ws.close()) + ) + context.add_init_script(_INSTRUMENT_JS) scratch = context.new_page() - scratch.add_init_script(_INSTRUMENT_JS) - scratch.route("**/*", route) + + def on_page(other: Any) -> None: + # A popup or new tab: the scratch run can't follow it. + stray.append(f"a new page ({other.url or 'about:blank'})") + try: + other.close() + except Exception: # noqa: BLE001 - already closed + pass + + context.on("page", on_page) scratch.on("pageerror", lambda exc: errors.append(str(exc)[:200])) def on_dialog(d: Any) -> None: @@ -278,113 +309,215 @@ def on_dialog(d: Any) -> None: f"the form page raised an error as it loaded ({errors[0]})", ) phase["loaded"] = True - try: - result = scratch.evaluate( - _EVALUATE_JS, - { - "actionPath": action_path, - "leaf": action_path.rsplit("/", 1)[-1], - "fields": [[k, v] for k, v in fields], - "settleMs": settle_ms, - "submitter": submitter, - }, - ) - except Exception as exc: # noqa: BLE001 - e.g. the page navigated away - result = {"navigated": type(exc).__name__} - return _verdict( - result, late, errors, dialogs, action_path, set(submitter_requests), done + setup = scratch.evaluate( + _SETUP_JS, + {"actionPath": action_path, "leaf": action_path.rsplit("/", 1)[-1]}, ) + if setup.get("unevaluable"): + return done("unevaluable", setup["unevaluable"]) + problem = _type_values(scratch, fields) + if problem: + return done("unevaluable", problem) + scratch.wait_for_timeout(settle_ms) + pre = scratch.evaluate(_PRESUBMIT_JS) + if submitter: + target = scratch.locator(submitter).first + if target.count() == 0: + return done( + "unevaluable", f"the scratch page has no {submitter} to click" + ) + target.click(timeout=_ACT_TIMEOUT_MS) + elif pre.get("buttonScript"): + return done( + "unevaluable", + "a button of the form runs script on click and the recording names " + "no control to click", + ) + else: + last = fields[-1][0] if fields else None + if last: + _control(scratch, last).evaluate("e => e.blur()") + if pre.get("invalid"): + return done( + "invalid", + "the browser's own checks refuse these values (" + + "; ".join(pre["invalid"][:3]) + + ")", + ) + scratch.evaluate( + "() => document.querySelector('form[data-openadapt-eval]').requestSubmit()" + ) + scratch.wait_for_timeout(settle_ms) + state = scratch.evaluate(_COLLECT_JS) + if confirm and not _submitted(state, late, action_path): + # The recorded reads ran; the person confirmed in what they + # showed. Confirm the same way, wherever the control is. + try: + scratch.wait_for_load_state("load", timeout=_ACT_TIMEOUT_MS) + except Exception: # noqa: BLE001 - no navigation is fine + pass + frame = _frame_with(scratch, confirm) + if frame is not None: + frame.locator(confirm).first.click(timeout=_ACT_TIMEOUT_MS) + scratch.wait_for_timeout(settle_ms) + state = scratch.evaluate(_COLLECT_JS) + return _verdict(state, late, errors, dialogs, stray, action_path, done) except Exception as exc: # noqa: BLE001 - a failure to run is unevaluable return done("unevaluable", f"the scratch page failed ({type(exc).__name__})") finally: try: - if scratch is not None: - scratch.close() if context is not None: context.close() except Exception: # noqa: BLE001 - cleanup only pass +#: For a control that isn't on screen (a field on a tab the person opened +#: first): the same events, key by key, dispatched by script. Key handlers +#: still run; the events are not trusted. +_SCRIPT_TYPE_JS = """ +(e, value) => { + const fire = (type, init) => e.dispatchEvent(new (type.startsWith('key') ? KeyboardEvent : Event)( + type, Object.assign({ bubbles: true, cancelable: true }, init || {}))); + if (e.tagName === 'SELECT') { + e.value = value; + fire('input'); fire('change'); + return e.value === value; + } + e.value = ''; + for (const ch of value) { + if (!fire('keydown', { key: ch })) continue; + fire('keypress', { key: ch }); + e.value = e.value + ch; + fire('input'); + fire('keyup', { key: ch }); + } + fire('change'); + e.dispatchEvent(new FocusEvent('blur')); + return true; +} +""" + + +def _type_values(scratch: Any, fields: list[tuple[str, str]]) -> Optional[str]: + """Type each value the way a person does; why it couldn't be, or None.""" + + for name, value in fields: + control = _control(scratch, name) + if control.count() == 0: + return f"{name} is not a control in the form" + tag, kind = control.evaluate("e => [e.tagName, (e.type || '').toLowerCase()]") + if kind in ("checkbox", "radio", "file", "hidden"): + return f"{name} is a {kind} the API step doesn't fill" + if not control.is_visible(): + if not control.evaluate(_SCRIPT_TYPE_JS, value): + return f"{name} has no option with value {value!r}" + continue + if tag == "SELECT": + label = control.evaluate( + "(e, v) => { const o = Array.from(e.options).find((x) => x.value === v);" + " return o ? o.text.trim() : null; }", + value, + ) + if label is None: + return f"{name} has no option with value {value!r}" + control.focus(timeout=_ACT_TIMEOUT_MS) + # A focused select picks the option whose label is typed. + control.press_sequentially(label, timeout=_ACT_TIMEOUT_MS) + if control.evaluate("e => e.value") != value: + return f"typing {label!r} into {name} didn't pick its option" + continue + control.click(timeout=_ACT_TIMEOUT_MS) + control.press("ControlOrMeta+a", timeout=_ACT_TIMEOUT_MS) + control.press("Delete", timeout=_ACT_TIMEOUT_MS) + if value: + control.press_sequentially(value, timeout=_ACT_TIMEOUT_MS) + return None + + +def _frame_with(scratch: Any, selector: str) -> Any: + for frame in scratch.frames: + try: + if frame.locator(selector).count() > 0: + return frame + except Exception: # noqa: BLE001 - a detached frame + continue + return None + + +def _submitted( + state: dict[str, Any], late: list[dict[str, Any]], action_path: str +) -> bool: + saves = [b for b in late if b["method"] == "POST" and b["path"] == action_path] + return bool(state.get("submits") or state.get("programmatic") or saves) + + def _pairs(items: Any) -> list[tuple[str, str]]: return [(str(k), str(v)) for k, v in items or []] def _verdict( - result: dict[str, Any], + state: dict[str, Any], late: list[dict[str, Any]], errors: list[str], dialogs: list[str], + stray: list[str], action_path: str, - expected: set[str], done: Any, ) -> FormEvaluation: """Decide from what the page did; any doubt is a reason for the GUI.""" - if result.get("unevaluable"): - return done("unevaluable", result["unevaluable"]) saves = [b for b in late if b["method"] == "POST" and b["path"] == action_path] - recorded = [b for b in late if b["what"] in expected and b not in saves] - others = [b for b in late if b not in saves and b not in recorded] - opened_all = list(result.get("opened") or []) - opened = [u for u in opened_all if f"GET {up.urlsplit(u).path}" not in expected] + others = [b for b in late if b not in saves] if others: sent = sorted({b["what"] for b in others}) return done( "unevaluable", f"the form's script tried to send {', '.join(sent)} while it ran (blocked)", ) + channels = list(state.get("channels") or []) + stray + if channels: + return done( + "unevaluable", + f"the form's script opened {channels[0]} while it ran (blocked)", + ) if errors: return done("unevaluable", f"the form's script raised an error ({errors[0]})") - if result.get("threw"): + if state.get("threw"): return done( - "unevaluable", f"the form's script raised an error ({result['threw']})" + "unevaluable", f"the form's script raised an error ({state['threw']})" ) - if opened: - return done("unevaluable", "the form's script opened a window while it ran") if dialogs: return done("invalid", f"the form's script showed a dialog ({dialogs[0]})") - if result.get("rules"): - names = ", ".join(sorted(result["rules"])[:5]) + if state.get("rules"): + names = ", ".join(sorted(state["rules"])[:5]) return done( "invalid", f"the form's validation rules refuse these values ({names})" ) - # What the page submits, at the moment it submits. - if saves: - save = saves[0] + # What the page submits, at the moment it submits; exactly once. + bodies: list[tuple[str, list[tuple[str, str]]]] = [] + for save in saves: if save["body"] is None or "x-www-form-urlencoded" not in save["content_type"]: return done( "unevaluable", "the page submitted a body the API step can't compare" ) - body = up.parse_qsl(save["body"], keep_blank_values=True) - return done("valid", "", body, "submit") - if result.get("reached") and not result.get("prevented"): - return done("valid", "", _pairs(result.get("submitted")), "submit") - if result.get("programmatic"): - body = result["programmatic"][0] - if body is None: + bodies.append(("POST", up.parse_qsl(save["body"], keep_blank_values=True))) + bodies += [("listener", _pairs(b)) for b in state.get("submits") or []] + for b in state.get("programmatic") or []: + if b is None: return done("unevaluable", "the page's own submit could not be read") - return done("valid", "", _pairs(body), "submit") - if recorded or len(opened_all) > len(opened): - via = recorded[0]["what"] if recorded else "a window from the recording" - return done("valid", "", _pairs(result.get("after")), via) - if result.get("navigated"): - return done("unevaluable", "the page navigated while its script ran") - if result.get("invalid"): + bodies.append(("form.submit()", _pairs(b))) + if len(bodies) > 1: return done( - "invalid", - "the browser's own checks refuse these values (" - + "; ".join(result["invalid"][:3]) - + ")", + "unevaluable", + f"the page submitted the form {len(bodies)} times while its script ran", ) - if result.get("prevented"): + if bodies: + via, body = bodies[0] + return done("valid", "", body, via) + if state.get("prevented"): return done("invalid", "the form's script refused the submit for these values") - if result.get("clicked"): - return done( - "invalid", - "after the recorded click the page neither submitted nor went on as " - "recorded (its script may have refused these values)", - ) return done( - "unevaluable", "the form's script stopped the submit before it could be seen" + "invalid", + "the page didn't submit the form (its script may have refused these values)", ) diff --git a/packages/flow/openadapt_flow/runtime/actuators/web_form.py b/packages/flow/openadapt_flow/runtime/actuators/web_form.py index d6f3966a71..838a582b42 100644 --- a/packages/flow/openadapt_flow/runtime/actuators/web_form.py +++ b/packages/flow/openadapt_flow/runtime/actuators/web_form.py @@ -506,6 +506,7 @@ def evaluate_form( *, submitter: Optional[str] = None, submitter_requests: tuple[str, ...] = (), + confirm: Optional[str] = None, ) -> Any: from openadapt_flow.runtime.actuators.form_eval import evaluate_form @@ -517,6 +518,7 @@ def evaluate_form( fields, submitter=submitter, submitter_requests=submitter_requests, + confirm=confirm, timeout_s=self._timeout_ms / 1000.0, ) @@ -805,12 +807,11 @@ def _run_page_script( ) -> Optional[str]: """Run the form's own script on the run's values; why not to send, or None. - The fields the person typed or picked go into a scratch copy of the - form, and the recorded control is clicked (form_eval). The save is - sent only when the page submits, or goes on to a request the click - made in the recording, and the body it submits is exactly ``body`` - apart from the session values the scratch page was issued for - itself. Any difference keeps the save on the GUI. + The fields the person typed or picked are typed into a scratch copy + of the form, and the recorded controls are clicked (form_eval). The + save is sent only when the page submits, once, and the body it + submits is exactly ``body`` apart from the session values the scratch + page was issued for itself. Any difference keeps the save on the GUI. """ evaluate = getattr(self.transport, "evaluate_form", None) @@ -823,6 +824,7 @@ def _run_page_script( [(k, v) for k, v in body if k in typed], submitter=spec.submitter, submitter_requests=tuple(spec.submitter_requests), + confirm=spec.confirm, ) self.log.append( { diff --git a/packages/flow/tests/test_api_step_page_script.py b/packages/flow/tests/test_api_step_page_script.py index e8d886cf16..6fe73fa5bd 100644 --- a/packages/flow/tests/test_api_step_page_script.py +++ b/packages/flow/tests/test_api_step_page_script.py @@ -49,6 +49,7 @@ class _App: def __init__(self) -> None: self.pages: dict[str, str] = {"/": "home"} self.writes: list[str] = [] + self.paths: list[str] = [] self.loads = 0 app = self @@ -58,6 +59,7 @@ def log_message(self, *a: Any) -> None: def do_GET(self) -> None: # noqa: N802 path = self.path.split("?")[0] + app.paths.append(path) if path in app.pages: app.loads += 1 body = app.pages[path].replace("{n}", str(app.loads)).encode() @@ -385,7 +387,11 @@ def test_an_onsubmit_rewrite_is_in_the_body_compared(app_page): form_attrs='onsubmit="this.lname.value = this.lname.value.toUpperCase(); return true"' ) result = _evaluate(app, page) - assert result.status == "valid" and result.via == "submit" + assert result.status == "valid" and result.via in ( + "listener", + "POST", + "form.submit()", + ) assert ("lname", "MARSH") in result.serialized @@ -405,36 +411,114 @@ def test_a_click_handler_on_a_button_needs_the_recorded_control(app_page): assert app.writes == [] -def test_a_click_that_goes_on_to_the_recorded_request_counts(app_page): - app, page = app_page - app.pages["/dup.php"] = "No matches" +DIALOG = """{body}""" + + +def _dup_flow(app: _App, matches: bool) -> None: + """The OpenEMR shape: #create validates, opens the duplicate search in a + dialog frame, and the person confirms there; then the form submits.""" + + app.pages["/dup.php"] = DIALOG.format( + body="

Rosa Delgado 1958-03-27

" + if matches + else '

No matches were found.

' + ) app.form( form_attrs="", - lname_attrs="", - script="function check() { if (pastDate(document.forms[0].dob.value)) {" - " const f = document.createElement('iframe'); f.src = '/dup.php?lname=x';" - " document.body.appendChild(f); } }", + script="var force_submit = false;" + " function check() { const f = document.forms[0];" + " if (!pastDate(f.dob.value)) return;" + " if (force_submit) { f.submit(); return; }" + " const d = document.createElement('iframe'); d.id = 'modalframe'; d.src = '/dup.php?lname=x';" + " document.body.appendChild(d); }", ) - page_html = app.pages["/new.php"].replace( + app.pages["/new.php"] = app.pages["/new.php"].replace( '', '', ) - app.pages["/new.php"] = page_html - loads = app.loads - ok = _evaluate(app, page, submitter="#create", submitter_requests=("GET /dup.php",)) - assert ok.status == "valid" and ok.via == "GET /dup.php" - # The duplicate search was blocked: only the form page itself loaded. - assert app.loads == loads + 1 + + +DUP = { + "submitter": "#create", + "submitter_requests": ("GET /dup.php",), + "confirm": "#confirmCreate", +} + + +def test_the_recorded_read_completes_and_the_page_must_then_submit(app_page): + app, page = app_page + _dup_flow(app, matches=False) + ok = _evaluate(app, page, **DUP) + assert ok.status == "valid" and ok.via == "form.submit()", ok.reason + assert ("lname", "Marsh") in ok.serialized bad = [("lname", "Marsh"), ("dob", "2999-01-01"), ("state", "CA")] - refused = _evaluate( - app, page, bad, submitter="#create", submitter_requests=("GET /dup.php",) + refused = _evaluate(app, page, bad, **DUP) + assert refused.status == "invalid" and "didn't submit" in refused.reason + assert app.writes == [] + + +def test_a_duplicate_search_that_finds_a_match_is_not_valid(app_page): + # Review hole 5: the read's answer decides, not that it was made. + app, page = app_page + _dup_flow(app, matches=True) + result = _evaluate(app, page, **DUP) + assert result.status == "invalid" and "didn't submit" in result.reason + assert app.writes == [] + + +def test_a_popup_is_routed_too_and_makes_the_run_unevaluable(app_page): + # Review hole 1: a target=_blank form submitted past every listener. + app, page = app_page + app.form( + form_attrs='target="_blank"', + script="document.forms[0].addEventListener('submit', e => e.stopPropagation())", + ) + result = _evaluate(app, page) + assert result.status == "unevaluable" + assert app.writes == [] + + +def test_a_websocket_opened_by_the_page_is_closed_and_unevaluable(app_page): + # Review hole 2. + app, page = app_page + port = app.base.rsplit(":", 1)[1] + app.form( + script="document.forms[0].dob.addEventListener('change', () => {" + f" try {{ new WebSocket('ws://127.0.0.1:{port}/ws'); }} catch (e) {{}} }});" ) - assert refused.status == "invalid" and "neither submitted" in refused.reason - # A request the recording didn't make is refused. - other = _evaluate( - app, page, submitter="#create", submitter_requests=("GET /else.php",) + result = _evaluate(app, page) + assert result.status == "unevaluable" and "WebSocket" in result.reason + assert app.writes == [] and "/ws" not in app.paths + + +def test_key_handlers_run_as_the_person_types(app_page): + # Review hole 3: an onkeyup uppercaser. + app, page = app_page + app.form(lname_attrs='onkeyup="this.value = this.value.toUpperCase()"') + result = _evaluate(app, page) + assert result.status == "valid" and ("lname", "MARSH") in result.serialized + + +def test_the_click_is_trusted(app_page): + # Review hole 4: a handler that validates only trusted clicks. + app, page = app_page + app.form( + form_attrs="", + script="document.querySelector('button').addEventListener('click', e => {" + " if (e.isTrusted && !pastDate(document.forms[0].dob.value)) e.preventDefault(); })", ) - assert other.status == "unevaluable" and "GET /dup.php" in other.reason + bad = [("lname", "Marsh"), ("dob", "2999-01-01"), ("state", "CA")] + assert _evaluate(app, page, bad, submitter="button").status == "invalid" + assert _evaluate(app, page, submitter="button").status == "valid" + + +def test_a_page_that_submits_twice_is_unevaluable(app_page): + app, page = app_page + app.form(form_attrs='onsubmit="this.submit(); return true"') + result = _evaluate(app, page) + assert result.status == "unevaluable" and "2 times" in result.reason def test_no_request_leaves_after_the_page_has_loaded(app_page): From e27c1cc02b40eac96585b8cdd8b551ae1a3bc064 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 02:32:26 -0400 Subject: [PATCH 14/15] flow: a field the scratch run can't type with real key events fails closed A field that isn't on screen when the form opens (on a tab the person opened first) made the scratch run use script events. It now makes the run unevaluable, so the save stays on the GUI. Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/API_TIER.md | 4 +- .../runtime/actuators/form_eval.py | 40 ++++--------------- .../flow/tests/test_api_step_page_script.py | 16 ++++++++ 3 files changed, 26 insertions(+), 34 deletions(-) diff --git a/packages/flow/docs/API_TIER.md b/packages/flow/docs/API_TIER.md index 1cbeffd980..7b4db897ee 100644 --- a/packages/flow/docs/API_TIER.md +++ b/packages/flow/docs/API_TIER.md @@ -153,8 +153,8 @@ Before it sends the save, the API step: - it types the run's typed and picked values with real key events, so key handlers (`onkeyup`, `onchange`) run as they do for the person. A field that isn't on screen when the form opens (on a tab the person - opened first) gets the same events key by key from script; its - handlers run, but those events aren't trusted; + opened first) can't be typed with real key events, so the run is + unevaluable and the GUI steps run; - it clicks the control the person clicked (`submitter`) with a real, trusted click. When the recording names none it submits the form, and a form whose buttons have click handlers is refused. When the click diff --git a/packages/flow/openadapt_flow/runtime/actuators/form_eval.py b/packages/flow/openadapt_flow/runtime/actuators/form_eval.py index 6577c8d237..43e7397a48 100644 --- a/packages/flow/openadapt_flow/runtime/actuators/form_eval.py +++ b/packages/flow/openadapt_flow/runtime/actuators/form_eval.py @@ -19,8 +19,8 @@ 3. type the run's values into the fields the person typed or picked, with real key events (``keydown``, ``keyup``, ``input``), so key handlers run as they do for the person. A field that isn't on screen when the form - opens (on a tab the person opened first) gets the same events key by - key from script: its handlers run, but those events aren't trusted; + opens (on a tab the person opened first) can't be typed that way, so the + run is unevaluable; 4. click the control the person clicked (``submitter``) with a real click; when the recording names none, submit the form, and refuse a form whose buttons have click handlers. When the page then shows the recorded @@ -372,33 +372,6 @@ def on_dialog(d: Any) -> None: pass -#: For a control that isn't on screen (a field on a tab the person opened -#: first): the same events, key by key, dispatched by script. Key handlers -#: still run; the events are not trusted. -_SCRIPT_TYPE_JS = """ -(e, value) => { - const fire = (type, init) => e.dispatchEvent(new (type.startsWith('key') ? KeyboardEvent : Event)( - type, Object.assign({ bubbles: true, cancelable: true }, init || {}))); - if (e.tagName === 'SELECT') { - e.value = value; - fire('input'); fire('change'); - return e.value === value; - } - e.value = ''; - for (const ch of value) { - if (!fire('keydown', { key: ch })) continue; - fire('keypress', { key: ch }); - e.value = e.value + ch; - fire('input'); - fire('keyup', { key: ch }); - } - fire('change'); - e.dispatchEvent(new FocusEvent('blur')); - return true; -} -""" - - def _type_values(scratch: Any, fields: list[tuple[str, str]]) -> Optional[str]: """Type each value the way a person does; why it couldn't be, or None.""" @@ -410,9 +383,12 @@ def _type_values(scratch: Any, fields: list[tuple[str, str]]) -> Optional[str]: if kind in ("checkbox", "radio", "file", "hidden"): return f"{name} is a {kind} the API step doesn't fill" if not control.is_visible(): - if not control.evaluate(_SCRIPT_TYPE_JS, value): - return f"{name} has no option with value {value!r}" - continue + # Only real, trusted input counts; a field the person reached + # through another step (a tab, a section) can't be typed here. + return ( + f"{name} isn't on screen when the form opens, so it can't be typed " + "with real key events" + ) if tag == "SELECT": label = control.evaluate( "(e, v) => { const o = Array.from(e.options).find((x) => x.value === v);" diff --git a/packages/flow/tests/test_api_step_page_script.py b/packages/flow/tests/test_api_step_page_script.py index 6fe73fa5bd..a6688350b7 100644 --- a/packages/flow/tests/test_api_step_page_script.py +++ b/packages/flow/tests/test_api_step_page_script.py @@ -572,3 +572,19 @@ def test_the_recorded_control_and_the_request_it_made_are_in_the_proposal(tmp_pa spec = step.api_binding.web_form assert reasons == [] and spec.submitter == "#create" assert spec.submitter_requests == ["GET /app/dup.php"] + + +def test_a_field_that_isnt_on_screen_makes_the_run_unevaluable(app_page): + # Fail closed: only real, trusted key events count. + app, page = app_page + app.form() + app.pages["/new.php"] = ( + app.pages["/new.php"] + .replace('', 'onchange="noteChange(this)">', 1 + ) + ) + result = _evaluate(app, page) + assert result.status == "unevaluable" and "dob isn't on screen" in result.reason + assert app.writes == [] From f02890763394a07628d8013e7506b505229f6fd7 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 02:39:26 -0400 Subject: [PATCH 15/15] flow: after load, only the scratch page's own code and recorded reads get through Round 3: a script or stylesheet passed by resource type alone, so a handler could add and reach the app with the session. Now only the exact script and stylesheet URLs the page loaded, the recorded reads, and the static .js/.css files of a recorded read's own frame get through; anything else is blocked and makes the run unevaluable. Submits and blocked channels are collected from every frame, not only the form's. Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/API_TIER.md | 10 ++- .../runtime/actuators/form_eval.py | 89 ++++++++++++++++--- .../flow/tests/test_api_step_page_script.py | 39 ++++++++ 3 files changed, 120 insertions(+), 18 deletions(-) diff --git a/packages/flow/docs/API_TIER.md b/packages/flow/docs/API_TIER.md index 7b4db897ee..20adbf9ec1 100644 --- a/packages/flow/docs/API_TIER.md +++ b/packages/flow/docs/API_TIER.md @@ -146,10 +146,12 @@ Before it sends the save, the API step: every WebSocket is closed, and EventSource, RTCPeerConnection, `sendBeacon` and `window.open` do nothing. While the page loads, only same-origin GET requests go through. Once it has loaded, everything is - blocked except same-origin scripts and stylesheets and the reads the - person's click made in the recording (`submitter_requests`; on OpenEMR, - the duplicate search), so nothing the page does there can write. A - popup or new tab makes the run unevaluable; + blocked except the exact scripts and stylesheets the page loaded, the + reads the person's click made in the recording (`submitter_requests`; + on OpenEMR, the duplicate search) and the static `.js` and `.css` files + of a recorded read's own frame, so nothing the page does there can + write. A popup or new tab, and a form submit or a blocked channel in + any frame, make the run unevaluable; - it types the run's typed and picked values with real key events, so key handlers (`onkeyup`, `onchange`) run as they do for the person. A field that isn't on screen when the form opens (on a tab the person diff --git a/packages/flow/openadapt_flow/runtime/actuators/form_eval.py b/packages/flow/openadapt_flow/runtime/actuators/form_eval.py index 43e7397a48..d4568747af 100644 --- a/packages/flow/openadapt_flow/runtime/actuators/form_eval.py +++ b/packages/flow/openadapt_flow/runtime/actuators/form_eval.py @@ -12,10 +12,12 @@ request of every page in it routed, every WebSocket closed, and EventSource and RTCPeerConnection stubbed out; 2. while the page loads, let only same-origin GET requests through (no - other origin at all). Once it has loaded, block every request except - same-origin scripts and stylesheets and the reads the person's click - made in the recording (``submitter_requests``, such as OpenEMR's - duplicate search), so nothing the page does can write; + other origin at all). Once it has loaded, block every request except the + exact scripts and stylesheets the page loaded, the reads the person's + click made in the recording (``submitter_requests``, such as OpenEMR's + duplicate search) and the static ``.js``/``.css`` files of a recorded + read's own frame, so nothing the page does can write; a form submit or a + blocked channel in any frame counts as the page sending; 3. type the run's values into the fields the person typed or picked, with real key events (``keydown``, ``keyup``, ``input``), so key handlers run as they do for the person. A field that isn't on screen when the form @@ -202,8 +204,38 @@ def evaluate_form( phase = {"loaded": False} origin = up.urlsplit(base_url).netloc reads = {r.split(" ", 1)[1] for r in submitter_requests if r.startswith("GET ")} + #: Exact script and stylesheet URLs the form page loaded. + loaded_code: set[str] = set() context = None + def _allowed_after_load(req: Any, parts: Any) -> bool: + """Once loaded, only what the person's steps also load gets through. + + The exact scripts and stylesheets the form page loaded; the reads the + person's click made in the recording (the API step makes them too, as + pre-checks); and the static ``.js``/``.css`` files a recorded read's + own frame loads (the dialog it shows). A script or stylesheet a + handler adds at another URL is the page sending something. + """ + + if parts.path in reads and req.resource_type in ("document", "xhr", "fetch"): + return True + if req.resource_type not in ("script", "stylesheet"): + return False + if req.url in loaded_code: + return True + try: + frame_path = up.urlsplit(req.frame.url).path + except Exception: # noqa: BLE001 - no frame + return False + return ( + frame_path in reads + and not parts.query + and parts.path.endswith( + ".css" if req.resource_type == "stylesheet" else ".js" + ) + ) + def route(r: Any) -> None: req = r.request parts = up.urlsplit(req.url) @@ -215,17 +247,13 @@ def route(r: Any) -> None: return if not phase["loaded"]: if req.method in ("GET", "HEAD") and same_origin: + if req.resource_type in ("script", "stylesheet"): + loaded_code.add(req.url) r.continue_() else: r.abort() return - if ( - same_origin - and req.method == "GET" - and (req.resource_type in ("script", "stylesheet") or parts.path in reads) - ): - # Static code, or a read the person's click made in the - # recording (the API step makes it too, as a pre-check). + if same_origin and req.method == "GET" and _allowed_after_load(req, parts): r.continue_() return try: @@ -348,7 +376,7 @@ def on_dialog(d: Any) -> None: "() => document.querySelector('form[data-openadapt-eval]').requestSubmit()" ) scratch.wait_for_timeout(settle_ms) - state = scratch.evaluate(_COLLECT_JS) + state = _collect(scratch) if confirm and not _submitted(state, late, action_path): # The recorded reads ran; the person confirmed in what they # showed. Confirm the same way, wherever the control is. @@ -360,7 +388,7 @@ def on_dialog(d: Any) -> None: if frame is not None: frame.locator(confirm).first.click(timeout=_ACT_TIMEOUT_MS) scratch.wait_for_timeout(settle_ms) - state = scratch.evaluate(_COLLECT_JS) + state = _collect(scratch) return _verdict(state, late, errors, dialogs, stray, action_path, done) except Exception as exc: # noqa: BLE001 - a failure to run is unevaluable return done("unevaluable", f"the scratch page failed ({type(exc).__name__})") @@ -411,6 +439,39 @@ def _type_values(scratch: Any, fields: list[tuple[str, str]]) -> Optional[str]: return None +#: What a frame other than the form's did: its own form submits and the +#: channels it tried, all of which count as the page sending. +_FRAME_JS = """ +() => { + const state = window.__openadaptFormEval; + if (!state) return { uninstrumented: true }; + return { programmatic: state.programmatic.length, submits: state.submits.length, + channels: state.channels }; +} +""" + + +def _collect(scratch: Any) -> dict[str, Any]: + """The form frame's record, plus anything any other frame did.""" + + state = scratch.evaluate(_COLLECT_JS) + for frame in scratch.frames: + if frame == scratch.main_frame: + continue + try: + other = frame.evaluate(_FRAME_JS) + except Exception: # noqa: BLE001 - a detached or cross-origin frame + other = {"uninstrumented": True} + url = frame.url or "a frame" + if other.get("uninstrumented") and url not in ("", "about:blank"): + state.setdefault("channels", []).append(f"a frame it can't read ({url})") + for channel in other.get("channels") or []: + state.setdefault("channels", []).append(f"{channel} in {url}") + if other.get("programmatic") or other.get("submits"): + state.setdefault("channels", []).append(f"a form submit in {url}") + return state + + def _frame_with(scratch: Any, selector: str) -> Any: for frame in scratch.frames: try: @@ -455,7 +516,7 @@ def _verdict( if channels: return done( "unevaluable", - f"the form's script opened {channels[0]} while it ran (blocked)", + f"the form's script reached outside the scratch run ({channels[0]}; blocked)", ) if errors: return done("unevaluable", f"the form's script raised an error ({errors[0]})") diff --git a/packages/flow/tests/test_api_step_page_script.py b/packages/flow/tests/test_api_step_page_script.py index a6688350b7..3170fc6173 100644 --- a/packages/flow/tests/test_api_step_page_script.py +++ b/packages/flow/tests/test_api_step_page_script.py @@ -588,3 +588,42 @@ def test_a_field_that_isnt_on_screen_makes_the_run_unevaluable(app_page): result = _evaluate(app, page) assert result.status == "unevaluable" and "dob isn't on screen" in result.reason assert app.writes == [] + + +@pytest.mark.parametrize( + "tag", + [ + "const l = document.createElement('link'); l.rel = 'stylesheet'; l.href = '/act.php?delete=1';", + "const l = document.createElement('script'); l.src = '/act2.php?delete=1';", + ], +) +def test_code_a_handler_adds_after_load_never_reaches_the_app(app_page, tag): + # Round 3: a stylesheet or script URL the page didn't load is the page + # sending something, whatever its resource type. + app, page = app_page + app.pages["/act.php"] = app.pages["/act2.php"] = "ok" + app.form( + script="document.forms[0].dob.addEventListener('change', () => {" + f" {tag} document.head.appendChild(l); }});" + ) + result = _evaluate(app, page) + assert result.status == "unevaluable" and "act" in result.reason + assert "/act.php" not in app.paths and "/act2.php" not in app.paths + + +def test_a_form_submit_in_another_frame_is_seen(app_page): + # Round 3: an iframe's own form.submit() is recorded in that frame. + app, page = app_page + app.pages["/side.php"] = ( + '
' + '
' + ) + app.form( + script="document.forms[0].dob.addEventListener('change', () => {" + " const f = document.createElement('iframe'); f.src = '/side.php';" + " f.onload = () => f.contentDocument.forms[0].submit();" + " document.body.appendChild(f); });" + ) + result = _evaluate(app, page, submitter_requests=("GET /side.php",)) + assert result.status == "unevaluable" and "form submit in" in result.reason + assert app.writes == []