diff --git a/CAPABILITIES.md b/CAPABILITIES.md index cc0d305da2..6824f16658 100644 --- a/CAPABILITIES.md +++ b/CAPABILITIES.md @@ -200,6 +200,8 @@ 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` | +| 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/llms-full.txt b/llms-full.txt index cc4a53ea85..f36937d7d7 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -597,6 +597,8 @@ 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` | +| 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) | 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() diff --git a/packages/flow/docs/API_TIER.md b/packages/flow/docs/API_TIER.md index 4399ac911e..3428796a6f 100644 --- a/packages/flow/docs/API_TIER.md +++ b/packages/flow/docs/API_TIER.md @@ -3,9 +3,14 @@ **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, multi-page server state, and forms whose script validates or rewrites -values all stay on the GUI. +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. 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 @@ -93,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; @@ -113,6 +118,52 @@ 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. 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 (there the request + never reaches the app; the page gets an empty 200, as the recorded write + was accepted), and each request it sent matches one step write; + 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 + 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. + +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 @@ -125,16 +176,53 @@ 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 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. runs the pre-checks. +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. 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 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 + 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 + 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. After the save request starts, the API tier's no-double-write rule @@ -357,15 +445,18 @@ 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 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/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/compiler/api_proposal.py b/packages/flow/openadapt_flow/compiler/api_proposal.py index 58297d4512..74152ee0fd 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. """ @@ -61,14 +67,15 @@ WebFormField, WebFormPrecheck, WebFormSpec, + WebFormWrite, Workflow, ) from openadapt_flow.runtime.actuators.web_form import ( client_rules, folded_checks_digest, form_controls, - form_scripts, - unsupported_client_rules, + page_scripts, + web_form_effects, ) @@ -241,6 +248,212 @@ 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 a control + of ``html`` that can start a submit (a `` + +""" + +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.paths: 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] + app.paths.append(path) + 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, **kw: Any) -> FormEvaluation: + return PlaywrightSessionTransport(page).evaluate_form( + "/new.php", "/save.php", fields, **kw + ) + + +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())", + }, + "valid", + "", + ), + ({"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, **kw): + self.calls.append(fields) + self.kw = kw + 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=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, + ) + 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 t.kw["submitter"] == "button[type=submit]" + 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 == [] + + +# -- 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 in ( + "listener", + "POST", + "form.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 == [] + + +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="", + 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); }", + ) + app.pages["/new.php"] = app.pages["/new.php"].replace( + '', + '', + ) + + +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, **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) {{}} }});" + ) + 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(); })", + ) + 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): + 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, 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": + 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) + + +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] + + +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",) + + +def test_two_declared_writes_need_two_sends_from_the_page(tmp_path): + second = dict(DRAFT, t=6.2) + wf, _step, reasons = propose_api_step(_workflow(), _rec(tmp_path, DRAFT, second)) + assert reasons == [] and len(wf.steps[-1].api_binding.web_form.writes) == 2 + wf = _with_step_effect(_with_step_effect(wf, "step_003"), "step_005") + wf, _ = admit_api_step(wf, admitted_by="reviewer") + # The page's script sends the draft once; one request can't match two. + t = SequenceTransport() + out = WebFormActuator(t).actuate(wf.steps[-1].api_binding, RUN) + assert out.status is ActuationStatus.UNAVAILABLE and "didn't send" in out.reason + assert t.posts == [] + + +def test_the_scratch_run_captures_a_step_write_the_page_sends_and_blocks_it(tmp_path): + pytest.importorskip("playwright.sync_api") + from playwright.sync_api import sync_playwright + + from openadapt_flow.runtime.actuators.web_form import PlaywrightSessionTransport + from tests.test_api_step_page_script import _App + + app = _App() + app.form( + script="document.forms[0].lname.addEventListener('change', e => fetch('/draft.php', {" + " method: 'POST', headers: {'Content-Type': 'application/x-www-form-urlencoded'}," + " body: 'form_lname=' + encodeURIComponent(e.target.value) }));" + ) + try: + with sync_playwright() as pw: + browser = pw.chromium.launch(headless=True) + page = browser.new_page() + page.goto(app.base + "/") + result = PlaywrightSessionTransport(page).evaluate_form( + "/new.php", + "/save.php", + [("lname", "Marsh"), ("dob", "1979-06-14"), ("state", "CA")], + step_writes=("POST /draft.php",), + ) + browser.close() + finally: + app.server.shutdown() + assert result.status == "valid", result.reason + assert result.writes == [("POST /draft.php", [("form_lname", "Marsh")])] + assert app.writes == [] diff --git a/packages/flow/tests/test_api_tier_web_form.py b/packages/flow/tests/test_api_tier_web_form.py index c45f0f6643..70c2a6c36e 100644 --- a/packages/flow/tests/test_api_tier_web_form.py +++ b/packages/flow/tests/test_api_tier_web_form.py @@ -411,6 +411,21 @@ def post_form(self, path, body, *, referer): self.posts.append((path, body)) return self.save + def evaluate_form(self, form_path, action_path, fields, **kw): + """A page whose script changes nothing: the browser sends the form as filled.""" + + from openadapt_flow.runtime.actuators.form_eval import FormEvaluation + + self.evaluated = getattr(self, "evaluated", 0) + 1 + controls = form_controls(self.form, action_path) or {} + typed = dict(fields) + body = [ + (name, typed.get(name, c.get("default") or "")) + for name, c in controls.items() + if c.get("default") is not None or name in typed + ] + return FormEvaluation(status="valid", serialized=body, via="submit") + def _admitted_binding(tmp_path) -> ApiBinding: wf, _, _ = propose_api_step(_workflow(), _recording(tmp_path))