From 1a73a547f73068bd5170118d680b4b1e68587a0f Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 11:06:56 -0400 Subject: [PATCH 01/11] Propose undo steps and verify them on a test copy undo propose finds candidate undo steps for each saving step (another recording, captured traffic, the field's old value), tries each on a declared test copy, and writes only the ones that restore every record as proposals. undo accept is a person's one-time approval; each run still needs approval after a stop. Co-Authored-By: Claude Opus 5.5 --- CAPABILITIES.md | 3 +- .../flow/benchmark/undo_proposal/__init__.py | 0 packages/flow/benchmark/undo_proposal/run.py | 325 ++++ packages/flow/docs/AFTER_A_STOP.md | 139 +- packages/flow/docs/LIMITS.md | 22 +- packages/flow/openadapt_flow/after_stop.py | 291 +++- packages/flow/openadapt_flow/cli_undo.py | 238 ++- packages/flow/openadapt_flow/ir.py | 104 +- .../flow/openadapt_flow/runtime/replayer.py | 35 + packages/flow/openadapt_flow/undo_proposal.py | 1414 +++++++++++++++++ packages/flow/tests/test_undo_proposal.py | 840 ++++++++++ 11 files changed, 3381 insertions(+), 30 deletions(-) create mode 100644 packages/flow/benchmark/undo_proposal/__init__.py create mode 100644 packages/flow/benchmark/undo_proposal/run.py create mode 100644 packages/flow/openadapt_flow/undo_proposal.py create mode 100644 packages/flow/tests/test_undo_proposal.py diff --git a/CAPABILITIES.md b/CAPABILITIES.md index c9125ded3a..03139c7984 100644 --- a/CAPABILITIES.md +++ b/CAPABILITIES.md @@ -133,6 +133,7 @@ improvising or retrying when something is off. | Checked later: a save confirmed by a later report or acknowledgment, never counted as done until then | In review ([#1297](https://github.com/OpenAdaptAI/OpenAdapt/pull/1297)) | Synthetic | `SAVED_AWAITING_CHECK` | | Keep the server's own answer to a save as lower-level evidence on the receipt | In review ([#1298](https://github.com/OpenAdaptAI/OpenAdapt/pull/1298)) | Synthetic | receipts | | A ledger of what changed before a stop, a write-ordering lint, and undo steps a person approves | On main ([#1263](https://github.com/OpenAdaptAI/OpenAdapt/pull/1263)) | Synthetic (MockMed) | change ledger | +| Propose an undo for each saving step (another recording, captured traffic, or the old value), keep it only if a test copy shows every record restored, and offer it after a stop once a person approves it | In review ([#PRNUM](https://github.com/OpenAdaptAI/OpenAdapt/pull/PRNUM)) | Synthetic (MockMed, MockLoan, live browser) | `openadapt-flow undo propose`, `undo accept`; [AFTER_A_STOP.md](packages/flow/docs/AFTER_A_STOP.md) | ## Approvals @@ -231,7 +232,7 @@ fails when a command is added without being listed here. - **Check and admit:** `lint`, `scaffold-verifier`, `check`, `api`, `bind-record-checks`, `certify`, `qualify`, `propose-qualification`, `known-ids`, `jit`, `admit`, `approver-grant`. - **Run and answer:** `run`, `replay`, `resume`, `approve`, `item-approval`, `triage`, `business-decisions`, `explain`, `console`. - **Judge any executor's write:** `verify-run`, `serve-reward` (scores agent training episodes by reading the system of record). -- **Repair and learn from halts:** `teach`, `teach-branch`, `repair`, `fleet`, `undo` (the undo steps a person approves after a stop). +- **Repair and learn from halts:** `teach`, `teach-branch`, `repair`, `fleet`, `undo` (the undo steps a person approves after a stop; `undo propose` finds and verifies them on a test copy, `undo accept` is a person's one-time approval of a proposal). - **Give agents the program:** `emit-skill`, `emit-mcp`, `serve-execute`. - **Proof:** `seal`, `local-seal`, `countersign`, `attach-seal`. - **Run on premises:** `service`, `no-outbound`. diff --git a/packages/flow/benchmark/undo_proposal/__init__.py b/packages/flow/benchmark/undo_proposal/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/flow/benchmark/undo_proposal/run.py b/packages/flow/benchmark/undo_proposal/run.py new file mode 100644 index 0000000000..4b4e81bd9d --- /dev/null +++ b/packages/flow/benchmark/undo_proposal/run.py @@ -0,0 +1,325 @@ +"""Measure how many saving steps get a verified undo (``undo propose``). + +For each synthetic app, end to end and live: + +1. start the app and its record store (in process, on 127.0.0.1); +2. record the canonical demonstration in a real browser with network capture + on (the demo driver's clicks, as ``record --capture-network`` keeps them); +3. compile it, and bind the save's record effect the way an operator does + (the fields the save's request carries, counted as new records only); +4. write a deployment file that declares the app a test copy, with a reset + URL and a ``rest`` reward contract over the whole record store; +5. run ``openadapt-flow undo propose`` against it, and keep its report. + +Synthetic data only; nothing leaves 127.0.0.1 and no model is called. + + PYTHONPATH=packages/types:packages/flow python -m benchmark.undo_proposal.run \\ + --out packages/flow/benchmark/undo_proposal/results + +Run one fixture at a time (``--fixture mockmed``) when the machine is busy. +""" + +from __future__ import annotations + +import argparse +import json +import os +import platform +import subprocess +import tempfile +import time +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Callable, Optional + +import yaml + + +@dataclass(frozen=True) +class Fixture: + name: str + serve: Callable[..., Any] + record: Callable[..., Path] + record_kwargs: dict[str, Any] + query: str + #: The request-body fields that identify the saved record. + match_fields: tuple[str, ...] + identity_key: str + #: Literal selector fields the request doesn't carry. + extra_match: tuple[tuple[str, str], ...] = () + #: Plain words on what this fixture's save does. + note: str = "" + + +def _fixtures() -> dict[str, Fixture]: + from openadapt_flow import demo_driver + from openadapt_flow.mockloan.fault_server import serve as serve_loan + from openadapt_flow.mockmed.fault_server import serve as serve_med + + return { + "mockmed": Fixture( + name="mockmed", + serve=serve_med, + record=demo_driver.record_triage_demo, + record_kwargs={"note_text": "Synthetic follow-up in two weeks"}, + query="?fault=ok", + match_fields=("patient_id", "type"), + identity_key="patient_id", + note="Save Encounter writes one encounter record", + ), + "mockloan": Fixture( + name="mockloan", + serve=serve_loan, + record=demo_driver.record_disbursement_demo, + record_kwargs={"memo_text": "Synthetic funding memo"}, + query="?fault=ok", + match_fields=("loan_id", "product"), + identity_key="loan_id", + extra_match=(("surface", "disbursements"),), + note="Authorize writes one disbursement record", + ), + "mockloan-fee-row": Fixture( + name="mockloan-fee-row", + serve=serve_loan, + record=demo_driver.record_disbursement_demo, + record_kwargs={"memo_text": "Synthetic funding memo"}, + query="?fault=collateral", + match_fields=("loan_id", "product"), + identity_key="loan_id", + extra_match=(("surface", "disbursements"),), + note=( + "Authorize writes the disbursement and also a fee row on a " + "second surface (MockLoan's collateral mode)" + ), + ), + } + + +def _record_with_capture(fixture: Fixture, url: str, out: Path) -> Path: + """The demo driver's recording, with the browser's traffic captured.""" + from openadapt_flow import demo_driver + from openadapt_flow.network_capture import NetworkCapture + + original = demo_driver.PlaywrightBackend.launch + captures: list[tuple[NetworkCapture, float]] = [] + + def launch(*args: Any, **kwargs: Any): + backend, close = original(*args, **kwargs) + captures.append( + (NetworkCapture(backend.page.context).start(), time.monotonic()) + ) + return backend, close + + demo_driver.PlaywrightBackend.launch = launch # type: ignore[method-assign] + try: + recording = fixture.record(url, out, **fixture.record_kwargs) + finally: + demo_driver.PlaywrightBackend.launch = original # type: ignore[method-assign] + capture, t0 = captures[0] + capture.write(recording, t0) + return Path(recording) + + +def _save_body(recording: Path) -> dict[str, Any]: + from openadapt_flow.network_capture import load + + for line in reversed(load(recording)): + if line.get("method") == "POST" and "/api/" in line.get("url", ""): + try: + return json.loads(line.get("body") or "{}") + except json.JSONDecodeError: + return {} + return {} + + +def _bind_save_effect(bundle: Path, fixture: Fixture, body: dict[str, Any]) -> str: + """Bind the save's record effect as an operator would; return its step id.""" + from openadapt_flow.ir import Workflow + from openadapt_flow.runtime.effects.effect import Effect, EffectKind + from openadapt_flow.undo_proposal import saving_steps + + workflow = Workflow.load(bundle) + saves = saving_steps(workflow) + if len(saves) != 1: + raise SystemExit(f"{fixture.name}: expected one saving step, got {len(saves)}") + index, step = saves[-1] + match = {k: str(body[k]) for k in fixture.match_fields if k in body} + if len(match) != len(fixture.match_fields): + raise SystemExit( + f"{fixture.name}: the save's request lacks {fixture.match_fields}" + ) + match.update(dict(fixture.extra_match)) + step.effects = [ + Effect( + kind=EffectKind.RECORD_WRITTEN, + match=match, + expected_count=1, + count_new_only=True, + risk="irreversible", + probe="one new record for the save", + timeout_s=5.0, + ) + ] + workflow.steps[index] = step + workflow.save(bundle) + return step.id + + +def _contract(directory: Path, base_url: str, fixture: Fixture, body: dict) -> Path: + from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey + + from openadapt_flow.reward.seed import write_bundle + + match: dict[str, Any] = {k: str(body[k]) for k in fixture.match_fields} + match.update(dict(fixture.extra_match)) + match[fixture.identity_key] = {"param": fixture.identity_key} + write_bundle( + directory, + contract_id=f"undo_propose_{fixture.name}", + oracle={"kind": "rest", "base_url": base_url}, + channel="api", + key=Ed25519PrivateKey.generate(), + issuer_key_id="self_signed:benchmark", + certify=False, + required_effects=[ + { + "kind": "record_written", + "match": match, + "expected_count": 1, + "count_new_only": True, + } + ], + forbidden_effects=[], + identity_keys=[fixture.identity_key], + ) + return directory + + +def _deployment(path: Path, fixture: Fixture, base_url: str, contract: Path) -> Path: + path.write_text( + yaml.safe_dump( + { + "name": f"{fixture.name}-test-copy", + "backend": {"kind": "web", "url": base_url + "/" + fixture.query}, + "effects": {"kind": "rest", "base_url": base_url}, + "actuation": {"api": True, "base_url": base_url}, + "test_copy": { + "declared": True, + "reset_url": base_url + "/api/reset", + "reward_contract": str(contract), + }, + }, + sort_keys=False, + ), + encoding="utf-8", + ) + return path + + +def _git_head() -> str: + try: + return subprocess.run( + ["git", "rev-parse", "HEAD"], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + except (OSError, subprocess.CalledProcessError): + return "unknown" + + +def run_fixture(fixture: Fixture, out_dir: Path, work: Path) -> dict[str, Any]: + from openadapt_flow.__main__ import main + from openadapt_flow.compiler.compile import compile_recording + + url, _db, stop = fixture.serve(port=0) + base = url.rstrip("/") + started = time.monotonic() + try: + recording = _record_with_capture( + fixture, base + "/" + fixture.query, work / "rec" + ) + bundle = work / "bundle" + compile_recording(recording, bundle, name=f"{fixture.name}-demo") + body = _save_body(recording) + step_id = _bind_save_effect(bundle, fixture, body) + contract = _contract(work / "contract", base, fixture, body) + deployment = _deployment(work / "deployment.yaml", fixture, base, contract) + report_path = out_dir / f"{fixture.name}.json" + code = main( + [ + "undo", + "propose", + str(bundle), + "--config", + str(deployment), + "--recording", + str(recording), + "--work-dir", + str(work / "propose"), + "--report", + str(report_path), + ] + ) + finally: + stop() + report = json.loads(report_path.read_text(encoding="utf-8")) + return { + "fixture": fixture.name, + "what_the_save_does": fixture.note, + "exit_code": code, + "save_step": step_id, + "seconds": round(time.monotonic() - started, 1), + "summary": report["summary"], + "steps": [ + { + "step_id": row["step_id"], + "verified_undo": row["verified_undo"], + "reason": row["reason"], + "candidates": [ + {k: c[k] for k in ("source", "status", "basis", "reason")} + for c in row["candidates"] + ], + } + for row in report["saving_steps"] + ], + } + + +def main(argv: Optional[list[str]] = None) -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("--out", required=True, type=Path) + parser.add_argument("--fixture", action="append", default=None) + args = parser.parse_args(argv) + fixtures = _fixtures() + names = args.fixture or list(fixtures) + args.out.mkdir(parents=True, exist_ok=True) + rows = [] + for name in names: + with tempfile.TemporaryDirectory(prefix=f"undo-{name}-") as tmp: + rows.append(run_fixture(fixtures[name], args.out, Path(tmp))) + summary_path = args.out / "summary.json" + previous: dict[str, Any] = {} + if summary_path.is_file(): + previous = json.loads(summary_path.read_text(encoding="utf-8")) + by_name = {r["fixture"]: r for r in previous.get("fixtures", [])} + by_name.update({r["fixture"]: r for r in rows}) + summary = { + "schema": "openadapt.undo-propose-measurement/v1", + "engine_commit": _git_head(), + "measured_at": time.strftime("%Y-%m-%dT%H:%M:%S%z"), + "host": platform.platform(), + "load_1m": round(os.getloadavg()[0], 2), + "fixtures": [by_name[k] for k in sorted(by_name)], + "totals": { + key: sum(r["summary"][key] for r in by_name.values()) + for key in ("saving_steps", "verified_undo", "no_verified_undo") + }, + } + summary_path.write_text(json.dumps(summary, indent=2) + "\n", encoding="utf-8") + print(json.dumps(summary["totals"])) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/packages/flow/docs/AFTER_A_STOP.md b/packages/flow/docs/AFTER_A_STOP.md index 3779b381da..38b2cd0d0e 100644 --- a/packages/flow/docs/AFTER_A_STOP.md +++ b/packages/flow/docs/AFTER_A_STOP.md @@ -6,16 +6,19 @@ application. Flow doesn't hide that or guess about it. It tells a person what may need undoing, flags programs that write before they check, and runs a declared undo only after a person approves it. -This page covers three parts: +This page covers four parts: - [The change ledger](#the-change-ledger): what a stopped run may have changed. - [Write-ordering checks](#write-ordering-checks): writes that come before a check the program could have done first. - [Declared undo steps](#declared-undo-steps): a compensating step that a person approves and Flow verifies. +- [Proposed undo steps](#proposed-undo-steps): undo steps Flow finds for each + saving step and tries on a test copy before a person approves them. -The code is in `openadapt_flow/after_stop.py` and `openadapt_flow/cli_undo.py`. -The end-to-end tests are in `tests/test_after_stop_mockmed.py`. +The code is in `openadapt_flow/after_stop.py`, `openadapt_flow/undo_proposal.py` +and `openadapt_flow/cli_undo.py`. The end-to-end tests are in +`tests/test_after_stop_mockmed.py` and `tests/test_undo_proposal.py`. ## The change ledger @@ -202,10 +205,138 @@ assert result.status == "verified" `approve_undo` is the record a person's approval produces. Call it only from a path where a person made that decision. +## Proposed undo steps + +Writing every undo by hand doesn't scale. Most business apps already have an +inverse path: create and delete, add a line and remove it, set a field and set +it back. `undo propose` looks for that path for each saving step, tries each +candidate on a test copy, and writes only the ones that pass into the bundle, +as proposals. A person approves a proposal once. After that, Flow offers it +after a stop like a declared undo, and each run of it still needs a person's +approval at the time. + +### Where the candidates come from + +For each saving step with a bound record check (a `record_written` or +`field_equals` effect), Flow looks in four places: + +| Source | What Flow proposes | +| --- | --- | +| Other recordings (`--from-recording`) | A compiled recording of a person deleting, cancelling, voiding or removing a record. Its steps are used as recorded. If none of them declares an effect, Flow adds the save's inverse: the record the save created is gone. | +| Captured traffic (`--recording`, from `record --capture-network`) | If the recording sent a `DELETE` to the record the save created, that request. Otherwise, for a JSON `POST` whose response carried a record id, a `DELETE` to that record's URL (REST convention). For a JSON `PUT` or `PATCH` that edits a field, the same request with the field's old value. | +| The field's old value | For a field edit typed from a parameter, the same steps again with the value read back before the change. | +| The app's own pages | A Delete, Cancel, Void or Remove control on a page loaded after the save. Flow reports it but doesn't build steps from it; record a person using it and pass that recording. | + +A browser form post isn't turned into an API undo: it needs the live page's +session token. Record a person doing the undo instead. + +### How a candidate is verified + +Only on a target the deployment file declares a test copy, with the same gate +as `induce --probe-on` ([PROBE_INDUCTION.md](PROBE_INDUCTION.md)): +`test_copy.declared: true`, a reset (`reset_url` or `reset_command`), a reward +contract for the read-back, no production runtime profile, and a fresh +session for every run. + +For each candidate, Flow: + +1. Resets the copy and reads every record through the reward contract's + oracle. When earlier steps also save, it first runs the steps ahead of the + save, reads every record, and resets, so "before" is the state just before + this save. +2. Runs the workflow up to and including the save, with the deployment's own + effect verifier, and reads every record again. The save must complete and + must change something; otherwise there is nothing to undo. +3. Runs the candidate undo in a new session, the way `undo run` would. The + undo's own declared effect must be confirmed by an independent read-back. +4. Reads every record again and compares them with step 1. The candidate + passes only if they are the same records with the same values: the saved + record is back as it was, and no other record changed (the collateral + guard). `--ignore-field` skips a field such as a modified-at timestamp. +5. Resets the copy. + +Each candidate's evidence (digests and counts of the three snapshots, how each +run ended, and the reason) is in the report. A failed candidate is reported +with its reason and never written. For example: + +```text +2 saving step(s): 1 with a verified undo, 1 without. + save_triage: verified undo undo-save_triage-network-1 + authorize: no verified undo. network: after the undo the test copy differs from before the save: 1 record(s) added since before the save remain +``` + +The evidence file holds no record values. The report names fields, never +values. + +### Finding the record at undo time + +An undo that deletes "the record the save created" needs that record's id, +which the app assigns during the save. A proposed undo can carry: + +- `locate`: before the save runs, the replayer keeps the ids of the records + that already match the save's record check. At undo time, the one matching + record that isn't in that list is the one the save created, and its id is + bound to the parameter `undo_record_id`. No such record, or more than one, + refuses the undo before anything runs. +- `restore`: before the save runs, the replayer keeps the old value of each + edited field from the one matching record. The undo writes it back. + +The replayer writes these values to `/undo/prior/` from the +pre-action read it already makes for the save's record check, and only for a +step whose bundle holds an undo with `locate` or `restore` when the run +starts. A run made before the proposal was written has no such values, and +its undo refuses. The file is +owner-only, like `params.json`, and it holds record values, so treat the run +directory as sensitive. They are never copied into the report. `undo run` reads +the system of record through the deployment file's effect verifier +(`--config`) to find the record. + +### Propose, approve once, run with approval + +```bash +openadapt-flow undo propose bundle --config test-copy.yaml --recording rec --from-recording delete-rec-bundle --report propose.json +openadapt-flow undo accept bundle --undo undo-save_triage-network-1 +``` + +`undo propose` without `--config` lists the candidates and runs nothing. +Written proposals carry their evidence in the bundle under `undo-evidence/`, +bound by its SHA-256. Writing them re-seals the bundle and marks any +certification expired, as `undo declare` does. + +`undo accept` works only at an interactive terminal. It shows the undo, why it +was proposed and where it was verified, and the person types the undo id. It +refuses when the evidence file is missing or changed, or the undo changed +since it was verified. Until then the proposal is in the bundle but Flow +doesn't offer it, and the change ledger names no undo for the step. + +After a stop, the approved proposal is offered, approved and run exactly as in +[Offer, approve, run, verify](#offer-approve-run-verify). + +### Measured coverage + +`benchmark/undo_proposal/` records each synthetic app's canonical +demonstration in a real browser with network capture, then runs +`undo propose` against the app as a test copy. + +MEASUREMENT_TABLE + ## What this doesn't do - **No automatic rollback.** Flow doesn't undo anything by itself, and there - is no transaction across a GUI workflow. + is no transaction across a GUI workflow. A proposed undo runs only on a test + copy until a person approves it, and then only after a stop, with a + person's approval at that time. +- **Proposals need a bound record check.** A save whose record effect is a + placeholder gets no candidate. +- **Exact restore only.** A candidate passes only if every record is back as + it was. An app that cancels by changing a status, or that writes an audit + row on delete, fails unless those fields are ignored with + `--ignore-field`. Ignoring a field is the person's call. +- **One test-copy run per candidate.** A pass shows the undo worked on that + copy, with the bundle's parameters, once. It isn't a proof for every + record. +- **No undo built from an app control.** A Delete or Cancel control on the + saved record's page is reported, not driven. Record a person using it. - **No network write buffering.** Flow doesn't hold writes back until the whole workflow has passed its checks. - **No undo in a workflow with a loop.** An undo runs with the run's base diff --git a/packages/flow/docs/LIMITS.md b/packages/flow/docs/LIMITS.md index 90f33aa90d..83dd599665 100644 --- a/packages/flow/docs/LIMITS.md +++ b/packages/flow/docs/LIMITS.md @@ -529,12 +529,32 @@ What it does provide ([`AFTER_A_STOP.md`](AFTER_A_STOP.md)): to a person, runs only after that person approves it, runs at most once, and counts as done only when an independent read of the system of record confirms its own declared effects. +- `undo propose` finds candidate undo steps for each saving step (another + recording, captured traffic, or the field's old value) and tries each one + on a declared test copy. It writes a candidate into the bundle, as a + proposal, only when every record afterwards matches the read-back from + before the save. A person approves a proposal once (`undo accept`), and + each run still needs approval after the stop. The limits of that: - A step with no declared effect is `unknown` in the ledger. Flow can't say whether it changed anything. -- An undo exists only where someone declared it. Flow doesn't infer one. +- An undo exists only where someone declared it or approved a verified + proposal. Flow never uses an undo no person approved. +- A proposal is verified once, on a test copy, with the bundle's parameters. + It is evidence that the inverse path works on that copy, not a proof for + every record in production. +- A proposal must restore every record exactly. An app that cancels by + changing a status, or writes an audit row when a record is deleted, fails + verification unless a person names those fields with `--ignore-field`. +- Flow builds API undo steps only from JSON requests. A browser form post, or + a Delete control on the saved record's page, needs a recording of a person + doing the undo. +- A proposal that finds its record (`locate`) or restores old values + (`restore`) needs values the replayer keeps just before the save. They exist + only for runs whose bundle already held the proposal, and only when the + save's record check read the system of record before the save. - An undo can fail or be refused. Then a person must repair the record by hand. - No undo is offered for a workflow with a loop: the undo can't be bound to diff --git a/packages/flow/openadapt_flow/after_stop.py b/packages/flow/openadapt_flow/after_stop.py index edf05bb923..ad5e55f9e3 100644 --- a/packages/flow/openadapt_flow/after_stop.py +++ b/packages/flow/openadapt_flow/after_stop.py @@ -35,7 +35,7 @@ from dataclasses import dataclass, field from datetime import datetime, timezone from pathlib import Path -from typing import TYPE_CHECKING, Any, Literal, Optional +from typing import TYPE_CHECKING, Any, Callable, Literal, Mapping, Optional from pydantic import BaseModel, Field @@ -170,7 +170,9 @@ def build_change_ledger( steps_by_id = {step.id: step for step in iter_workflow_steps(workflow)} undo_by_target: dict[str, str] = {} for undo in workflow.undo_steps: - undo_by_target.setdefault(undo.undoes, undo.id) + # A proposal no person has approved is not an undo yet. + if not undo.awaiting_approval: + undo_by_target.setdefault(undo.undoes, undo.id) ledger: list[ChangeLedgerEntry] = [] position = 0 @@ -468,7 +470,7 @@ class UndoOffer(BaseModel): undo_id: str undoes: str intent: str - source: Literal["demonstration", "hand_written"] + source: Literal["demonstration", "hand_written", "synthesized"] #: The ledger entry that makes this undo relevant. ledger_entry: ChangeLedgerEntry #: SHA-256 of the run's report.json bytes. @@ -648,7 +650,14 @@ def offer_undos( for entry in sorted(report.change_ledger, key=lambda e: -e.position): if not entry.may_need_undo or runs_per_step[entry.step_id] > 1: continue - undo = next((u for u in workflow.undo_steps if u.undoes == entry.step_id), None) + undo = next( + ( + u + for u in workflow.undo_steps + if u.undoes == entry.step_id and not u.awaiting_approval + ), + None, + ) if undo is None: continue digest = hashlib.sha256( @@ -693,8 +702,9 @@ def _find_offer( why if why else "the run finished, the workflow has a loop, the system of " - "record proved the step's change absent, or the bundle declares no " - "such undo" + "record proved the step's change absent, the bundle declares no " + "such undo, or it is a proposal no person has approved yet " + "(openadapt-flow undo accept)" ) ) @@ -798,17 +808,198 @@ def undo_workflow(workflow: Workflow, undo: UndoStep) -> Workflow: ) +# --------------------------------------------------------------------------- +# Values kept before the save, for an undo that locates or restores +# --------------------------------------------------------------------------- + +PRIOR_DIRNAME = "prior" +_PRIOR_SCHEMA = "openadapt.undo-prior/v1" + +RecordsReader = Callable[[], Optional[list[dict[str, Any]]]] + + +def _resolved_selector( + match: dict[str, Any], params: Mapping[str, Any] +) -> Optional[dict[str, str]]: + """``match`` bound to ``params``; None when a value is missing.""" + from openadapt_flow.runtime.effects.effect import Effect, EffectKind + + effect = Effect(kind=EffectKind.RECORD_WRITTEN, match=match) + out: dict[str, str] = {} + for key, expr in effect.match.items(): + value = expr.resolve(params) + if value is None: + return None + out[key] = value + return out + + +def _matching( + records: list[dict[str, Any]], selector: dict[str, str] +) -> list[dict[str, Any]]: + from openadapt_flow.runtime.effects.effect import record_matches + + return [r for r in records if record_matches(r, selector)] + + +def prior_state_path(run_dir: Path | str, step_id: str) -> Path: + digest = hashlib.sha256(step_id.encode("utf-8")).hexdigest()[:16] + return Path(run_dir) / UNDO_DIRNAME / PRIOR_DIRNAME / f"{digest}.json" + + +def keep_prior_state( + workflow: Workflow, + step: Step, + records: Optional[list[dict[str, Any]]], + params: Mapping[str, Any], + run_dir: Path | str, +) -> Optional[Path]: + """Keep what an undo of ``step`` needs, read just before the save. + + Called by the replayer with the system-of-record records it read for the + step's pre-action baseline. Writes only when the bundle has an undo for + ``step`` that locates or restores. For ``locate``: the ids of records + that already matched, so the undo can tell the new one. For + ``restore``: the old values of the named fields, from the one matching + record. The file holds record values, like ``params.json``: it is + created owner-only and never copied into the report. + + Never raises: a run is not stopped because an undo can't be prepared. + The undo then refuses, and a person repairs the record by hand. + """ + undos = [ + u + for u in workflow.undo_steps + if u.undoes == step.id and (u.locate is not None or u.restore is not None) + ] + if not undos or records is None: + return None + try: + payload: dict[str, Any] = {"schema": _PRIOR_SCHEMA, "step_id": step.id} + existing: dict[str, list[str]] = {} + restored: dict[str, dict[str, Any]] = {} + for undo in undos: + if undo.locate is not None: + selector = _resolved_selector(undo.locate.match, params) + if selector is not None: + existing[undo.id] = sorted( + str(r.get(undo.locate.field)) + for r in _matching(records, selector) + ) + if undo.restore is not None: + selector = _resolved_selector(undo.restore.match, params) + rows = _matching(records, selector) if selector is not None else [] + if len(rows) == 1: + restored[undo.id] = { + name: rows[0].get(name) for name in undo.restore.fields + } + payload["existing_ids"] = existing + payload["restore"] = restored + path = prior_state_path(run_dir, step.id) + path.parent.mkdir(parents=True, exist_ok=True) + staging = path.with_name(f".{path.name}.{os.getpid()}") + fd = os.open(staging, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as handle: + json.dump(payload, handle) + os.replace(staging, path) + return path + except Exception: # noqa: BLE001 - never stop a run over undo bookkeeping + return None + + +def read_prior_state(run_dir: Path | str, step_id: str) -> Optional[dict[str, Any]]: + path = prior_state_path(run_dir, step_id) + if not path.is_file(): + return None + try: + raw = json.loads(path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + return None + if not isinstance(raw, dict) or raw.get("schema") != _PRIOR_SCHEMA: + return None + return raw + + +def needs_records(undo: UndoStep) -> bool: + """Whether running ``undo`` needs a fresh read of the system of record.""" + return undo.locate is not None + + +def resolve_undo_params( + undo: UndoStep, + run_dir: Path | str, + params: Mapping[str, Any], + read_records: Optional[RecordsReader] = None, +) -> dict[str, Any]: + """The run's params plus what ``undo.locate`` and ``undo.restore`` bind. + + Raises :class:`UndoRefused` when a value can't be found: no values were + kept before the save, the record the save created can't be told apart, + or the system of record can't be read. Nothing has run at that point. + """ + out = dict(params) + if undo.locate is None and undo.restore is None: + return out + prior = read_prior_state(run_dir, undo.undoes) + if prior is None: + raise UndoRefused( + f"the run kept no values before step {undo.undoes!r} for undo " + f"{undo.id!r} (the step ran without a record read-back, or before " + "this undo was in the bundle); repair the record by hand" + ) + if undo.restore is not None: + values = (prior.get("restore") or {}).get(undo.id) + if not isinstance(values, dict) or set(values) != set(undo.restore.fields): + raise UndoRefused( + f"the run could not read one record's old values before step " + f"{undo.undoes!r}; undo {undo.id!r} has nothing to restore" + ) + for field_name, param in undo.restore.fields.items(): + value = values[field_name] + out[param] = "" if value is None else str(value) + if undo.locate is not None: + existing = (prior.get("existing_ids") or {}).get(undo.id) + selector = _resolved_selector(undo.locate.match, params) + if not isinstance(existing, list) or selector is None: + raise UndoRefused( + f"the run kept no record list before step {undo.undoes!r}; " + f"undo {undo.id!r} can't tell which record the step created" + ) + records = read_records() if read_records is not None else None + if records is None: + raise UndoRefused( + "the system of record could not be read, so the record the " + f"step {undo.undoes!r} created can't be found; nothing ran" + ) + new = [ + r + for r in _matching(records, selector) + if str(r.get(undo.locate.field)) not in set(existing) + ] + if len(new) != 1: + raise UndoRefused( + f"{len(new)} record(s) match step {undo.undoes!r}'s record and " + "were not there before it ran; undo " + f"{undo.id!r} needs exactly one. Repair the record by hand" + ) + out[undo.locate.param] = str(new[0].get(undo.locate.field)) + return out + + def prepare_undo_run( run_dir: Path | str, workflow: Workflow, undo_id: str, *, bundle_dir: Optional[Path | str] = None, + read_records: Optional[RecordsReader] = None, ) -> PreparedUndo: """Check the approval and claim it. Raises :class:`UndoRefused` otherwise. The claim is written with exclusive create, so an approval is used at - most once even when two processes race. + most once even when two processes race. An undo that locates or restores + resolves its values first (``read_records`` reads the system of record), + so a refusal there leaves the approval unused. """ run_dir = Path(run_dir) offer = _find_offer(run_dir, workflow, undo_id, bundle_dir) @@ -830,6 +1021,9 @@ def prepare_undo_run( "offer (the report, bundle or undo changed since); a person must " "approve it again" ) + undo = next(u for u in workflow.undo_steps if u.id == undo_id) + report, _ = _load_report(run_dir) + params = resolve_undo_params(undo, run_dir, report.params, read_records) started = _now() try: _write_once( @@ -841,14 +1035,12 @@ def prepare_undo_run( f"undo {undo_id!r} was already run with this approval; check " f"{_undo_dir(run_dir, undo_id) / RESULT_FILENAME}" ) from exc - undo = next(u for u in workflow.undo_steps if u.id == undo_id) - report, _ = _load_report(run_dir) return PreparedUndo( offer=offer, approval=approval, undo=undo, workflow=undo_workflow(workflow, undo), - params=dict(report.params), + params=params, undo_run_dir=_undo_dir(run_dir, undo_id) / UNDO_RUN_DIRNAME, started_at=started, ) @@ -857,12 +1049,23 @@ def prepare_undo_run( def _judge_undo( prepared: PreparedUndo, report: Optional[RunReport] ) -> tuple[bool, str]: + return judge_undo_report(prepared.undo, report) + + +def judge_undo_report(undo: UndoStep, report: Optional[RunReport]) -> tuple[bool, str]: + """Whether ``report`` shows ``undo`` ran in full and was confirmed. + + Every undo step must have run and succeeded, and every step that + declares an effect must be confirmed by an independent read of the + system of record. ``undo propose`` judges its test-copy runs with this + same function. + """ from openadapt_flow.transaction import _effect_evidence_has_exact_coverage if report is None: return False, "the undo run left no report" by_id = {result.step_id: result for result in report.results} - for step in prepared.undo.steps: + for step in undo.steps: result = by_id.get(step.id) if result is None: return False, f"undo step {step.id!r} did not run" @@ -923,6 +1126,23 @@ def finish_undo_run( return result +def records_reader(verifier: Any) -> Optional[RecordsReader]: + """Read every record through an effect verifier's own read path.""" + if verifier is None or not callable(getattr(verifier, "capture_pre_state", None)): + return None + + def read() -> Optional[list[dict[str, Any]]]: + try: + state = verifier.capture_pre_state() + except Exception: # noqa: BLE001 - an unreadable store is not a guess + return None + if getattr(state, "reachable", False) is not True: + return None + return [dict(r) for r in state.records] + + return read + + def run_undo( run_dir: Path | str, workflow: Workflow, @@ -937,7 +1157,13 @@ def run_undo( approval was already used. The undo runs through the normal runtime, so identity checks, the risk gate and effect verification all apply. """ - prepared = prepare_undo_run(run_dir, workflow, undo_id, bundle_dir=bundle_dir) + prepared = prepare_undo_run( + run_dir, + workflow, + undo_id, + bundle_dir=bundle_dir, + read_records=records_reader(getattr(replayer, "effect_verifier", None)), + ) report: Optional[RunReport] = None try: report = replayer.run( @@ -1024,8 +1250,6 @@ def declare_undo( needs certifying again), and approvals bound to the old digest no longer match. """ - from openadapt_flow.bundle_sealing import _invalidate_prior_certification - bundle_dir = Path(bundle_dir) workflow = Workflow.load(bundle_dir) if any(existing.id == undo.id for existing in workflow.undo_steps): @@ -1044,14 +1268,35 @@ def declare_undo( for rel, source in (assets or {}).items(): (bundle_dir / rel).parent.mkdir(parents=True, exist_ok=True) shutil.copyfile(source, bundle_dir / rel) + return save_undo_steps( + bundle_dir, + workflow, + [*workflow.undo_steps, undo], + reason="an undo step was declared after certification", + ) + + +def save_undo_steps( + bundle_dir: Path | str, + workflow: Workflow, + undo_steps: list[UndoStep], + *, + reason: str, +) -> Workflow: + """Replace the bundle's undo steps and re-seal it. + + Runs made with the bundle as it was still match it afterwards (the old + digest goes into ``undo-lineage.json``). A prior certification is marked + expired with ``reason``. + """ + from openadapt_flow.bundle_sealing import _invalidate_prior_certification + + bundle_dir = Path(bundle_dir) if workflow.manifest is not None and workflow.manifest.content_digest: - # Runs made with the bundle as it was still match it afterwards. _record_lineage(bundle_dir, workflow.manifest.content_digest) - workflow.undo_steps = [*workflow.undo_steps, undo] + workflow.undo_steps = list(undo_steps) if workflow.manifest is not None: - _invalidate_prior_certification( - workflow, reason="an undo step was declared after certification" - ) + _invalidate_prior_certification(workflow, reason=reason) # The template is rebuilt from the certification on every load; with # the certification gone it can't be, so it goes too. workflow.manifest.provenance.governed_authorization_template = None @@ -1076,6 +1321,13 @@ def declare_undo( "change_ledger_lines", "declare_undo", "finish_undo_run", + "judge_undo_report", + "keep_prior_state", + "needs_records", + "prior_state_path", + "read_prior_state", + "records_reader", + "resolve_undo_params", "offer_undos", "prepare_undo_run", "LINEAGE_FILENAME", @@ -1083,6 +1335,7 @@ def declare_undo( "read_undo_approval", "read_undo_result", "run_undo", + "save_undo_steps", "steps_from_demonstration", "undo_declaration_problems", "undo_workflow", diff --git a/packages/flow/openadapt_flow/cli_undo.py b/packages/flow/openadapt_flow/cli_undo.py index 9249a0e7d7..7232b56a51 100644 --- a/packages/flow/openadapt_flow/cli_undo.py +++ b/packages/flow/openadapt_flow/cli_undo.py @@ -1,7 +1,9 @@ """CLI: ``openadapt-flow undo`` -- declared undo steps after a stop. -``declare`` adds an undo to a bundle. ``list`` shows what a stopped run -offers. ``approve`` records a person's approval at an interactive terminal. +``declare`` adds an undo to a bundle. ``propose`` finds an undo for each +saving step, tries it on a declared test copy and writes the ones that pass +as proposals; ``accept`` is a person's one-time approval of a proposal. +``list`` shows what a stopped run offers. ``approve`` records a person's approval at an interactive terminal. ``run`` runs one approved undo, once, through the normal replay path and checks its declared effects in the system of record. See ``docs/AFTER_A_STOP.md``. @@ -18,14 +20,17 @@ from typing import Callable, Literal, Optional, Sequence from openadapt_flow.after_stop import ( + RecordsReader, UndoRefused, approve_undo, declare_undo, finish_undo_run, + needs_records, offer_undos, prepare_undo_run, read_undo_approval, read_undo_result, + records_reader, steps_from_demonstration, ) from openadapt_flow.cli_hints import command @@ -85,6 +90,67 @@ def register( ) p.set_defaults(func=_cmd_undo_declare) + p = verbs.add_parser( + "propose", + help=( + "Find an undo for each saving step, verify it on a declared test " + "copy, and write the verified ones as proposals" + ), + ) + p.add_argument("bundle", help="Workflow bundle directory") + p.add_argument( + "--config", + dest="test_copy_config", + default=None, + help=( + "Deployment file that declares a test copy (test_copy.declared: " + "true). Without it, candidates are listed and nothing runs" + ), + ) + p.add_argument( + "--recording", + default=None, + help="The bundle's own recording directory (its network.jsonl)", + ) + p.add_argument( + "--from-recording", + action="append", + default=[], + dest="from_recordings", + help=( + "A compiled bundle of a person deleting or cancelling a record (repeatable)" + ), + ) + p.add_argument( + "--ignore-field", + action="append", + default=[], + dest="ignore_fields", + help=( + "A record field the before/after comparison skips, such as a " + "modified-at timestamp (repeatable)" + ), + ) + p.add_argument("--work-dir", default=None, help="Keep the test-copy runs here") + p.add_argument( + "--report", default=None, help="Write the full report (JSON) to this path" + ) + p.add_argument( + "--no-write", + action="store_true", + help="Verify, but don't write the proposals into the bundle", + ) + p.add_argument("--json", action="store_true", help="Print the report as JSON") + p.set_defaults(func=_cmd_undo_propose) + + p = verbs.add_parser( + "accept", + help="A person approves a proposed undo once (interactive terminal only)", + ) + p.add_argument("bundle", help="Workflow bundle directory") + p.add_argument("--undo", required=True, dest="undo_id", help="Undo id") + p.set_defaults(func=_cmd_undo_accept) + p = verbs.add_parser("list", help="Show the undos a stopped run offers") p.add_argument("run_dir", help="The stopped run's directory") p.add_argument("--bundle", required=True, help="The run's bundle directory") @@ -289,6 +355,168 @@ def _cmd_undo_approve(args: argparse.Namespace) -> int: return 0 +def _records_reader_for( + args: argparse.Namespace, workflow: Workflow, run_dir: Path +) -> Optional[RecordsReader]: + """A read of the system of record for an undo that locates its record. + + Built from the deployment file's effect verifier (``--config``), the same + read the undo's own check uses. None when the undo needs no read or no + deployment file names one; ``prepare_undo_run`` then refuses. + """ + undo = next((u for u in workflow.undo_steps if u.id == args.undo_id), None) + config = getattr(args, "config", None) + if undo is None or not needs_records(undo) or not config: + return None + from openadapt_flow.deployment import build_effect_verifier, load_deployment + + try: + report = RunReport.model_validate_json( + (run_dir / "report.json").read_text(encoding="utf-8") + ) + cfg = load_deployment(Path(config)) + return records_reader(build_effect_verifier(cfg.effects, report.params)) + except (OSError, ValueError): + return None + + +def _cmd_undo_propose(args: argparse.Namespace) -> int: + from openadapt_flow.compiler.probing import probe_target_refusals + from openadapt_flow.deployment import load_deployment + from openadapt_flow.undo_proposal import ( + DeploymentUndoTestCopy, + Sources, + load_network_lines, + propose_undos, + write_proposals, + ) + + bundle = Path(args.bundle) + workflow = Workflow.load(bundle) + target = None + if args.test_copy_config: + try: + cfg = load_deployment(Path(args.test_copy_config)) + except (FileNotFoundError, ValueError) as exc: + print(f"undo propose REFUSED: {exc}", file=sys.stderr) + return 3 + reasons = probe_target_refusals(cfg) + if reasons: + print( + "undo propose REFUSED: nothing was reset or run. A candidate " + "undo is tried only on a declared test copy:", + file=sys.stderr, + ) + for reason in reasons: + print(f" - {reason}", file=sys.stderr) + return 3 + target = DeploymentUndoTestCopy( + cfg, deployment_path=Path(args.test_copy_config) + ) + try: + sources = Sources( + recordings=[Path(p) for p in args.from_recordings], + network_lines=load_network_lines(args.recording), + ) + report, verified = propose_undos( + workflow, + bundle_dir=bundle, + sources=sources, + target=target, + work_dir=Path(args.work_dir) if args.work_dir else None, + ignore_fields=args.ignore_fields, + ) + except ValueError as exc: + print(f"undo propose REFUSED: {exc}", file=sys.stderr) + return 2 + if verified and not args.no_write: + try: + write_proposals(bundle, verified) + except ValueError as exc: + print(f"undo propose: nothing written: {exc}", file=sys.stderr) + return 2 + report.written = True + payload = report.model_dump(mode="json", by_alias=True) + payload["summary"] = report.summary() + if args.report: + Path(args.report).write_text( + json.dumps(payload, indent=2, sort_keys=True), encoding="utf-8" + ) + if args.json: + print(json.dumps(payload, indent=2, sort_keys=True)) + return 0 + summary = report.summary() + print( + f"{summary['saving_steps']} saving step(s): {summary['verified_undo']} " + f"with a verified undo, {summary['no_verified_undo']} without." + ) + for row in report.saving_steps: + if row.verified_undo: + print(f" {row.step_id}: verified undo {row.verified_undo}") + else: + print(f" {row.step_id}: no verified undo. {row.reason}") + if report.written: + quoted = shlex.quote(str(bundle)) + print( + "Written as proposals. Flow offers none of them until a person " + "approves each one once: " + + command(f"undo accept {quoted} --undo ") + ) + return 0 + + +def _cmd_undo_accept(args: argparse.Namespace) -> int: + import getpass + + from openadapt_flow.undo_proposal import accept_proposal, proposal_problem + + bundle = Path(args.bundle) + if not (sys.stdin.isatty() and sys.stdout.isatty()): + print( + "undo accept needs a person at an interactive terminal; an agent " + "or script can't approve an undo. Nothing was approved.", + file=sys.stderr, + ) + return 2 + workflow = Workflow.load(bundle) + undo = next((u for u in workflow.undo_steps if u.id == args.undo_id), None) + if undo is None: + print(f"The bundle has no undo {args.undo_id!r}.", file=sys.stderr) + return 1 + problem = proposal_problem(bundle, undo) + if problem: + print(f"Not approved: {problem}.", file=sys.stderr) + return 1 + assert undo.proposal is not None + print(f"Proposed undo {undo.id}: {undo.intent}") + print(f"It undoes step `{undo.undoes}`. Why it was proposed: {undo.proposal.basis}") + print( + f"Verified on test copy {undo.proposal.test_copy!r} at " + f"{undo.proposal.verified_at} (evidence: {undo.proposal.evidence_path})." + ) + print("It runs these steps:") + for step in undo.steps: + print(f" - {step.id}: {step.intent}") + print( + "Approving makes Flow offer it after a stop. Each run still needs a " + "person's approval then (undo approve)." + ) + typed = input(f"Type the undo id ({undo.id}) to approve it, or press Enter: ") + if typed.strip() != undo.id: + print("Not approved. Nothing was recorded.") + return 1 + try: + accept_proposal(bundle, undo.id, approver=getpass.getuser()) + except ValueError as exc: + print(f"Not approved: {exc}", file=sys.stderr) + return 1 + print( + "Approved. The bundle was re-sealed and any certification is now " + "expired: certify it again before production use." + ) + return 0 + + def _cmd_undo_run(args: argparse.Namespace) -> int: run_dir = Path(args.run_dir) workflow = Workflow.load(args.bundle) @@ -305,7 +533,11 @@ def _cmd_undo_run(args: argparse.Namespace) -> int: return 2 try: prepared = prepare_undo_run( - run_dir, workflow, args.undo_id, bundle_dir=args.bundle + run_dir, + workflow, + args.undo_id, + bundle_dir=args.bundle, + read_records=_records_reader_for(args, workflow, run_dir), ) except UndoRefused as exc: print(f"undo run REFUSED: {exc}. Nothing was executed.", file=sys.stderr) diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index 1b64ae8ecd..67e5e441d1 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -1843,6 +1843,84 @@ def _validate_rich_action_contract(self) -> "Step": # effect/risk machinery), a transition's guard IS a Phase-1 ``Predicate``, and a # branch reuses the SAME model-free predicate evaluation. BACKWARD-COMPATIBLE: UNDO_ID_PATTERN: Final[str] = r"^[A-Za-z0-9][A-Za-z0-9_.-]{0,63}$" +UNDO_PARAM_PATTERN: Final[str] = r"^[A-Za-z_][A-Za-z0-9_]{0,63}$" + + +class UndoLocate(BaseModel): + """How an undo finds the record the save created. + + Before the save runs, the replayer keeps the ids of the records that + already match :attr:`match` (``/undo/prior/.json``). At undo + time, the one matching record whose id is not in that list is the record + the save created; its id is bound to the run parameter :attr:`param`. + Zero or several such records refuse the undo. See docs/AFTER_A_STOP.md. + """ + + model_config = ConfigDict(extra="forbid") + + #: The save's own effect selector (serialized ``ValueExpr`` values). + match: dict[str, Any] = Field(min_length=1) + #: The record field that holds the record's id. + field: str = Field(default="id", min_length=1, max_length=128) + #: The run parameter the id is bound to. + param: str = Field(default="undo_record_id", pattern=UNDO_PARAM_PATTERN) + + +class UndoRestore(BaseModel): + """Which values to keep before a field edit so its undo can restore them. + + Before the save runs, the replayer reads the one record matching + :attr:`match` and keeps each named field's value. At undo time each value + is bound to its run parameter, and the undo writes it back. + """ + + model_config = ConfigDict(extra="forbid") + + match: dict[str, Any] = Field(min_length=1) + #: Record field -> the run parameter the undo reads its old value from. + fields: dict[str, str] = Field(min_length=1) + + @field_validator("fields") + @classmethod + def _param_names(cls, value: dict[str, str]) -> dict[str, str]: + for name in value.values(): + if not re.match(UNDO_PARAM_PATTERN, name): + raise ValueError(f"{name!r} is not a valid undo parameter name") + return value + + +class UndoProposal(BaseModel): + """A compiler-proposed undo that passed verification on a test copy. + + Flow offers it after a stop only once a person has approved it + (:attr:`approved_by`), and every run of it still needs that person's + approval at the time it runs. ``evidence_sha256`` binds the verification + record kept in the bundle (``evidence_path``). + """ + + model_config = ConfigDict(extra="forbid") + + #: Where the candidate came from: ``recording`` (another recording), + #: ``network`` (captured network traffic) or ``field_restore`` (the + #: value read back before the change). + candidate_source: Literal["recording", "network", "field_restore"] + #: Plain words on why it was proposed; never record values. + basis: str = Field(min_length=1, max_length=1024) + test_copy: str = Field(min_length=1, max_length=256) + verified_at: str + evidence_path: str = Field(min_length=1, max_length=512) + evidence_sha256: str = Field(pattern=r"^[0-9a-f]{64}$") + approved_by: Optional[str] = Field(default=None, min_length=1, max_length=256) + approved_at: Optional[str] = None + + @model_serializer(mode="wrap") + def _serialize_compatible(self, handler: Any) -> dict[str, Any]: + data: dict[str, Any] = handler(self) + if self.approved_by is None: + data.pop("approved_by", None) + if self.approved_at is None: + data.pop("approved_at", None) + return data class UndoStep(BaseModel): @@ -1865,11 +1943,33 @@ class UndoStep(BaseModel): #: Plain words a person reads before approving. intent: str = Field(min_length=1, max_length=512) #: ``demonstration``: compiled from a recording of a person doing the - #: undo. ``hand_written``: written by an author. - source: Literal["demonstration", "hand_written"] + #: undo. ``hand_written``: written by an author. ``synthesized``: built by + #: ``undo propose`` from captured traffic or a field's old value. + source: Literal["demonstration", "hand_written", "synthesized"] #: The compensating actions, run in order by the normal runtime (identity #: checks, risk gate and effect verification all apply). steps: list[Step] = Field(min_length=1) + #: Find the record the save created (binds its id to a run parameter). + locate: Optional[UndoLocate] = None + #: Keep a field's old value before the save so the undo can restore it. + restore: Optional[UndoRestore] = None + #: Set by ``undo propose``: verified on a test copy, waiting for (or + #: holding) a person's one-time approval. None for a declared undo. + proposal: Optional[UndoProposal] = None + + @property + def awaiting_approval(self) -> bool: + """True for a proposed undo no person has approved yet.""" + return self.proposal is not None and self.proposal.approved_by is None + + @model_serializer(mode="wrap") + def _serialize_compatible(self, handler: Any) -> dict[str, Any]: + """Keep declared undos byte-identical (the bundle digest covers them).""" + data: dict[str, Any] = handler(self) + for name in ("locate", "restore", "proposal"): + if getattr(self, name) is None: + data.pop(name, None) + return data @model_validator(mode="after") def _requires_a_checkable_result(self) -> "UndoStep": diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index fe41059052..d8a2ec736c 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -7056,6 +7056,8 @@ def _run_step_on_active_surface( so a linear run is byte-for-byte unchanged. """ t0 = time.monotonic() + # Where a locating or restoring undo keeps its pre-save values. + self._undo_prior_run_dir = run_dir evidence_step_id = self._new_step_evidence_key(run_dir, step.id) result = StepResult( step_id=step.id, @@ -8141,6 +8143,10 @@ def _run_step_on_active_surface( f"postcondition ({type(exc).__name__})" ) + if effect_pre_state is not None: + self._keep_undo_prior_state( + workflow, step, effect_pre_state, resolved_effects, params + ) # Independent system-of-record verification, AFTER the screen # postconditions passed: the screen oracle cannot see a partial # save, a phantom optimistic-UI success, a duplicate write, or a @@ -8687,6 +8693,7 @@ def _try_api_tier( "send the request — run aborted" ) return True + self._keep_undo_prior_state(workflow, step, before, effects, params) refusal = self._profile_effect_tier_refusal( workflow, step, @@ -9123,6 +9130,34 @@ def _capture_effect_pre_state(verifier: Any, effects: list["Effect"]) -> Any: ) return verifier.capture_pre_state() + def _keep_undo_prior_state( + self, + workflow: Workflow, + step: Step, + before: Any, + effects: list["Effect"], + params: Mapping[str, Any], + ) -> None: + """Keep what a locating or restoring undo of ``step`` needs. + + Reads only the pre-action baseline already captured; never raises and + never changes the step's result (``after_stop.keep_prior_state``). + """ + run_dir = getattr(self, "_undo_prior_run_dir", None) + if run_dir is None or not workflow.undo_steps or not effects: + return + try: + state = self._effect_pre_state_for(before, effects[0]) + except Exception: # noqa: BLE001 - bookkeeping never stops a run + return + if getattr(state, "reachable", None) is not True: + return + from openadapt_flow.after_stop import keep_prior_state + + keep_prior_state( + workflow, step, list(getattr(state, "records", []) or []), params, run_dir + ) + @staticmethod def _effect_pre_state_for(before: Any, effect: "Effect") -> Any: """Return one selected effect baseline, or a normal shared baseline.""" diff --git a/packages/flow/openadapt_flow/undo_proposal.py b/packages/flow/openadapt_flow/undo_proposal.py new file mode 100644 index 0000000000..a5db6bd0a7 --- /dev/null +++ b/packages/flow/openadapt_flow/undo_proposal.py @@ -0,0 +1,1414 @@ +"""Propose an undo for each saving step and verify it on a test copy. + +A declared undo (``after_stop``) needs a person to write or record it. Most +business apps already have an inverse path: create and delete, add a line and +remove it, set a field and set it back. This module finds candidates for that +path and keeps only the ones a test copy shows to work +(``docs/AFTER_A_STOP.md``, "Proposed undo steps"). + +1. **Propose.** For each saving step, :func:`find_candidates` looks in three + places: + + - ``recording``: other compiled recordings that delete, cancel or remove + something (a person doing the undo once); + - ``network``: the recording's captured traffic (``network.jsonl``). A + DELETE the recording sent to the record the save created, or, for a JSON + ``POST`` that returned an id, a ``DELETE`` to that record's URL. A + ``PUT``/``PATCH`` save gets the same request with the old values; + - ``field_restore``: for a field edit, the same steps again with the + value read back before the change. + + A control on the page after the save whose label is Delete, Cancel, Void + or Remove is reported (``app_ui``) but is not turned into steps: Flow + builds an undo from a control only from a recording of a person using it. + +2. **Verify on a test copy only.** :func:`verify_candidate` resets the copy, + reads every record, runs the save, runs the candidate undo, and reads every + record again. The candidate passes only when the undo itself ran and was + confirmed by the record read-back, and the records after the undo are the + records from before the save: the record the save touched and every other + record (the collateral guard). The evidence holds digests and counts, + never record values. + +3. **Approval stays.** :func:`write_proposals` writes each verified undo into + the bundle as a proposal. Flow offers it after a stop only after a person + approves it once (:func:`accept_proposal`, ``undo accept``), and each run + of it still needs that person's approval at that time (``undo approve``). + A candidate that fails is reported with the reason and never written. + +Nothing here runs against a target the deployment file doesn't declare a +test copy (``compiler.probing.probe_target_refusals``). +""" + +from __future__ import annotations + +import hashlib +import html +import json +import re +import shutil +import tempfile +import urllib.parse as up +from dataclasses import dataclass, field +from datetime import datetime, timezone +from pathlib import Path +from typing import Any, Literal, Mapping, Optional, Protocol, Sequence + +from pydantic import BaseModel, Field + +from openadapt_flow.after_stop import ( + UndoRefused, + judge_undo_report, + resolve_undo_params, + save_undo_steps, + steps_from_demonstration, + undo_workflow, +) +from openadapt_flow.ir import ( + ActionKind, + ApiBinding, + RunReport, + Step, + UndoLocate, + UndoProposal, + UndoRestore, + UndoStep, + Workflow, +) +from openadapt_flow.runtime.effects.effect import Effect, EffectKind, ValueExpr + +REPORT_SCHEMA = "openadapt.undo-propose-report/v1" +EVIDENCE_SCHEMA = "openadapt.undo-verification/v1" +EVIDENCE_DIRNAME = "undo-evidence" +LOCATE_PARAM = "undo_record_id" + +CandidateSource = Literal["recording", "network", "field_restore", "app_ui"] + +#: Words that name an inverse action in a recording name, a step intent or a +#: control label. +INVERSE_WORDS = re.compile( + r"\b(delete|remove|cancel|void|discard|undo|revert|withdraw|retract|" + r"deactivate|unlink)\b", + re.I, +) +_WRITE_METHODS = frozenset({"POST", "PUT", "PATCH"}) + + +def _now() -> str: + return datetime.now(timezone.utc).isoformat() + + +def _canonical(payload: Any) -> bytes: + return json.dumps( + payload, sort_keys=True, separators=(",", ":"), ensure_ascii=False, default=str + ).encode("utf-8") + + +def _sha256(payload: Any) -> str: + return hashlib.sha256(_canonical(payload)).hexdigest() + + +# --------------------------------------------------------------------------- +# Report models (no record values anywhere) +# --------------------------------------------------------------------------- + + +class CandidateResult(BaseModel): + candidate_id: str + source: CandidateSource + basis: str + #: ``verified``: passed on the test copy and written as a proposal. + #: ``failed``: ran on the test copy and did not pass. ``not_run``: could + #: not be built or run, or an earlier candidate for the step passed. + status: Literal["verified", "failed", "not_run"] + reason: str + evidence: Optional[dict[str, Any]] = None + + +class SavingStepResult(BaseModel): + step_id: str + intent: str + #: The proposal written for the step, if a candidate verified. + verified_undo: Optional[str] = None + #: Why the step has no verified undo (empty when it has one). + reason: str = "" + candidates: list[CandidateResult] = Field(default_factory=list) + + +class ProposeReport(BaseModel): + schema_: str = Field(default=REPORT_SCHEMA, alias="schema") + workflow_name: str + test_copy: Optional[str] = None + written: bool = False + saving_steps: list[SavingStepResult] = Field(default_factory=list) + + model_config = {"populate_by_name": True} + + @property + def verified_count(self) -> int: + return sum(1 for s in self.saving_steps if s.verified_undo) + + def summary(self) -> dict[str, int]: + return { + "saving_steps": len(self.saving_steps), + "verified_undo": self.verified_count, + "no_verified_undo": len(self.saving_steps) - self.verified_count, + } + + +# --------------------------------------------------------------------------- +# Candidates +# --------------------------------------------------------------------------- + + +@dataclass +class Candidate: + """One possible undo of one saving step.""" + + candidate_id: str + undoes: str + source: CandidateSource + basis: str + #: The undo to verify. None when it could not be built (``problem``). + undo: Optional[UndoStep] = None + #: Bundle-relative path -> source file, for crops a recording brings. + assets: dict[str, Path] = field(default_factory=dict) + problem: Optional[str] = None + + +def saving_steps(workflow: Workflow) -> list[tuple[int, Step]]: + """The straight-line workflow's consequential steps, with their index.""" + from openadapt_flow.run_gate import is_consequential + + return [ + (i, step) + for i, step in enumerate(workflow.steps) + if is_consequential(step, workflow, require_current_risk_certification=False) + ] + + +def record_effect(step: Step) -> Optional[Effect]: + """The save's checkable record effect: its first bound ``record_written`` + or ``field_equals`` effect (the API binding's when the step has none).""" + declared = list(step.effects) or ( + list(step.api_binding.effects) if step.api_binding is not None else [] + ) + for effect in declared: + if effect.needs_operator_confirmation: + continue + if effect.kind in (EffectKind.RECORD_WRITTEN, EffectKind.FIELD_EQUALS): + if effect.kind is EffectKind.RECORD_WRITTEN and effect.expected_count < 1: + continue + return effect + return None + + +def _match_dump(effect: Effect) -> dict[str, Any]: + return {k: v.model_dump(mode="json") for k, v in effect.match.items()} + + +def _prior_param(field_name: str) -> str: + safe = re.sub(r"[^A-Za-z0-9_]", "_", field_name) + return f"prior_{safe}"[:64] + + +def _inverse_effect(step: Step, effect: Effect) -> tuple[Effect, dict[str, Any]]: + """What the system of record must show after an undo of ``effect``, and + the ``locate``/``restore`` the undo needs to say it.""" + if effect.kind is EffectKind.FIELD_EQUALS: + assert effect.field is not None + param = _prior_param(effect.field) + inverse = Effect( + kind=EffectKind.FIELD_EQUALS, + match=effect.match, + field=effect.field, + value=ValueExpr(param=param), + risk="irreversible", + probe=f"the field step {step.id} changed holds its old value again", + timeout_s=effect.timeout_s, + ) + restore = UndoRestore(match=_match_dump(effect), fields={effect.field: param}) + return inverse, {"restore": restore} + id_field = effect.identity_field or "id" + inverse = Effect( + kind=EffectKind.RECORD_WRITTEN, + match={id_field: ValueExpr(param=LOCATE_PARAM)}, + expected_count=0, + risk="irreversible", + probe=f"the record step {step.id} created is gone", + timeout_s=effect.timeout_s, + ) + locate = UndoLocate(match=_match_dump(effect), field=id_field, param=LOCATE_PARAM) + return inverse, {"locate": locate} + + +def _undo_id(step: Step, source: str, n: int) -> str: + base = re.sub(r"[^A-Za-z0-9_.-]", "-", step.id)[:40] or "step" + return f"undo-{base}-{source.replace('_', '-')}-{n}" + + +# -- recordings --------------------------------------------------------------- + + +def _looks_inverse(workflow: Workflow) -> bool: + texts = [workflow.name, *(s.intent for s in workflow.steps)] + return any(INVERSE_WORDS.search(t or "") for t in texts) + + +def recording_candidates( + step: Step, effect: Effect, bundles: Sequence[Path] +) -> list[Candidate]: + """Candidates from compiled recordings of a person deleting or cancelling. + + A recording qualifies when its name or a step intent names an inverse + action. Its steps are used as recorded; when none declares an effect, the + inverse of the save's effect is added to its last step. + """ + out: list[Candidate] = [] + for path in bundles: + try: + demo = Workflow.load(path) + except Exception as exc: # noqa: BLE001 - reported, never guessed + out.append( + Candidate( + candidate_id=f"{step.id}:recording:{Path(path).name}", + undoes=step.id, + source="recording", + basis=f"recording {Path(path).name}", + problem=f"could not load it ({type(exc).__name__})", + ) + ) + continue + if not _looks_inverse(demo): + continue + undo_id = _undo_id(step, "recording", len(out) + 1) + basis = ( + f"recording {Path(path).name!r} ({demo.name}) deletes or cancels a record" + ) + try: + demonstrated = steps_from_demonstration(path, undo_id) + except ValueError as exc: + out.append( + Candidate( + candidate_id=undo_id, + undoes=step.id, + source="recording", + basis=basis, + problem=str(exc), + ) + ) + continue + steps = list(demonstrated.steps) + inverse, extra = _inverse_effect(step, effect) + if not any(s.effects for s in steps): + steps[-1] = steps[-1].model_copy( + update={"effects": [*steps[-1].effects, inverse]} + ) + out.append( + Candidate( + candidate_id=undo_id, + undoes=step.id, + source="recording", + basis=basis, + undo=UndoStep( + id=undo_id, + undoes=step.id, + intent=f"{demo.name} (undoes step {step.id}: {step.intent})"[:512], + source="demonstration", + steps=steps, + **extra, + ), + assets=demonstrated.assets, + ) + ) + return out + + +# -- network ------------------------------------------------------------------ + + +def _flat_items(node: Any, out: dict[str, set[str]]) -> dict[str, set[str]]: + """Leaf ``key -> values`` of a JSON body (nested keys by their own name).""" + if isinstance(node, dict): + for key, value in node.items(): + if isinstance(value, (dict, list)): + _flat_items(value, out) + elif value is not None: + out.setdefault(str(key), set()).add(str(value)) + elif isinstance(node, list): + for item in node: + _flat_items(item, out) + return out + + +def _selector_values(effect: Effect, params: Mapping[str, Any]) -> dict[str, str]: + out: dict[str, str] = {} + for key, expr in effect.match.items(): + value = expr.resolve(params) + if value not in (None, ""): + out[key] = value + return out + + +def _request_items(line: dict[str, Any]) -> tuple[Optional[str], dict[str, set[str]]]: + """``(kind, items)``: ``json``, ``form`` or None, and the request's + ``key -> values`` (body, form and query).""" + items: dict[str, set[str]] = {} + for key, value in up.parse_qsl(up.urlsplit(line.get("url", "")).query): + items.setdefault(key, set()).add(value) + if "form" in line: + for pair in line["form"]: + if len(pair) == 2: + items.setdefault(str(pair[0]), set()).add(str(pair[1])) + return "form", items + body = line.get("body") + if isinstance(body, str): + try: + return "json", _flat_items(json.loads(body), items) + except json.JSONDecodeError: + return None, items + return None, items + + +def _explains(items: dict[str, set[str]], wanted: dict[str, str]) -> bool: + """The request names at least one field of the save's record selector, + and every selector field it names has the selector's value.""" + named = [key for key in wanted if key in items] + return bool(named) and all(wanted[key] in items[key] for key in named) + + +def _save_request( + lines: Sequence[dict[str, Any]], wanted: dict[str, str] +) -> Optional[int]: + """Index of the last successful write whose request carries the save's + record selector values.""" + found = None + for index, line in enumerate(lines): + if str(line.get("method", "")).upper() not in _WRITE_METHODS: + continue + status = line.get("status") + if not isinstance(status, int) or not 200 <= status < 300: + continue + _kind, items = _request_items(line) + if _explains(items, wanted): + found = index + return found + + +def _response_id(line: dict[str, Any], id_field: str) -> Optional[str]: + text = line.get("response_text") + if isinstance(text, str): + try: + body = json.loads(text) + except json.JSONDecodeError: + body = None + if isinstance(body, dict): + for key in (id_field, "id"): + if body.get(key) not in (None, ""): + return str(body[key]) + data = body.get("data") + if isinstance(data, dict) and data.get(id_field) not in (None, ""): + return str(data[id_field]) + location = line.get("location") + if isinstance(location, str) and location.rstrip("/"): + return location.rstrip("/").rsplit("/", 1)[-1] + return None + + +def _path(url: str) -> str: + return up.urlsplit(url).path or "/" + + +def _api_undo_step( + undo_id: str, + step: Step, + *, + method: str, + url_template: str, + body: dict[str, Any], + inverse: Effect, +) -> Step: + return Step( + id=f"{undo_id}.api", + intent=f"{method} {url_template} (undo of step {step.id})", + action=ActionKind.WAIT, + risk="irreversible", + effects=[inverse], + api_binding=ApiBinding( + kind="rest", + method=method, + url_template=url_template, + body_template=body, + on_unavailable="halt", + effects=[inverse], + ), + ) + + +def network_candidates( + step: Step, + effect: Effect, + lines: Sequence[dict[str, Any]], + params: Mapping[str, Any], +) -> tuple[list[Candidate], str]: + """Candidates from captured traffic, and a note when there are none. + + Only JSON requests become undo steps: a browser form post needs the live + page's session token, which a plain API call does not have. + """ + if not lines: + return [], "the recording has no network capture (record --capture-network)" + wanted = _selector_values(effect, params) + index = _save_request(lines, wanted) + if index is None: + return [], ( + "no captured request carries the save's record values, so the " + "save's request could not be found" + ) + save = lines[index] + method = str(save.get("method", "")).upper() + kind, _items = _request_items(save) + path = _path(str(save.get("url", ""))) + if kind != "json": + return [], ( + f"the save is a browser form post ({method} {path}); Flow builds " + "an API undo only from a JSON request" + ) + out: list[Candidate] = [] + inverse, extra = _inverse_effect(step, effect) + if method == "POST" and effect.kind is EffectKind.RECORD_WRITTEN: + record_id = _response_id(save, effect.identity_field or "id") + if record_id is None: + return [], ( + f"the save's response ({method} {path}) carried no record id, " + "so no record URL can be derived" + ) + observed = [ + line + for line in lines[index + 1 :] + if str(line.get("method", "")).upper() == "DELETE" + and _path(str(line.get("url", ""))).rstrip("/").endswith("/" + record_id) + ] + if observed: + target = _path(str(observed[0]["url"])).rstrip("/") + template = target[: -len(record_id)] + "{" + LOCATE_PARAM + "}" + basis = ( + f"the recording sent DELETE {template} for the record the " + f"save's POST {path} created" + ) + else: + template = path.rstrip("/") + "/{" + LOCATE_PARAM + "}" + basis = ( + f"the save's JSON POST {path} returned a record id; REST " + f"convention gives DELETE {template}" + ) + undo_id = _undo_id(step, "network", 1) + out.append( + Candidate( + candidate_id=undo_id, + undoes=step.id, + source="network", + basis=basis, + undo=UndoStep( + id=undo_id, + undoes=step.id, + intent=f"delete the record step {step.id} saved ({step.intent})"[ + :512 + ], + source="synthesized", + steps=[ + _api_undo_step( + undo_id, + step, + method="DELETE", + url_template=template, + body={}, + inverse=inverse, + ) + ], + **extra, + ), + ) + ) + return out, "" + if method in {"PUT", "PATCH"} and effect.kind is EffectKind.FIELD_EQUALS: + assert effect.field is not None + param = _prior_param(effect.field) + body = {effect.field: "{" + param + "}"} + undo_id = _undo_id(step, "network", 1) + out.append( + Candidate( + candidate_id=undo_id, + undoes=step.id, + source="network", + basis=( + f"the save sent {method} {path}; the undo sends the old " + f"value of {effect.field!r} the same way" + ), + undo=UndoStep( + id=undo_id, + undoes=step.id, + intent=f"set {effect.field} back to its old value (undoes step " + f"{step.id})"[:512], + source="synthesized", + steps=[ + _api_undo_step( + undo_id, + step, + method=method, + url_template=path, + body=body, + inverse=inverse, + ) + ], + **extra, + ), + ) + ) + return out, "" + return [], ( + f"no inverse is derived for a {method} save with a {effect.kind.value} effect" + ) + + +_CONTROL = re.compile( + r"<(button|a)\b[^>]*>(.*?)|]*\bvalue\s*=\s*['\"]([^'\"]*)['\"][^>]*>", + re.I | re.S, +) +_TAGS = re.compile(r"<[^>]+>") + + +def app_ui_controls(lines: Sequence[dict[str, Any]], after: int) -> list[str]: + """Labels of delete/cancel controls on the pages loaded after the save.""" + labels: list[str] = [] + for line in lines[after + 1 :]: + if "html" not in str(line.get("response_type", "")): + continue + for match in _CONTROL.finditer(str(line.get("response_text", ""))): + text = match.group(2) if match.group(2) is not None else match.group(3) + label = html.unescape(_TAGS.sub("", text or "")).strip() + if label and len(label) <= 40 and INVERSE_WORDS.search(label): + if label not in labels: + labels.append(label) + return labels + + +# -- field restore ------------------------------------------------------------- + + +def field_restore_candidate( + workflow: Workflow, index: int, effect: Effect +) -> tuple[Optional[Candidate], str]: + """The save's own steps again, with the edited field's old value.""" + from openadapt_flow.run_gate import is_consequential + + step = workflow.steps[index] + if effect.kind is not EffectKind.FIELD_EQUALS: + return None, "the save creates a record; there is no old value to restore" + if effect.value is None or effect.value.param is None: + return None, ( + f"the field {effect.field!r} is saved as a recorded constant, not a " + "parameter, so the steps can't type its old value" + ) + assert effect.field is not None + for earlier in workflow.steps[:index]: + if is_consequential( + earlier, workflow, require_current_risk_certification=False + ): + return None, ( + f"the steps that lead to the save include another save (step " + f"{earlier.id}); repeating them would write twice" + ) + edited = effect.value.param + param = _prior_param(effect.field) + steps: list[Step] = [] + typed = False + for original in workflow.steps[: index + 1]: + copy = original.model_copy(deep=True) + copy.id = f"undo.{original.id}" + copy.api_binding = None + if copy.param == edited: + copy.param = param + typed = True + steps.append(copy) + if not typed: + return None, ( + f"no step types the parameter {edited!r}, so the old value can't be " + "typed back" + ) + inverse, extra = _inverse_effect(step, effect) + steps[-1].effects = [inverse] + undo_id = _undo_id(step, "field_restore", 1) + return ( + Candidate( + candidate_id=undo_id, + undoes=step.id, + source="field_restore", + basis=( + f"step {step.id} sets {effect.field!r}; the undo runs the same " + "steps with the value read back before the change" + ), + undo=UndoStep( + id=undo_id, + undoes=step.id, + intent=f"set {effect.field} back to the value it had before step " + f"{step.id}"[:512], + source="synthesized", + steps=steps, + **extra, + ), + ), + "", + ) + + +@dataclass +class Sources: + """Where :func:`find_candidates` looks.""" + + #: Compiled bundles of other recordings. + recordings: list[Path] = field(default_factory=list) + #: The save's recording directory (its ``network.jsonl``). + network_lines: list[dict[str, Any]] = field(default_factory=list) + + +def find_candidates( + workflow: Workflow, index: int, sources: Sources +) -> tuple[list[Candidate], list[str]]: + """Every candidate for the saving step at ``index``, best first, and the + notes on sources that gave none.""" + step = workflow.steps[index] + notes: list[str] = [] + effect = record_effect(step) + if effect is None: + return [], [ + "the save declares no record effect Flow can check (bind its " + "record_written or field_equals effect first)" + ] + params = dict(workflow.params or {}) + out = recording_candidates(step, effect, sources.recordings) + if not sources.recordings: + notes.append("no other recordings were given") + elif not out: + notes.append("no other recording deletes or cancels a record") + found, note = network_candidates(step, effect, sources.network_lines, params) + out.extend(found) + if note: + notes.append(note) + restore, note = field_restore_candidate(workflow, index, effect) + if restore is not None: + out.append(restore) + elif note: + notes.append(note) + if sources.network_lines: + wanted = _selector_values(effect, params) + at = _save_request(sources.network_lines, wanted) + for label in app_ui_controls( + sources.network_lines, at if at is not None else -1 + ): + out.append( + Candidate( + candidate_id=f"{step.id}:app_ui:{label}", + undoes=step.id, + source="app_ui", + basis=f"a {label!r} control is on a page loaded after the save", + problem=( + "Flow builds an undo from a control only from a " + "recording of a person using it; record that and pass " + "it with --from-recording" + ), + ) + ) + return out, notes + + +# --------------------------------------------------------------------------- +# Verification on a test copy +# --------------------------------------------------------------------------- + + +class UndoTestCopy(Protocol): + """A resettable test copy an undo can be tried on.""" + + name: str + + def reset(self) -> None: ... + + def snapshot(self) -> Optional[list[dict[str, Any]]]: + """Every record the read-back can see; None when unreadable.""" + ... + + def read_records(self) -> Optional[list[dict[str, Any]]]: + """Records through the run's own effect verifier (``undo run`` path).""" + ... + + def run( + self, + workflow: Workflow, + *, + bundle_dir: Path, + run_dir: Path, + params: dict[str, Any], + ) -> Optional[RunReport]: ... + + +def _canonical_records( + records: list[dict[str, Any]], ignore: frozenset[str] +) -> list[str]: + return sorted( + json.dumps( + {k: v for k, v in r.items() if k not in ignore}, sort_keys=True, default=str + ) + for r in records + ) + + +def _record_key(record: dict[str, Any]) -> str: + if "id" in record: + return f"id:{record['id']}" + return json.dumps(record, sort_keys=True, default=str) + + +def _differing_keys( + before: list[dict[str, Any]], after: list[dict[str, Any]], ignore: frozenset[str] +) -> tuple[list[str], list[str], dict[str, set[str]]]: + """``(added, removed, changed fields per key)`` between two snapshots.""" + old = {_record_key(r): r for r in before} + new = {_record_key(r): r for r in after} + added = [k for k in new if k not in old] + removed = [k for k in old if k not in new] + changed: dict[str, set[str]] = {} + for key in set(old) & set(new): + a = {k: v for k, v in old[key].items() if k not in ignore} + b = {k: v for k, v in new[key].items() if k not in ignore} + if a != b: + changed[key] = {k for k in set(a) | set(b) if a.get(k) != b.get(k)} + return added, removed, changed + + +def compare_snapshots( + before: list[dict[str, Any]], + after: list[dict[str, Any]], + *, + ignore: frozenset[str] = frozenset(), + after_save: Optional[list[dict[str, Any]]] = None, +) -> Optional[str]: + """None when ``after`` holds exactly the records of ``before``; else why + not, in counts and field names only. + + With ``after_save``, a record that differs but that the save itself did + not add or change is named as collateral: the undo changed a record it + had no business touching. + """ + if _canonical_records(before, ignore) == _canonical_records(after, ignore): + return None + added, removed, changed = _differing_keys(before, after, ignore) + touched: Optional[set[str]] = None + if after_save is not None: + s_added, s_removed, s_changed = _differing_keys(before, after_save, ignore) + touched = set(s_added) | set(s_removed) | set(s_changed) + parts = [] + if added: + parts.append(f"{len(added)} record(s) added since before the save remain") + if removed: + parts.append(f"{len(removed)} record(s) from before the save are gone") + if changed: + fields = sorted({f for names in changed.values() for f in names}) + parts.append(f"{len(changed)} record(s) changed (fields: {', '.join(fields)})") + if touched is not None: + collateral = [k for k in [*removed, *changed] if k not in touched] + if collateral: + parts.append( + f"{len(collateral)} of them the save never touched (collateral)" + ) + if not parts: + parts.append("the record sets differ") + return "after the undo the test copy differs from before the save: " + "; ".join( + parts + ) + + +def _snapshot_digest(records: Optional[list[dict[str, Any]]], ignore) -> Optional[str]: + if records is None: + return None + return hashlib.sha256(_canonical(_canonical_records(records, ignore))).hexdigest() + + +def _earlier_saves(workflow: Workflow, index: int) -> list[str]: + return [step.id for i, step in saving_steps(workflow) if i < index] + + +def _prefix_workflow(workflow: Workflow, index: int) -> Workflow: + return workflow.model_copy( + update={ + "steps": [s.model_copy(deep=True) for s in workflow.steps[:index]], + "program": None, + "subflows": {}, + "undo_steps": [], + } + ) + + +def _save_workflow(workflow: Workflow, index: int, undo: UndoStep) -> Workflow: + return workflow.model_copy( + update={ + "steps": [s.model_copy(deep=True) for s in workflow.steps[: index + 1]], + "program": None, + "subflows": {}, + "undo_steps": [undo], + } + ) + + +def _report_line(report: Optional[RunReport]) -> str: + if report is None: + return "left no report" + failed = next((r for r in report.results if not r.ok and not r.skipped), None) + if report.success: + return "completed" + if failed is not None: + return f"stopped at step {failed.step_id}" + return "did not complete" + + +def verify_candidate( + candidate: Candidate, + workflow: Workflow, + index: int, + *, + target: UndoTestCopy, + bundle_dir: Path, + work_dir: Path, + params: Optional[dict[str, Any]] = None, + ignore_fields: Sequence[str] = (), +) -> CandidateResult: + """Run the save, then ``candidate``'s undo, on ``target``; judge both. + + Passes only when the save changed the test copy, the undo ran and its + declared effect was confirmed by an independent read-back, and every + record afterwards equals the record before the save (the save's own record + and every other one). The copy is reset before and after. + """ + assert candidate.undo is not None + undo = candidate.undo + ignore = frozenset(ignore_fields) + params = dict(params if params is not None else (workflow.params or {})) + stage = work_dir / "bundles" / candidate.candidate_id + runs = work_dir / "runs" / candidate.candidate_id + evidence: dict[str, Any] = { + "schema": EVIDENCE_SCHEMA, + "candidate_id": candidate.candidate_id, + "undoes": candidate.undoes, + "source": candidate.source, + "basis": candidate.basis, + "test_copy": target.name, + "undo_digest": _sha256(undo.model_dump(mode="json")), + "ignored_fields": sorted(ignore), + "started_at": _now(), + } + + def done(status: str, reason: str) -> CandidateResult: + evidence["finished_at"] = _now() + evidence["verdict"] = status + evidence["reason"] = reason + try: + target.reset() + except Exception: # noqa: BLE001 - every attempt resets first + pass + return CandidateResult( + candidate_id=candidate.candidate_id, + source=candidate.source, + basis=candidate.basis, + status="verified" if status == "verified" else "failed", + reason=reason, + evidence=evidence, + ) + + try: + if stage.exists(): + shutil.rmtree(stage) + shutil.copytree(bundle_dir, stage) + for rel, source in candidate.assets.items(): + (stage / rel).parent.mkdir(parents=True, exist_ok=True) + shutil.copyfile(source, stage / rel) + if runs.exists(): + shutil.rmtree(runs) + target.reset() + except Exception as exc: # noqa: BLE001 - nothing ran yet + return done("failed", f"could not prepare the test copy ({exc})") + earlier = _earlier_saves(workflow, index) + evidence["earlier_saves"] = len(earlier) + if earlier: + # "Before the save" is the state the steps ahead of it leave: run + # them once, read every record, and reset for the real attempt. + try: + prefix_report = target.run( + _prefix_workflow(workflow, index), + bundle_dir=stage, + run_dir=runs / "before-save", + params=params, + ) + except Exception as exc: # noqa: BLE001 - recorded as a failure + prefix_report = None + evidence["prefix_error"] = type(exc).__name__ + evidence["before_save_steps"] = _report_line(prefix_report) + if prefix_report is None or not prefix_report.success: + return done( + "failed", + "the steps before the save did not complete on the test copy " + f"({evidence['before_save_steps']})", + ) + try: + before = target.snapshot() + if earlier: + target.reset() + except Exception as exc: # noqa: BLE001 - nothing more ran + return done("failed", f"could not read or reset the test copy ({exc})") + evidence["before"] = { + "records": None if before is None else len(before), + "sha256": _snapshot_digest(before, ignore), + } + if before is None: + return done( + "failed", "the test copy's records could not be read before the save" + ) + + save_wf = _save_workflow(workflow, index, undo) + try: + save_report = target.run( + save_wf, bundle_dir=stage, run_dir=runs / "save", params=params + ) + except Exception as exc: # noqa: BLE001 - a crashed replay decides nothing + save_report = None + evidence["save_error"] = type(exc).__name__ + evidence["save"] = _report_line(save_report) + evidence["save_model_calls"] = int(getattr(save_report, "model_calls", 0) or 0) + if save_report is None or not save_report.success: + return done( + "failed", + f"the save did not complete on the test copy ({evidence['save']}), " + "so there is nothing to undo", + ) + mid = target.snapshot() + evidence["after_save"] = { + "records": None if mid is None else len(mid), + "sha256": _snapshot_digest(mid, ignore), + } + if mid is None: + return done( + "failed", "the test copy's records could not be read after the save" + ) + if compare_snapshots(before, mid, ignore=ignore) is None: + return done( + "failed", + "the save changed no record on the test copy, so the undo can't be " + "shown to reverse it", + ) + try: + undo_params = resolve_undo_params( + undo, runs / "save", params, target.read_records + ) + except UndoRefused as exc: + return done("failed", f"the undo could not be prepared: {exc}") + try: + undo_report = target.run( + undo_workflow(save_wf, undo), + bundle_dir=stage, + run_dir=runs / "undo", + params=undo_params, + ) + except Exception as exc: # noqa: BLE001 - recorded as a failed undo + undo_report = None + evidence["undo_error"] = type(exc).__name__ + evidence["undo"] = _report_line(undo_report) + evidence["undo_model_calls"] = int(getattr(undo_report, "model_calls", 0) or 0) + confirmed, judged = judge_undo_report(undo, undo_report) + evidence["undo_judgement"] = judged + after = target.snapshot() + evidence["after_undo"] = { + "records": None if after is None else len(after), + "sha256": _snapshot_digest(after, ignore), + } + if after is None: + return done( + "failed", "the test copy's records could not be read after the undo" + ) + difference = compare_snapshots(before, after, ignore=ignore, after_save=mid) + if difference is not None: + return done("failed", difference) + if not confirmed: + return done("failed", f"the undo was not confirmed: {judged}") + return done( + "verified", + "the undo ran, its record read-back confirmed it, and every record " + "matches the read-back from before the save", + ) + + +# --------------------------------------------------------------------------- +# The whole pass +# --------------------------------------------------------------------------- + +_ORDER = {"recording": 0, "network": 1, "field_restore": 2, "app_ui": 3} + + +def propose_undos( + workflow: Workflow, + *, + bundle_dir: Path, + sources: Sources, + target: Optional[UndoTestCopy], + work_dir: Optional[Path] = None, + ignore_fields: Sequence[str] = (), +) -> tuple[ProposeReport, dict[str, tuple[Candidate, CandidateResult]]]: + """Find and verify candidates for every saving step. + + Returns the report and, per saving step, the first verified candidate. + Without a ``target`` nothing runs and nothing can verify: every built + candidate is ``not_run``. + """ + report = ProposeReport( + workflow_name=workflow.name, test_copy=target.name if target else None + ) + verified: dict[str, tuple[Candidate, CandidateResult]] = {} + if workflow.program is not None: + raise ValueError( + "undo propose works on straight-line bundles; this one is a program graph" + ) + from openadapt_flow.after_stop import _has_loop + + if _has_loop(workflow): + raise ValueError( + "undo propose does not run on a workflow with a loop: an undo runs " + "with the run's base parameters" + ) + declared = {u.undoes: u.id for u in workflow.undo_steps} + own_tmp = work_dir is None + work = Path(tempfile.mkdtemp(prefix="undo-propose-")) if own_tmp else work_dir + assert work is not None + try: + for index, step in saving_steps(workflow): + row = SavingStepResult(step_id=step.id, intent=step.intent) + report.saving_steps.append(row) + if step.id in declared: + row.verified_undo = None + row.reason = ( + f"the bundle already has undo {declared[step.id]!r} for this step" + ) + continue + candidates, notes = find_candidates(workflow, index, sources) + candidates.sort(key=lambda c: _ORDER[c.source]) + for candidate in candidates: + if candidate.undo is None: + row.candidates.append( + CandidateResult( + candidate_id=candidate.candidate_id, + source=candidate.source, + basis=candidate.basis, + status="not_run", + reason=candidate.problem or "could not be built", + ) + ) + continue + if step.id in verified: + reason = "an earlier candidate for this step verified" + elif target is None: + reason = "no test copy was given (--config), so nothing ran" + else: + reason = "" + if reason: + row.candidates.append( + CandidateResult( + candidate_id=candidate.candidate_id, + source=candidate.source, + basis=candidate.basis, + status="not_run", + reason=reason, + ) + ) + continue + assert target is not None + result = verify_candidate( + candidate, + workflow, + index, + target=target, + bundle_dir=bundle_dir, + work_dir=work, + ignore_fields=ignore_fields, + ) + row.candidates.append(result) + if result.status == "verified": + verified[step.id] = (candidate, result) + row.verified_undo = candidate.candidate_id + if row.verified_undo is None: + failed = [c for c in row.candidates if c.status == "failed"] + if failed: + row.reason = "; ".join(f"{c.source}: {c.reason}" for c in failed) + elif row.candidates: + row.reason = "; ".join( + f"{c.source}: {c.reason}" for c in row.candidates + ) + else: + row.reason = "no candidate: " + "; ".join(notes) + if notes and row.candidates: + row.reason += " (also: " + "; ".join(notes) + ")" + finally: + if own_tmp: + shutil.rmtree(work, ignore_errors=True) + return report, verified + + +def write_proposals( + bundle_dir: Path | str, + verified: Mapping[str, tuple[Candidate, CandidateResult]], +) -> Workflow: + """Write each verified undo into the bundle as a proposal awaiting a + person's approval, with its evidence under ``undo-evidence/``.""" + bundle_dir = Path(bundle_dir) + workflow = Workflow.load(bundle_dir) + if not verified: + return workflow + taken = {u.id for u in workflow.undo_steps} + targets = {u.undoes for u in workflow.undo_steps} + new: list[UndoStep] = [] + copies: dict[str, Path] = {} + payloads: dict[str, bytes] = {} + for step_id, (candidate, result) in verified.items(): + assert candidate.undo is not None and result.evidence is not None + if candidate.undo.id in taken or step_id in targets: + raise ValueError( + f"the bundle already has an undo for step {step_id!r} or the id " + f"{candidate.undo.id!r}" + ) + rel = f"{EVIDENCE_DIRNAME}/{candidate.undo.id}.json" + payload = json.dumps(result.evidence, indent=2, sort_keys=True).encode("utf-8") + payloads[rel] = payload + for asset_rel, source in candidate.assets.items(): + copies[asset_rel] = source + new.append( + candidate.undo.model_copy( + update={ + "proposal": UndoProposal( + candidate_source=candidate.source, # type: ignore[arg-type] + basis=candidate.basis[:1024], + test_copy=str(result.evidence.get("test_copy", "test copy")), + verified_at=str(result.evidence.get("finished_at", _now())), + evidence_path=rel, + evidence_sha256=hashlib.sha256(payload).hexdigest(), + ) + } + ) + ) + for rel in [*copies, *payloads]: + if (bundle_dir / rel).exists(): + raise ValueError(f"the bundle already has {rel}") + for rel, source in copies.items(): + (bundle_dir / rel).parent.mkdir(parents=True, exist_ok=True) + shutil.copyfile(source, bundle_dir / rel) + for rel, payload in payloads.items(): + (bundle_dir / rel).parent.mkdir(parents=True, exist_ok=True) + (bundle_dir / rel).write_bytes(payload) + return save_undo_steps( + bundle_dir, + workflow, + [*workflow.undo_steps, *new], + reason="a verified undo was proposed after certification", + ) + + +def proposal_problem(bundle_dir: Path | str, undo: UndoStep) -> Optional[str]: + """Why ``undo``'s proposal can't be approved, or None.""" + if undo.proposal is None: + return f"undo {undo.id!r} is not a proposal" + if undo.proposal.approved_by is not None: + return f"undo {undo.id!r} was already approved by {undo.proposal.approved_by}" + path = Path(bundle_dir) / undo.proposal.evidence_path + try: + data = path.read_bytes() + except OSError: + return f"the verification evidence {undo.proposal.evidence_path} is missing" + if hashlib.sha256(data).hexdigest() != undo.proposal.evidence_sha256: + return ( + f"the verification evidence {undo.proposal.evidence_path} changed " + "since the undo was verified" + ) + try: + evidence = json.loads(data) + except json.JSONDecodeError: + return "the verification evidence is not JSON" + if evidence.get("verdict") != "verified": + return "the verification evidence does not record a verified undo" + proposed = undo.model_copy(update={"proposal": None}) + if evidence.get("undo_digest") != _sha256(proposed.model_dump(mode="json")): + return "the undo changed since it was verified on the test copy" + return None + + +def accept_proposal(bundle_dir: Path | str, undo_id: str, *, approver: str) -> Workflow: + """Record a person's one-time approval of a proposed undo; re-seal. + + Refuses when the evidence file is missing or changed, or the undo + changed since it was verified. Each run of the undo still needs its own + approval after a stop (``undo approve``). + """ + bundle_dir = Path(bundle_dir) + workflow = Workflow.load(bundle_dir) + undo = next((u for u in workflow.undo_steps if u.id == undo_id), None) + if undo is None: + raise ValueError(f"the bundle has no undo {undo_id!r}") + problem = proposal_problem(bundle_dir, undo) + if problem: + raise ValueError(problem) + assert undo.proposal is not None + approved = undo.model_copy( + update={ + "proposal": undo.proposal.model_copy( + update={"approved_by": approver, "approved_at": _now()} + ) + } + ) + steps = [approved if u.id == undo_id else u for u in workflow.undo_steps] + return save_undo_steps( + bundle_dir, + workflow, + steps, + reason="a proposed undo was approved after certification", + ) + + +# --------------------------------------------------------------------------- +# The deployment-file test copy +# --------------------------------------------------------------------------- + + +class DeploymentUndoTestCopy: + """The test copy a deployment file declares (``test_copy``). + + Reset: the probe path's reset (``reset_url`` / ``reset_command``). + Snapshot: the reward contract's oracle, which reads every record in the + collection. Runs: a fresh session each time, wired with the deployment's + own effect verifier and API actuator, so the undo is checked the way + ``undo run`` checks it. Model grounding and egress stay off. + """ + + def __init__(self, cfg: Any, *, deployment_path: Path) -> None: + from openadapt_flow.compiler.probing import DeploymentProbeTarget + from openadapt_flow.reward.models import RewardBundle + from openadapt_flow.reward.oracles import build_oracle + + self._probe = DeploymentProbeTarget(cfg) # refuses a non-test target + self.cfg = cfg + self.name = str(cfg.name) + contract = Path(str(cfg.test_copy.reward_contract)).expanduser() + if not contract.is_absolute(): + contract = Path(deployment_path).parent / contract + bundle = RewardBundle.load(contract) + self._oracle = build_oracle(bundle.oracle, bundle.directory) + self._params: dict[str, Any] = {} + + def reset(self) -> None: + self._probe.reset() + + def snapshot(self) -> Optional[list[dict[str, Any]]]: + from openadapt_flow.reward.oracles import records_of + + return records_of(self._oracle.read({})) + + def read_records(self) -> Optional[list[dict[str, Any]]]: + from openadapt_flow.after_stop import records_reader + from openadapt_flow.deployment import build_effect_verifier + + reader = records_reader(build_effect_verifier(self.cfg.effects, self._params)) + return reader() if reader is not None else None + + def _replay(self, backend, workflow, bundle_dir, run_dir, params) -> RunReport: + from openadapt_flow.deployment import ( + build_api_actuator, + build_effect_verifier, + build_replayer, + ) + + replayer = build_replayer( + backend, + allow_egress=False, + effect_verifier=build_effect_verifier(self.cfg.effects, params), + api_actuator=build_api_actuator(self.cfg.actuation), + durable=False, + use_structural=True, + ) + return replayer.run( + workflow, bundle_dir=bundle_dir, run_dir=run_dir, params=params + ) + + def run( + self, + workflow: Workflow, + *, + bundle_dir: Path, + run_dir: Path, + params: dict[str, Any], + ) -> Optional[RunReport]: + from openadapt_flow.backends.factory import build_backend + + self._params = dict(params) + backend_cfg = self.cfg.backend + if backend_cfg.kind != "web": + backend = build_backend(backend_cfg) + try: + return self._replay(backend, workflow, bundle_dir, run_dir, params) + finally: + close = getattr(backend, "close", None) + if callable(close): + close() + if not backend_cfg.url: + raise ValueError("a web test copy needs backend.url") + from playwright.sync_api import sync_playwright + + from openadapt_flow._browser_setup import launch_chromium + + with sync_playwright() as p: + browser = launch_chromium(p.chromium, headless=not backend_cfg.headed) + try: + context = browser.new_context(viewport={"width": 1280, "height": 800}) + page = context.new_page() + page.goto(backend_cfg.url) + backend = build_backend(backend_cfg, page=page) + return self._replay(backend, workflow, bundle_dir, run_dir, params) + finally: + browser.close() + + +def load_network_lines(recording: Optional[Path | str]) -> list[dict[str, Any]]: + if recording is None: + return [] + from openadapt_flow.network_capture import NETWORK_FILE, load + + if not (Path(recording) / NETWORK_FILE).is_file(): + return [] + return list(load(recording)) + + +__all__ = [ + "Candidate", + "CandidateResult", + "DeploymentUndoTestCopy", + "ProposeReport", + "SavingStepResult", + "Sources", + "UndoTestCopy", + "accept_proposal", + "app_ui_controls", + "compare_snapshots", + "field_restore_candidate", + "find_candidates", + "load_network_lines", + "network_candidates", + "proposal_problem", + "propose_undos", + "record_effect", + "recording_candidates", + "saving_steps", + "verify_candidate", + "write_proposals", +] diff --git a/packages/flow/tests/test_undo_proposal.py b/packages/flow/tests/test_undo_proposal.py new file mode 100644 index 0000000000..b22973b6e4 --- /dev/null +++ b/packages/flow/tests/test_undo_proposal.py @@ -0,0 +1,840 @@ +"""Proposed undo steps, verified on a test copy (docs/AFTER_A_STOP.md). + +End to end against the REAL MockMed record store (the in-process +``fault_server``) and a small field-edit store, with the REAL Replayer, the +REST record read-back and the REST API actuator. The screen and keyboard are +scripted, as in ``test_after_stop_mockmed``: each Enter saves, each Delete +removes, so every verdict reads the actual records. + +The story: ``undo propose`` finds an undo for each saving step, tries it on +the test copy, and writes only the ones that pass, as proposals. A proposal +is offered after a stop only once a person has approved it, and each run of +it still needs that person's approval then. +""" + +from __future__ import annotations + +import json +import threading +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from pathlib import Path + +import pytest +import requests + +from openadapt_flow.after_stop import ( + UndoNotOffered, + UndoRefused, + approve_undo, + build_change_ledger, + keep_prior_state, + offer_undos, + prior_state_path, + resolve_undo_params, + run_undo, +) +from openadapt_flow.ir import ( + ActionKind, + Postcondition, + PostconditionKind, + Step, + UndoStep, + Workflow, +) +from openadapt_flow.mockmed.fault_server import serve as fault_serve +from openadapt_flow.runtime.actuators import ApiActuator +from openadapt_flow.runtime.effects import Effect, EffectKind, RestRecordVerifier +from openadapt_flow.runtime.effects.effect import ValueExpr +from openadapt_flow.runtime.replayer import Replayer +from openadapt_flow.undo_proposal import ( + Sources, + accept_proposal, + app_ui_controls, + compare_snapshots, + find_candidates, + network_candidates, + proposal_problem, + propose_undos, + write_proposals, +) +from tests.test_replayer import FakeBackend, FakeVision, Match + +# --------------------------------------------------------------------------- +# MockMed: a create (save encounter) and its inverse (delete it) +# --------------------------------------------------------------------------- + + +def _records(url: str) -> list[dict]: + return requests.get(f"{url}/api/db", timeout=5).json()["records"] + + +class SavingBackend(FakeBackend): + """Each Enter saves the next encounter kind; Delete removes p1's Triage + rows (all of them, which is what makes it a bad undo).""" + + kinds = ("Triage", "Vitals") + + def __init__(self, url: str): + super().__init__(viewport=(300, 200)) + self.url = url + self.saves = 0 + + def press(self, key): + super().press(key) + if key == "Enter": + kind = self.kinds[self.saves % len(self.kinds)] + self.saves += 1 + requests.post( + f"{self.url}/api/encounter", + json={"patient_id": "p1", "type": kind, "note": "n"}, + timeout=5, + ) + elif key == "Delete": + for row in _records(self.url): + if row["patient_id"] == "p1" and row["type"] == "Triage": + requests.delete(f"{self.url}/api/encounter/{row['id']}", timeout=5) + + +def _vision() -> FakeVision: + vision = FakeVision() + hit = Match(point=(50, 10), region=(30, 5, 40, 10), confidence=0.9) + vision.text_results = {"Saved": hit, "Deleted": hit} + return vision + + +def _save(step_id: str, kind: str) -> Step: + return Step( + id=step_id, + intent=f"save {kind} encounter", + action=ActionKind.KEY, + key="Enter", + expect=[ + Postcondition( + kind=PostconditionKind.TEXT_PRESENT, text="Saved", timeout_s=0.2 + ) + ], + risk="irreversible", + effects=[ + Effect( + kind=EffectKind.RECORD_WRITTEN, + match={"patient_id": "p1", "type": kind}, + expected_count=1, + count_new_only=True, + risk="irreversible", + timeout_s=1.0, + ) + ], + ) + + +def _workflow() -> Workflow: + return Workflow( + name="mockmed-two-saves", + steps=[_save("save_triage", "Triage"), _save("save_vitals", "Vitals")], + ) + + +def _network(path: str = "/api/encounter") -> list[dict]: + """What ``record --capture-network`` keeps for the two saves.""" + lines = [] + for n, kind in enumerate(("Triage", "Vitals"), start=1): + lines.append( + { + "method": "POST", + "url": f"http://127.0.0.1:1{path}", + "request_type": "application/json", + "body": json.dumps({"patient_id": "p1", "type": kind, "note": "n"}), + "status": 200, + "response_type": "application/json", + "response_text": json.dumps({"ok": True, "id": n}), + } + ) + return lines + + +class MockMedCopy: + """MockMed as a resettable test copy, driven like ``undo run`` drives it.""" + + name = "mockmed-test-copy" + + def __init__(self, url: str, *, seed: list[dict] | None = None): + self.url = url + self.seed = seed or [] + self.resets = 0 + + def reset(self) -> None: + self.resets += 1 + requests.post(f"{self.url}/api/reset", json={}, timeout=5) + for row in self.seed: + requests.post(f"{self.url}/api/encounter", json=row, timeout=5) + + def snapshot(self): + return _records(self.url) + + def read_records(self): + return _records(self.url) + + def run(self, workflow, *, bundle_dir, run_dir, params): + replayer = Replayer( + SavingBackend(self.url), + vision=_vision(), + effect_verifier=RestRecordVerifier(self.url), + api_actuator=ApiActuator(self.url), + poll_interval_s=0.01, + ) + return replayer.run( + workflow, bundle_dir=bundle_dir, run_dir=run_dir, params=params + ) + + +@pytest.fixture +def mockmed(): + url, _db, stop = fault_serve() + try: + yield url.rstrip("/") + finally: + stop() + + +def _bundle(tmp_path: Path, workflow: Workflow) -> Path: + bundle = tmp_path / "bundle" + workflow.save(bundle) + return bundle + + +def test_network_undo_is_verified_written_as_a_proposal_and_waits_for_a_person( + tmp_path, mockmed +): + workflow = _workflow() + bundle = _bundle(tmp_path, workflow) + target = MockMedCopy(mockmed) + + report, verified = propose_undos( + Workflow.load(bundle), + bundle_dir=bundle, + sources=Sources(network_lines=_network()), + target=target, + work_dir=tmp_path / "work", + ) + + assert report.summary() == { + "saving_steps": 2, + "verified_undo": 2, + "no_verified_undo": 0, + } + triage = report.saving_steps[0] + assert triage.verified_undo == "undo-save_triage-network-1" + [result] = triage.candidates + assert result.status == "verified", result.reason + assert "REST convention gives DELETE /api/encounter/{undo_record_id}" in ( + result.basis + ) + evidence = result.evidence + assert evidence["before"]["sha256"] == evidence["after_undo"]["sha256"] + assert evidence["after_save"]["sha256"] != evidence["before"]["sha256"] + # The second save's "before" is the state the first save leaves. + assert report.saving_steps[1].candidates[0].evidence["earlier_saves"] == 1 + # PHI-free: no record value in the evidence or the report. + dumped = json.dumps(report.model_dump(mode="json")) + assert "p1" not in dumped and "Triage" not in json.dumps(evidence["before"]) + # The copy is left at its baseline. + assert _records(mockmed) == [] + + written = write_proposals(bundle, verified) + proposals = {u.undoes: u for u in written.undo_steps} + undo = proposals["save_triage"] + assert undo.source == "synthesized" + assert undo.awaiting_approval + assert undo.proposal.candidate_source == "network" + assert (bundle / undo.proposal.evidence_path).is_file() + assert proposal_problem(bundle, undo) is None + + # A stop now: the Triage row is saved, the run halts at step 2. + run_wf = Workflow.load(bundle) + run_dir = tmp_path / "run" + stopping = Replayer( + _RejectSecond(mockmed), + vision=_vision(), + effect_verifier=RestRecordVerifier(mockmed), + poll_interval_s=0.01, + ) + stopped = stopping.run(run_wf, bundle_dir=bundle, run_dir=run_dir) + assert not stopped.success + assert [r["type"] for r in _records(mockmed)] == ["Triage"] + # The replayer kept the ids that matched before the save (none). + assert prior_state_path(run_dir, "save_triage").is_file() + + # A proposal nobody approved is not offered, and the ledger names none. + assert offer_undos(run_dir, run_wf, bundle_dir=bundle) == [] + assert build_change_ledger(stopped, run_wf)[0].undo_id is None + with pytest.raises(UndoNotOffered): + approve_undo(run_dir, run_wf, undo.id, approver="nurse-a", bundle_dir=bundle) + + # A person approves the proposal once; the bundle is re-sealed. + accepted = accept_proposal(bundle, undo.id, approver="nurse-a") + assert not next(u for u in accepted.undo_steps if u.id == undo.id).awaiting_approval + offers = offer_undos(run_dir, accepted, bundle_dir=bundle) + assert [o.undo_id for o in offers] == [undo.id] + + # Still no undo without the at-the-time approval. + with pytest.raises(UndoRefused): + run_undo( + run_dir, + accepted, + undo.id, + replayer=_undo_replayer(mockmed), + bundle_dir=bundle, + ) + approve_undo(run_dir, accepted, undo.id, approver="nurse-a", bundle_dir=bundle) + result = run_undo( + run_dir, + accepted, + undo.id, + replayer=_undo_replayer(mockmed), + bundle_dir=bundle, + ) + assert result.status == "verified", result.reason + assert _records(mockmed) == [] + + +class _RejectSecond(SavingBackend): + def press(self, key): + if key == "Enter" and self.saves == 1: + FakeBackend.press(self, key) + self.saves += 1 + requests.post( + f"{self.url}/api/encounter?fault=optimistic", + json={"patient_id": "p1", "type": "Vitals", "note": "n"}, + timeout=5, + ) + return + super().press(key) + + +def _undo_replayer(url: str) -> Replayer: + return Replayer( + SavingBackend(url), + vision=_vision(), + effect_verifier=RestRecordVerifier(url), + api_actuator=ApiActuator(url), + poll_interval_s=0.01, + ) + + +def test_a_wrong_endpoint_fails_with_its_reason_and_is_never_written( + tmp_path, mockmed +): + workflow = Workflow(name="one-save", steps=[_save("save_triage", "Triage")]) + bundle = _bundle(tmp_path, workflow) + report, verified = propose_undos( + workflow, + bundle_dir=bundle, + sources=Sources(network_lines=_network("/api/encounter/new")), + target=MockMedCopy(mockmed), + work_dir=tmp_path / "work", + ) + [row] = report.saving_steps + assert row.verified_undo is None + [candidate] = row.candidates + assert candidate.status == "failed" + assert "1 record(s) added since before the save remain" in candidate.reason + assert verified == {} + assert Workflow.load(bundle).undo_steps == [] + assert _records(mockmed) == [] + + +def _delete_demo(tmp_path: Path) -> Path: + demo = Workflow( + name="delete triage encounter", + steps=[ + Step( + id="press_delete", + intent="delete the Triage encounter", + action=ActionKind.KEY, + key="Delete", + expect=[ + Postcondition( + kind=PostconditionKind.TEXT_PRESENT, + text="Deleted", + timeout_s=0.2, + ) + ], + risk="irreversible", + ) + ], + ) + path = tmp_path / "delete-demo" + demo.save(path) + return path + + +def test_recording_undo_that_also_deletes_another_record_fails_the_collateral_guard( + tmp_path, mockmed +): + workflow = Workflow(name="one-save", steps=[_save("save_triage", "Triage")]) + bundle = _bundle(tmp_path, workflow) + # Another Triage row for the patient was already there. The recorded + # delete removes every Triage row, so it takes that one too. + seed = [{"patient_id": "p1", "type": "Triage", "note": "earlier"}] + report, verified = propose_undos( + workflow, + bundle_dir=bundle, + sources=Sources(recordings=[_delete_demo(tmp_path)]), + target=MockMedCopy(mockmed, seed=seed), + work_dir=tmp_path / "work", + ) + [row] = report.saving_steps + [candidate] = row.candidates + assert candidate.source == "recording" + assert candidate.status == "failed" + assert "1 record(s) from before the save are gone" in candidate.reason + assert "the save never touched (collateral)" in candidate.reason + assert verified == {} + + +def test_recording_undo_passes_when_nothing_else_is_touched(tmp_path, mockmed): + workflow = Workflow(name="one-save", steps=[_save("save_triage", "Triage")]) + bundle = _bundle(tmp_path, workflow) + report, verified = propose_undos( + workflow, + bundle_dir=bundle, + sources=Sources(recordings=[_delete_demo(tmp_path)]), + target=MockMedCopy(mockmed), + work_dir=tmp_path / "work", + ) + [row] = report.saving_steps + assert row.candidates[0].status == "verified", row.candidates[0].reason + undo = verified["save_triage"][0].undo + assert undo.source == "demonstration" + # The recorded steps declared no effect, so the save's inverse was added. + assert undo.steps[-1].effects[0].expected_count == 0 + assert undo.locate is not None + + +def test_without_a_test_copy_nothing_runs_and_nothing_verifies(tmp_path): + workflow = _workflow() + bundle = _bundle(tmp_path, workflow) + report, verified = propose_undos( + workflow, + bundle_dir=bundle, + sources=Sources(network_lines=_network()), + target=None, + ) + assert verified == {} + assert all( + c.status == "not_run" and "no test copy" in c.reason + for row in report.saving_steps + for c in row.candidates + ) + + +# --------------------------------------------------------------------------- +# A field edit and its inverse (set the old value back) +# --------------------------------------------------------------------------- + + +class _FieldStore: + """A tiny record store with one editable field per patient.""" + + def __init__(self): + self.lock = threading.Lock() + self.reset() + + def reset(self): + with self.lock: + self.records = [ + {"id": 1, "patient_id": "p1", "allergy": "none recorded"}, + {"id": 2, "patient_id": "p2", "allergy": "latex"}, + ] + + def patch(self, record_id: int, body: dict): + with self.lock: + for row in self.records: + if row["id"] == record_id: + row.update(body) + return True + return False + + +@pytest.fixture +def field_store(): + store = _FieldStore() + + class Handler(BaseHTTPRequestHandler): + def log_message(self, *args): + pass + + def _json(self, status, payload): + body = json.dumps(payload).encode() + self.send_response(status) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def do_GET(self): # noqa: N802 + with store.lock: + self._json(200, {"records": [dict(r) for r in store.records]}) + + def do_POST(self): # noqa: N802 + store.reset() + self._json(200, {"ok": True}) + + def do_PATCH(self): # noqa: N802 + length = int(self.headers.get("Content-Length", 0) or 0) + body = json.loads(self.rfile.read(length) or b"{}") + ok = store.patch(int(self.path.rstrip("/").rsplit("/", 1)[-1]), body) + self._json(200 if ok else 404, {"ok": ok}) + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield f"http://127.0.0.1:{server.server_address[1]}", store + finally: + server.shutdown() + + +class EditingBackend(FakeBackend): + """Typing fills the allergy field; Enter saves it for patient p1.""" + + def __init__(self, url: str, *, also_touch_p2: bool = False): + super().__init__(viewport=(300, 200)) + self.url = url + self.typed = "" + self.also_touch_p2 = also_touch_p2 + + def type_text(self, text): + self.typed = text + return super().type_text(text) + + def press(self, key): + super().press(key) + if key == "Enter": + requests.patch( + f"{self.url}/api/patient/1", json={"allergy": self.typed}, timeout=5 + ) + if self.also_touch_p2: + requests.patch( + f"{self.url}/api/patient/2", json={"note": "seen"}, timeout=5 + ) + + +def _edit_workflow() -> Workflow: + return Workflow( + name="record-allergy", + params={"patient_id": "p1", "allergy": "penicillin"}, + steps=[ + Step( + id="type_allergy", + intent="type the allergy", + action=ActionKind.TYPE, + param="allergy", + ), + Step( + id="save_allergy", + intent="save the allergy", + action=ActionKind.KEY, + key="Enter", + expect=[ + Postcondition( + kind=PostconditionKind.TEXT_PRESENT, text="Saved", timeout_s=0.2 + ) + ], + risk="irreversible", + effects=[ + Effect( + kind=EffectKind.FIELD_EQUALS, + match={"patient_id": ValueExpr(param="patient_id")}, + field="allergy", + value=ValueExpr(param="allergy"), + risk="irreversible", + timeout_s=1.0, + ) + ], + ), + ], + ) + + +class FieldCopy: + name = "field-test-copy" + + def __init__(self, url, store, *, also_touch_p2=False): + self.url, self.store, self.also_touch_p2 = url, store, also_touch_p2 + + def reset(self): + self.store.reset() + + def snapshot(self): + return requests.get(f"{self.url}/api/db", timeout=5).json()["records"] + + read_records = snapshot + + def run(self, workflow, *, bundle_dir, run_dir, params): + replayer = Replayer( + EditingBackend(self.url, also_touch_p2=self.also_touch_p2), + vision=_vision(), + effect_verifier=RestRecordVerifier(self.url), + poll_interval_s=0.01, + ) + return replayer.run( + workflow, bundle_dir=bundle_dir, run_dir=run_dir, params=params + ) + + +def test_field_edit_gets_a_verified_restore_of_the_old_value(tmp_path, field_store): + url, store = field_store + workflow = _edit_workflow() + bundle = _bundle(tmp_path, workflow) + report, verified = propose_undos( + workflow, + bundle_dir=bundle, + sources=Sources(), + target=FieldCopy(url, store), + work_dir=tmp_path / "work", + ) + [row] = report.saving_steps + [candidate] = row.candidates + assert candidate.source == "field_restore" + assert candidate.status == "verified", candidate.reason + undo = verified["save_allergy"][0].undo + assert undo.restore is not None + assert undo.restore.fields == {"allergy": "prior_allergy"} + assert [s.param for s in undo.steps if s.param] == ["prior_allergy"] + # The copy is back at its baseline. + assert store.records[0]["allergy"] == "none recorded" + + +def test_field_restore_that_touches_another_record_fails(tmp_path, field_store): + url, store = field_store + workflow = _edit_workflow() + bundle = _bundle(tmp_path, workflow) + report, verified = propose_undos( + workflow, + bundle_dir=bundle, + sources=Sources(), + target=FieldCopy(url, store, also_touch_p2=True), + work_dir=tmp_path / "work", + ) + [candidate] = report.saving_steps[0].candidates + assert candidate.status == "failed" + assert "record(s) changed (fields: note)" in candidate.reason + assert verified == {} + + +# --------------------------------------------------------------------------- +# Units: candidates, refusals, the kept values, the approval +# --------------------------------------------------------------------------- + + +def test_network_candidates_explain_why_there_is_none(): + step = _save("save_triage", "Triage") + effect = step.effects[0] + assert "no network capture" in network_candidates(step, effect, [], {})[1] + form = [ + { + "method": "POST", + "url": "http://x/save.php", + "form": [["patient_id", "p1"], ["type", "Triage"]], + "status": 302, + } + ] + assert "could not be found" in network_candidates(step, effect, form, {})[1] + form[0]["status"] = 200 + assert "browser form post" in network_candidates(step, effect, form, {})[1] + no_id = _network()[:1] + no_id[0]["response_text"] = "{}" + assert "no record id" in network_candidates(step, effect, no_id, {})[1] + + +def test_an_observed_delete_is_preferred_over_the_convention(): + step = _save("save_triage", "Triage") + lines = _network()[:1] + [ + {"method": "DELETE", "url": "http://x/api/v2/encounters/1", "status": 204} + ] + [candidate], _ = network_candidates(step, step.effects[0], lines, {}) + assert "the recording sent DELETE /api/v2/encounters/{undo_record_id}" in ( + candidate.basis + ) + assert candidate.undo.steps[0].api_binding.url_template == ( + "/api/v2/encounters/{undo_record_id}" + ) + + +def test_app_ui_control_is_reported_and_never_built(): + lines = _network()[:1] + [ + { + "method": "GET", + "url": "http://x/encounter/1", + "status": 200, + "response_type": "text/html", + "response_text": "" + "", + } + ] + assert app_ui_controls(lines, 0) == ["Delete", "Void encounter"] + workflow = Workflow(name="one", steps=[_save("save_triage", "Triage")]) + candidates, _ = find_candidates(workflow, 0, Sources(network_lines=lines)) + ui = [c for c in candidates if c.source == "app_ui"] + assert len(ui) == 2 and all(c.undo is None for c in ui) + assert "recording of a person using it" in ui[0].problem + + +def test_a_save_without_a_bound_record_effect_gets_no_candidate(): + step = _save("save_triage", "Triage") + step.effects = [ + step.effects[0].model_copy(update={"needs_operator_confirmation": True}) + ] + workflow = Workflow(name="one", steps=[step]) + candidates, notes = find_candidates(workflow, 0, Sources(network_lines=_network())) + assert candidates == [] + assert "declares no record effect" in notes[0] + + +def test_compare_snapshots_reports_counts_and_field_names_only(): + before = [{"id": 1, "patient_id": "p1", "note": "a"}] + assert compare_snapshots(before, [dict(before[0])]) is None + assert compare_snapshots( + before, [{"id": 1, "patient_id": "p1", "note": "a", "at": 2}], ignore={"at"} + ) is None + why = compare_snapshots(before, [{"id": 1, "patient_id": "p1", "note": "b"}]) + assert "1 record(s) changed (fields: note)" in why + assert "p1" not in why and "'b'" not in why + + +def _locating_workflow() -> Workflow: + undo = UndoStep( + id="u1", + undoes="save_triage", + intent="delete it", + source="synthesized", + steps=[ + Step( + id="d", + intent="delete", + action=ActionKind.WAIT, + effects=[ + Effect( + kind=EffectKind.RECORD_WRITTEN, + match={"id": ValueExpr(param="undo_record_id")}, + expected_count=0, + ) + ], + ) + ], + locate={"match": {"patient_id": {"literal": "p1"}}}, + ) + return Workflow(name="w", steps=[_save("save_triage", "Triage")], undo_steps=[undo]) + + +def test_locate_needs_exactly_one_new_record(tmp_path): + workflow = _locating_workflow() + undo = workflow.undo_steps[0] + before = [{"id": 1, "patient_id": "p1"}] + with pytest.raises(UndoRefused, match="kept no values"): + resolve_undo_params(undo, tmp_path, {}, lambda: before) + keep_prior_state(workflow, workflow.steps[0], before, {}, tmp_path) + assert (prior_state_path(tmp_path, "save_triage").stat().st_mode & 0o777) == 0o600 + with pytest.raises(UndoRefused, match="0 record"): + resolve_undo_params(undo, tmp_path, {}, lambda: before) + two = before + [{"id": 2, "patient_id": "p1"}, {"id": 3, "patient_id": "p1"}] + with pytest.raises(UndoRefused, match="2 record"): + resolve_undo_params(undo, tmp_path, {}, lambda: two) + with pytest.raises(UndoRefused, match="could not be read"): + resolve_undo_params(undo, tmp_path, {}, lambda: None) + one = before + [{"id": 2, "patient_id": "p1"}] + assert resolve_undo_params(undo, tmp_path, {"x": "y"}, lambda: one) == { + "x": "y", + "undo_record_id": "2", + } + + +def test_a_declared_undo_serializes_exactly_as_before(): + undo = _locating_workflow().undo_steps[0].model_copy(update={"locate": None}) + dumped = undo.model_dump(mode="json") + assert set(dumped) == {"id", "undoes", "intent", "source", "steps"} + + +def test_accept_refuses_changed_evidence_and_a_second_approval(tmp_path, mockmed): + workflow = Workflow(name="one-save", steps=[_save("save_triage", "Triage")]) + bundle = _bundle(tmp_path, workflow) + _report, verified = propose_undos( + workflow, + bundle_dir=bundle, + sources=Sources(network_lines=_network()), + target=MockMedCopy(mockmed), + work_dir=tmp_path / "work", + ) + written = write_proposals(bundle, verified) + undo = written.undo_steps[0] + evidence = bundle / undo.proposal.evidence_path + original = evidence.read_bytes() + evidence.write_bytes(original.replace(b'"verified"', b'"verified" ')) + with pytest.raises(ValueError, match="changed since the undo was verified"): + accept_proposal(bundle, undo.id, approver="nurse-a") + evidence.write_bytes(original) + accept_proposal(bundle, undo.id, approver="nurse-a") + with pytest.raises(ValueError, match="already approved"): + accept_proposal(bundle, undo.id, approver="nurse-b") + + +def test_cli_propose_refuses_a_target_not_declared_a_test_copy(tmp_path, capsys): + from openadapt_flow.__main__ import main + + bundle = _bundle(tmp_path, _workflow()) + config = tmp_path / "deployment.yaml" + config.write_text("name: prod\nbackend:\n kind: web\n url: http://127.0.0.1:9\n") + code = main(["undo", "propose", str(bundle), "--config", str(config)]) + assert code == 3 + err = capsys.readouterr().err + assert "does not declare its target a test copy" in err + assert Workflow.load(bundle).undo_steps == [] + + +def test_cli_propose_without_a_test_copy_lists_and_writes_nothing(tmp_path, capsys): + from openadapt_flow.__main__ import main + + bundle = _bundle(tmp_path, _workflow()) + out = tmp_path / "report.json" + code = main(["undo", "propose", str(bundle), "--report", str(out)]) + assert code == 0 + payload = json.loads(out.read_text()) + assert payload["summary"] == { + "saving_steps": 2, + "verified_undo": 0, + "no_verified_undo": 2, + } + assert "0 with a verified undo" in capsys.readouterr().out + + +def test_cli_accept_needs_a_person_at_a_terminal(tmp_path, capsys): + from openadapt_flow.__main__ import main + + bundle = _bundle(tmp_path, _workflow()) + assert main(["undo", "accept", str(bundle), "--undo", "x"]) == 2 + assert "interactive terminal" in capsys.readouterr().err + + +def test_a_bundle_with_a_located_undo_round_trips(tmp_path): + # A bundle holding a proposal loads and saves byte-identically. + workflow = _locating_workflow() + bundle = _bundle(tmp_path, workflow) + first = (bundle / "workflow.json").read_bytes() + Workflow.load(bundle).save(bundle) + assert (bundle / "workflow.json").read_bytes() == first + + +def test_the_save_request_is_found_when_the_selector_has_a_field_it_lacks(): + # MockLoan-style: the record carries ``surface``, the request does not. + step = _save("save_triage", "Triage") + effect = step.effects[0].model_copy( + update={"match": {**step.effects[0].match, "surface": ValueExpr(literal="x")}} + ) + [candidate], note = network_candidates(step, effect, _network()[:1], {}) + assert note == "" + assert candidate.undo.locate.match["surface"] == {"literal": "x", "param": None} + # A request whose named field disagrees does not explain the save. + wrong = _network()[:1] + wrong[0]["body"] = json.dumps({"patient_id": "p9", "type": "Triage"}) + assert "could not be found" in network_candidates(step, effect, wrong, {})[1] From bf6c96f28986b52f260bbe49b39676ec101600e4 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 11:11:27 -0400 Subject: [PATCH 02/11] undo propose: form-post and GraphQL reasons, offline rows, undo list hint Co-Authored-By: Claude Opus 5.5 --- .../flow/benchmark/undo_proposal/README.md | 47 ++++++++++ .../results/openemr-new-patient.json | 36 ++++++++ .../undo_proposal/results/openimis-claim.json | 20 +++++ .../undo_proposal/results/summary.json | 65 ++++++++++++++ packages/flow/benchmark/undo_proposal/run.py | 89 +++++++++++++++---- packages/flow/openadapt_flow/cli_undo.py | 13 +++ packages/flow/openadapt_flow/undo_proposal.py | 40 ++++++--- packages/flow/tests/test_undo_proposal.py | 16 +++- 8 files changed, 298 insertions(+), 28 deletions(-) create mode 100644 packages/flow/benchmark/undo_proposal/README.md create mode 100644 packages/flow/benchmark/undo_proposal/results/openemr-new-patient.json create mode 100644 packages/flow/benchmark/undo_proposal/results/openimis-claim.json create mode 100644 packages/flow/benchmark/undo_proposal/results/summary.json diff --git a/packages/flow/benchmark/undo_proposal/README.md b/packages/flow/benchmark/undo_proposal/README.md new file mode 100644 index 0000000000..dbe3d57d2c --- /dev/null +++ b/packages/flow/benchmark/undo_proposal/README.md @@ -0,0 +1,47 @@ +# Verified undo coverage (`undo propose`) + +How many saving steps get an undo that a test copy shows to work, and why the +rest don't. The method is in +[`docs/AFTER_A_STOP.md`](../../docs/AFTER_A_STOP.md#proposed-undo-steps). + +## Method + +Live rows (`run.py`, one fixture at a time): + +1. Start the synthetic app and its record store in process, on 127.0.0.1. +2. Record the app's canonical demonstration in a real Chromium with network + capture on (the demo driver's clicks). +3. Compile it. Bind the save's record check as an operator would: the fields + the save's request carries, counted as new records only. +4. Write a deployment file that declares the app a test copy: a reset URL and + a `rest` reward contract over the whole record store (`/api/db`). +5. Run `openadapt-flow undo propose` with that file and the recording. + +Offline rows read an existing bundle and recording and run nothing +(`--offline`). They answer whether any candidate can be built. They were used +for the OpenEMR and openIMIS fixtures, which are shared with the scoreboard +and were not started for this. + +Synthetic data only. No model calls. + +## Results + +RESULTS_TABLE + +The full reports are in [`results/`](results/): one JSON file per fixture +(each candidate's source, status, basis and reason, and for a live row the +digests and counts of the three snapshots) and `summary.json`. + +## Reproduce + +```bash +cd packages/flow +PYTHONPATH=../types:. python benchmark/undo_proposal/run.py --out benchmark/undo_proposal/results --fixture mockmed +``` + +Use `--fixture mockloan` and `--fixture mockloan-fee-row` for the other live +rows. For an offline row: + +```bash +PYTHONPATH=../types:. python benchmark/undo_proposal/run.py --out benchmark/undo_proposal/results --offline NAME BUNDLE RECORDING "what the save does" +``` diff --git a/packages/flow/benchmark/undo_proposal/results/openemr-new-patient.json b/packages/flow/benchmark/undo_proposal/results/openemr-new-patient.json new file mode 100644 index 0000000000..6031add661 --- /dev/null +++ b/packages/flow/benchmark/undo_proposal/results/openemr-new-patient.json @@ -0,0 +1,36 @@ +{ + "saving_steps": [ + { + "candidates": [], + "intent": "click recorded visual target", + "reason": "no candidate: the save declares no record effect Flow can check (bind its record_written or field_equals effect first)", + "step_id": "step_039", + "verified_undo": null + }, + { + "candidates": [ + { + "basis": "pages loaded after the save have 'Cancel' controls", + "candidate_id": "step_040:app_ui", + "evidence": null, + "reason": "Flow found 'Cancel' on the pages after the save but builds an undo from a control only from a recording of a person using it; record that and pass it with --from-recording", + "source": "app_ui", + "status": "not_run" + } + ], + "intent": "click recorded visual target", + "reason": "app_ui: Flow found 'Cancel' on the pages after the save but builds an undo from a control only from a recording of a person using it; record that and pass it with --from-recording (also: no other recordings were given; the save is a browser form post (POST /interface/new/new_comprehensive_save.php); Flow builds an API undo only from a JSON request; the save creates a record; there is no old value to restore)", + "step_id": "step_040", + "verified_undo": null + } + ], + "schema": "openadapt.undo-propose-report/v1", + "summary": { + "no_verified_undo": 2, + "saving_steps": 2, + "verified_undo": 0 + }, + "test_copy": null, + "workflow_name": "openemr-create-synthetic-patient", + "written": false +} \ No newline at end of file diff --git a/packages/flow/benchmark/undo_proposal/results/openimis-claim.json b/packages/flow/benchmark/undo_proposal/results/openimis-claim.json new file mode 100644 index 0000000000..ec19be9628 --- /dev/null +++ b/packages/flow/benchmark/undo_proposal/results/openimis-claim.json @@ -0,0 +1,20 @@ +{ + "saving_steps": [ + { + "candidates": [], + "intent": "click recorded visual target", + "reason": "no candidate: no other recordings were given; the recording has no network capture (record --capture-network); the save creates a record; there is no old value to restore", + "step_id": "step_015", + "verified_undo": null + } + ], + "schema": "openadapt.undo-propose-report/v1", + "summary": { + "no_verified_undo": 1, + "saving_steps": 1, + "verified_undo": 0 + }, + "test_copy": null, + "workflow_name": "openimis.claim_entry", + "written": false +} \ No newline at end of file diff --git a/packages/flow/benchmark/undo_proposal/results/summary.json b/packages/flow/benchmark/undo_proposal/results/summary.json new file mode 100644 index 0000000000..fe5d7e6090 --- /dev/null +++ b/packages/flow/benchmark/undo_proposal/results/summary.json @@ -0,0 +1,65 @@ +{ + "schema": "openadapt.undo-propose-measurement/v1", + "engine_commit": "1a73a547f73068bd5170118d680b4b1e68587a0f", + "measured_at": "2026-10-19T11:09:49-0400", + "host": "macOS-15.7.3-arm64-arm-64bit", + "load_1m": 27.67, + "fixtures": [ + { + "fixture": "openemr-new-patient", + "what_the_save_does": "OpenEMR fixture: the new-patient form saves one chart (browser form post)", + "mode": "offline (no test copy; nothing was run)", + "exit_code": 0, + "summary": { + "no_verified_undo": 2, + "saving_steps": 2, + "verified_undo": 0 + }, + "steps": [ + { + "step_id": "step_039", + "verified_undo": null, + "reason": "no candidate: the save declares no record effect Flow can check (bind its record_written or field_equals effect first)", + "candidates": [] + }, + { + "step_id": "step_040", + "verified_undo": null, + "reason": "app_ui: Flow found 'Cancel' on the pages after the save but builds an undo from a control only from a recording of a person using it; record that and pass it with --from-recording (also: no other recordings were given; the save is a browser form post (POST /interface/new/new_comprehensive_save.php); Flow builds an API undo only from a JSON request; the save creates a record; there is no old value to restore)", + "candidates": [ + { + "source": "app_ui", + "status": "not_run", + "basis": "pages loaded after the save have 'Cancel' controls", + "reason": "Flow found 'Cancel' on the pages after the save but builds an undo from a control only from a recording of a person using it; record that and pass it with --from-recording" + } + ] + } + ] + }, + { + "fixture": "openimis-claim", + "what_the_save_does": "openIMIS fixture: the claim form saves one claim (scoreboard bundle; its recording has no network capture)", + "mode": "offline (no test copy; nothing was run)", + "exit_code": 0, + "summary": { + "no_verified_undo": 1, + "saving_steps": 1, + "verified_undo": 0 + }, + "steps": [ + { + "step_id": "step_015", + "verified_undo": null, + "reason": "no candidate: no other recordings were given; the recording has no network capture (record --capture-network); the save creates a record; there is no old value to restore", + "candidates": [] + } + ] + } + ], + "totals": { + "saving_steps": 3, + "verified_undo": 0, + "no_verified_undo": 3 + } +} diff --git a/packages/flow/benchmark/undo_proposal/run.py b/packages/flow/benchmark/undo_proposal/run.py index 4b4e81bd9d..61c6c3b6e5 100644 --- a/packages/flow/benchmark/undo_proposal/run.py +++ b/packages/flow/benchmark/undo_proposal/run.py @@ -13,8 +13,8 @@ Synthetic data only; nothing leaves 127.0.0.1 and no model is called. - PYTHONPATH=packages/types:packages/flow python -m benchmark.undo_proposal.run \\ - --out packages/flow/benchmark/undo_proposal/results + cd packages/flow && PYTHONPATH=../types:. python benchmark/undo_proposal/run.py \\ + --out benchmark/undo_proposal/results Run one fixture at a time (``--fixture mockmed``) when the machine is busy. """ @@ -25,6 +25,7 @@ import json import os import platform +import shutil import subprocess import tempfile import time @@ -267,22 +268,57 @@ def run_fixture(fixture: Fixture, out_dir: Path, work: Path) -> dict[str, Any]: return { "fixture": fixture.name, "what_the_save_does": fixture.note, + "mode": "live (real browser, app declared a test copy)", "exit_code": code, "save_step": step_id, "seconds": round(time.monotonic() - started, 1), "summary": report["summary"], - "steps": [ - { - "step_id": row["step_id"], - "verified_undo": row["verified_undo"], - "reason": row["reason"], - "candidates": [ - {k: c[k] for k in ("source", "status", "basis", "reason")} - for c in row["candidates"] - ], - } - for row in report["saving_steps"] - ], + "steps": _rows(report), + } + + +def _rows(report: dict[str, Any]) -> list[dict[str, Any]]: + return [ + { + "step_id": row["step_id"], + "verified_undo": row["verified_undo"], + "reason": row["reason"], + "candidates": [ + {k: c[k] for k in ("source", "status", "basis", "reason")} + for c in row["candidates"] + ], + } + for row in report["saving_steps"] + ] + + +def run_offline( + name: str, bundle: Path, recording: Optional[Path], out_dir: Path, note: str +) -> dict[str, Any]: + """Candidates from an existing bundle and recording, with no test copy. + + Used where the fixture is shared or down: nothing is reset or run, so a + buildable candidate stays ``not_run``. It answers whether any candidate + can be built at all. + """ + from openadapt_flow.__main__ import main + + report_path = out_dir / f"{name}.json" + with tempfile.TemporaryDirectory(prefix=f"undo-{name}-") as tmp: + copy = Path(tmp) / "bundle" + shutil.copytree(bundle, copy) + argv = ["undo", "propose", str(copy), "--report", str(report_path)] + if recording is not None: + argv += ["--recording", str(recording)] + code = main(argv) + report = json.loads(report_path.read_text(encoding="utf-8")) + return { + "fixture": name, + "what_the_save_does": note, + "mode": "offline (no test copy; nothing was run)", + "exit_code": code, + "summary": report["summary"], + "steps": _rows(report), } @@ -290,14 +326,37 @@ def main(argv: Optional[list[str]] = None) -> int: parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) parser.add_argument("--out", required=True, type=Path) parser.add_argument("--fixture", action="append", default=None) + parser.add_argument( + "--offline", + nargs=4, + metavar=("NAME", "BUNDLE", "RECORDING", "NOTE"), + action="append", + default=[], + help=( + "Also list candidates for an existing bundle and recording without " + "a test copy (RECORDING may be '-')" + ), + ) args = parser.parse_args(argv) fixtures = _fixtures() - names = args.fixture or list(fixtures) + names = args.fixture if args.fixture is not None else list(fixtures) + if args.offline and args.fixture is None: + names = [] args.out.mkdir(parents=True, exist_ok=True) rows = [] for name in names: with tempfile.TemporaryDirectory(prefix=f"undo-{name}-") as tmp: rows.append(run_fixture(fixtures[name], args.out, Path(tmp))) + for name, bundle, recording, note in args.offline: + rows.append( + run_offline( + name, + Path(bundle), + None if recording == "-" else Path(recording), + args.out, + note, + ) + ) summary_path = args.out / "summary.json" previous: dict[str, Any] = {} if summary_path.is_file(): diff --git a/packages/flow/openadapt_flow/cli_undo.py b/packages/flow/openadapt_flow/cli_undo.py index 7232b56a51..958c9f0ab5 100644 --- a/packages/flow/openadapt_flow/cli_undo.py +++ b/packages/flow/openadapt_flow/cli_undo.py @@ -259,8 +259,10 @@ def _cmd_undo_list(args: argparse.Namespace) -> int: payload.append(item) print(json.dumps(payload, indent=2, sort_keys=True)) return 0 + waiting = [u for u in workflow.undo_steps if u.awaiting_approval] if not offers: print("No undo is offered for this run.") + _print_waiting(waiting, args.bundle) return 0 print("Undos offered for this run (undo the latest change first):") quoted_run = shlex.quote(str(run_dir)) @@ -301,9 +303,20 @@ def _cmd_undo_list(args: argparse.Namespace) -> int: f"--undo {offer.undo_id}" ) ) + _print_waiting(waiting, args.bundle) return 0 +def _print_waiting(waiting: list[UndoStep], bundle: str) -> None: + """Name the proposals a person has not approved yet; Flow offers none.""" + for undo in waiting: + print( + f" {undo.id} (undoes step `{undo.undoes}`) is a verified proposal " + "that no person has approved, so it is not offered. Approve it once: " + + command(f"undo accept {shlex.quote(str(bundle))} --undo {undo.id}") + ) + + def _cmd_undo_approve(args: argparse.Namespace) -> int: run_dir = Path(args.run_dir) workflow = Workflow.load(args.bundle) diff --git a/packages/flow/openadapt_flow/undo_proposal.py b/packages/flow/openadapt_flow/undo_proposal.py index a5db6bd0a7..fc501b1ef3 100644 --- a/packages/flow/openadapt_flow/undo_proposal.py +++ b/packages/flow/openadapt_flow/undo_proposal.py @@ -374,7 +374,12 @@ def _explains(items: dict[str, set[str]], wanted: dict[str, str]) -> bool: """The request names at least one field of the save's record selector, and every selector field it names has the selector's value.""" named = [key for key in wanted if key in items] - return bool(named) and all(wanted[key] in items[key] for key in named) + if named: + return all(wanted[key] in items[key] for key in named) + # A form names its fields its own way (``form_lname`` for ``lname``): + # then every selector value must be among the request's values. + values = {value for found in items.values() for value in found} + return bool(wanted) and set(wanted.values()) <= values def _save_request( @@ -456,6 +461,13 @@ def network_candidates( Only JSON requests become undo steps: a browser form post needs the live page's session token, which a plain API call does not have. """ + binding = step.api_binding + if binding is not None and binding.kind == "web_form": + return [], ( + f"the save is a browser form post ({binding.method} " + f"{_path(binding.url_template)}); Flow builds an API undo only " + "from a JSON request" + ) if not lines: return [], "the recording has no network capture (record --capture-network)" wanted = _selector_values(effect, params) @@ -469,6 +481,11 @@ def network_candidates( method = str(save.get("method", "")).upper() kind, _items = _request_items(save) path = _path(str(save.get("url", ""))) + if "graphql" in path.lower(): + return [], ( + f"the save is a GraphQL mutation ({path}); Flow doesn't derive an " + "inverse mutation" + ) if kind != "json": return [], ( f"the save is a browser form post ({method} {path}); Flow builds " @@ -581,6 +598,7 @@ def network_candidates( def app_ui_controls(lines: Sequence[dict[str, Any]], after: int) -> list[str]: """Labels of delete/cancel controls on the pages loaded after the save.""" labels: list[str] = [] + seen: set[str] = set() for line in lines[after + 1 :]: if "html" not in str(line.get("response_type", "")): continue @@ -588,7 +606,8 @@ def app_ui_controls(lines: Sequence[dict[str, Any]], after: int) -> list[str]: text = match.group(2) if match.group(2) is not None else match.group(3) label = html.unescape(_TAGS.sub("", text or "")).strip() if label and len(label) <= 40 and INVERSE_WORDS.search(label): - if label not in labels: + if label.lower() not in seen: + seen.add(label.lower()) labels.append(label) return labels @@ -703,19 +722,20 @@ def find_candidates( if sources.network_lines: wanted = _selector_values(effect, params) at = _save_request(sources.network_lines, wanted) - for label in app_ui_controls( - sources.network_lines, at if at is not None else -1 - ): + labels = app_ui_controls(sources.network_lines, at if at is not None else -1) + if labels: + named = ", ".join(repr(label) for label in labels) out.append( Candidate( - candidate_id=f"{step.id}:app_ui:{label}", + candidate_id=f"{step.id}:app_ui", undoes=step.id, source="app_ui", - basis=f"a {label!r} control is on a page loaded after the save", + basis=f"pages loaded after the save have {named} controls", problem=( - "Flow builds an undo from a control only from a " - "recording of a person using it; record that and pass " - "it with --from-recording" + f"Flow found {named} on the pages after the save but " + "builds an undo from a control only from a recording of " + "a person using it; record that and pass it with " + "--from-recording" ), ) ) diff --git a/packages/flow/tests/test_undo_proposal.py b/packages/flow/tests/test_undo_proposal.py index b22973b6e4..86fcc0fbcb 100644 --- a/packages/flow/tests/test_undo_proposal.py +++ b/packages/flow/tests/test_undo_proposal.py @@ -203,7 +203,7 @@ def _bundle(tmp_path: Path, workflow: Workflow) -> Path: def test_network_undo_is_verified_written_as_a_proposal_and_waits_for_a_person( - tmp_path, mockmed + tmp_path, mockmed, capsys ): workflow = _workflow() bundle = _bundle(tmp_path, workflow) @@ -269,6 +269,14 @@ def test_network_undo_is_verified_written_as_a_proposal_and_waits_for_a_person( assert build_change_ledger(stopped, run_wf)[0].undo_id is None with pytest.raises(UndoNotOffered): approve_undo(run_dir, run_wf, undo.id, approver="nurse-a", bundle_dir=bundle) + from openadapt_flow.__main__ import main + + capsys.readouterr() + assert main(["undo", "list", str(run_dir), "--bundle", str(bundle)]) == 0 + listed = capsys.readouterr().out + assert "No undo is offered for this run." in listed + assert f"{undo.id} (undoes step `save_triage`) is a verified proposal" in listed + assert "undo accept" in listed # A person approves the proposal once; the bundle is re-sealed. accepted = accept_proposal(bundle, undo.id, approver="nurse-a") @@ -677,8 +685,10 @@ def test_app_ui_control_is_reported_and_never_built(): workflow = Workflow(name="one", steps=[_save("save_triage", "Triage")]) candidates, _ = find_candidates(workflow, 0, Sources(network_lines=lines)) ui = [c for c in candidates if c.source == "app_ui"] - assert len(ui) == 2 and all(c.undo is None for c in ui) - assert "recording of a person using it" in ui[0].problem + [only] = ui + assert only.undo is None + assert "'Delete', 'Void encounter'" in only.problem + assert "recording of a person using it" in only.problem def test_a_save_without_a_bound_record_effect_gets_no_candidate(): From b5189495df43a0884b7ce316b7502e3e89f171bc Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 11:16:21 -0400 Subject: [PATCH 03/11] undo propose: say what would close each step's gap; offline OpenEMR/openIMIS rows Co-Authored-By: Claude Opus 5.5 --- .../results/openemr-new-patient.json | 2 + .../undo_proposal/results/openimis-claim.json | 1 + .../undo_proposal/results/summary.json | 11 +++-- packages/flow/benchmark/undo_proposal/run.py | 1 + packages/flow/openadapt_flow/cli_undo.py | 1 + packages/flow/openadapt_flow/undo_proposal.py | 32 ++++++++++++ packages/flow/tests/test_undo_proposal.py | 49 ++++++++++++++++--- 7 files changed, 87 insertions(+), 10 deletions(-) diff --git a/packages/flow/benchmark/undo_proposal/results/openemr-new-patient.json b/packages/flow/benchmark/undo_proposal/results/openemr-new-patient.json index 6031add661..f04ab9a1f8 100644 --- a/packages/flow/benchmark/undo_proposal/results/openemr-new-patient.json +++ b/packages/flow/benchmark/undo_proposal/results/openemr-new-patient.json @@ -5,6 +5,7 @@ "intent": "click recorded visual target", "reason": "no candidate: the save declares no record effect Flow can check (bind its record_written or field_equals effect first)", "step_id": "step_039", + "to_close_the_gap": "bind the save's record check (a record_written or field_equals effect), then propose again", "verified_undo": null }, { @@ -21,6 +22,7 @@ "intent": "click recorded visual target", "reason": "app_ui: Flow found 'Cancel' on the pages after the save but builds an undo from a control only from a recording of a person using it; record that and pass it with --from-recording (also: no other recordings were given; the save is a browser form post (POST /interface/new/new_comprehensive_save.php); Flow builds an API undo only from a JSON request; the save creates a record; there is no old value to restore)", "step_id": "step_040", + "to_close_the_gap": "record a person doing the app's own delete or cancel path (pages loaded after the save have 'Cancel' controls) and pass it with --from-recording", "verified_undo": null } ], diff --git a/packages/flow/benchmark/undo_proposal/results/openimis-claim.json b/packages/flow/benchmark/undo_proposal/results/openimis-claim.json index ec19be9628..0245fe0d0e 100644 --- a/packages/flow/benchmark/undo_proposal/results/openimis-claim.json +++ b/packages/flow/benchmark/undo_proposal/results/openimis-claim.json @@ -5,6 +5,7 @@ "intent": "click recorded visual target", "reason": "no candidate: no other recordings were given; the recording has no network capture (record --capture-network); the save creates a record; there is no old value to restore", "step_id": "step_015", + "to_close_the_gap": "record a person doing the app's own delete or cancel path and pass it with --from-recording", "verified_undo": null } ], diff --git a/packages/flow/benchmark/undo_proposal/results/summary.json b/packages/flow/benchmark/undo_proposal/results/summary.json index fe5d7e6090..d484d2179c 100644 --- a/packages/flow/benchmark/undo_proposal/results/summary.json +++ b/packages/flow/benchmark/undo_proposal/results/summary.json @@ -1,9 +1,9 @@ { "schema": "openadapt.undo-propose-measurement/v1", - "engine_commit": "1a73a547f73068bd5170118d680b4b1e68587a0f", - "measured_at": "2026-10-19T11:09:49-0400", + "engine_commit": "835803ff14f7e0da6ee1b8de3f7ba37157819e6c", + "measured_at": "2026-10-19T11:16:15-0400", "host": "macOS-15.7.3-arm64-arm-64bit", - "load_1m": 27.67, + "load_1m": 135.74, "fixtures": [ { "fixture": "openemr-new-patient", @@ -20,12 +20,14 @@ "step_id": "step_039", "verified_undo": null, "reason": "no candidate: the save declares no record effect Flow can check (bind its record_written or field_equals effect first)", + "to_close_the_gap": "bind the save's record check (a record_written or field_equals effect), then propose again", "candidates": [] }, { "step_id": "step_040", "verified_undo": null, "reason": "app_ui: Flow found 'Cancel' on the pages after the save but builds an undo from a control only from a recording of a person using it; record that and pass it with --from-recording (also: no other recordings were given; the save is a browser form post (POST /interface/new/new_comprehensive_save.php); Flow builds an API undo only from a JSON request; the save creates a record; there is no old value to restore)", + "to_close_the_gap": "record a person doing the app's own delete or cancel path (pages loaded after the save have 'Cancel' controls) and pass it with --from-recording", "candidates": [ { "source": "app_ui", @@ -39,7 +41,7 @@ }, { "fixture": "openimis-claim", - "what_the_save_does": "openIMIS fixture: the claim form saves one claim (scoreboard bundle; its recording has no network capture)", + "what_the_save_does": "openIMIS fixture: the claim form saves one claim (scoreboard bundle; its recording has no network capture; the live fixture was not started)", "mode": "offline (no test copy; nothing was run)", "exit_code": 0, "summary": { @@ -52,6 +54,7 @@ "step_id": "step_015", "verified_undo": null, "reason": "no candidate: no other recordings were given; the recording has no network capture (record --capture-network); the save creates a record; there is no old value to restore", + "to_close_the_gap": "record a person doing the app's own delete or cancel path and pass it with --from-recording", "candidates": [] } ] diff --git a/packages/flow/benchmark/undo_proposal/run.py b/packages/flow/benchmark/undo_proposal/run.py index 61c6c3b6e5..5dbffaec7f 100644 --- a/packages/flow/benchmark/undo_proposal/run.py +++ b/packages/flow/benchmark/undo_proposal/run.py @@ -283,6 +283,7 @@ def _rows(report: dict[str, Any]) -> list[dict[str, Any]]: "step_id": row["step_id"], "verified_undo": row["verified_undo"], "reason": row["reason"], + "to_close_the_gap": row.get("to_close_the_gap", ""), "candidates": [ {k: c[k] for k in ("source", "status", "basis", "reason")} for c in row["candidates"] diff --git a/packages/flow/openadapt_flow/cli_undo.py b/packages/flow/openadapt_flow/cli_undo.py index 958c9f0ab5..622b81dc9a 100644 --- a/packages/flow/openadapt_flow/cli_undo.py +++ b/packages/flow/openadapt_flow/cli_undo.py @@ -468,6 +468,7 @@ def _cmd_undo_propose(args: argparse.Namespace) -> int: print(f" {row.step_id}: verified undo {row.verified_undo}") else: print(f" {row.step_id}: no verified undo. {row.reason}") + print(f" To close the gap: {row.to_close_the_gap}.") if report.written: quoted = shlex.quote(str(bundle)) print( diff --git a/packages/flow/openadapt_flow/undo_proposal.py b/packages/flow/openadapt_flow/undo_proposal.py index fc501b1ef3..98ad381309 100644 --- a/packages/flow/openadapt_flow/undo_proposal.py +++ b/packages/flow/openadapt_flow/undo_proposal.py @@ -132,6 +132,8 @@ class SavingStepResult(BaseModel): verified_undo: Optional[str] = None #: Why the step has no verified undo (empty when it has one). reason: str = "" + #: What would give the step a verified undo (empty when it has one). + to_close_the_gap: str = "" candidates: list[CandidateResult] = Field(default_factory=list) @@ -1173,12 +1175,42 @@ def propose_undos( row.reason = "no candidate: " + "; ".join(notes) if notes and row.candidates: row.reason += " (also: " + "; ".join(notes) + ")" + row.to_close_the_gap = _gap_hint(row, notes, target is not None) finally: if own_tmp: shutil.rmtree(work, ignore_errors=True) return report, verified +def _gap_hint(row: SavingStepResult, notes: list[str], had_target: bool) -> str: + """What a person can do so the step gets a verified undo.""" + if any("declares no record effect" in note for note in notes): + return ( + "bind the save's record check (a record_written or field_equals " + "effect), then propose again" + ) + if any(c.status == "failed" for c in row.candidates): + return ( + "record a person doing the app's own delete or cancel path and pass " + "it with --from-recording; if the only difference is a timestamp or " + "audit field, a person may name it with --ignore-field" + ) + built = [c for c in row.candidates if c.source != "app_ui"] + if not had_target and any(c.status == "not_run" for c in built): + return "run again with --config naming a declared test copy" + ui = next((c for c in row.candidates if c.source == "app_ui"), None) + if ui is not None: + return ( + "record a person doing the app's own delete or cancel path (" + + ui.basis + + ") and pass it with --from-recording" + ) + return ( + "record a person doing the app's own delete or cancel path and pass it " + "with --from-recording" + ) + + def write_proposals( bundle_dir: Path | str, verified: Mapping[str, tuple[Candidate, CandidateResult]], diff --git a/packages/flow/tests/test_undo_proposal.py b/packages/flow/tests/test_undo_proposal.py index 86fcc0fbcb..ba98690f00 100644 --- a/packages/flow/tests/test_undo_proposal.py +++ b/packages/flow/tests/test_undo_proposal.py @@ -329,9 +329,7 @@ def _undo_replayer(url: str) -> Replayer: ) -def test_a_wrong_endpoint_fails_with_its_reason_and_is_never_written( - tmp_path, mockmed -): +def test_a_wrong_endpoint_fails_with_its_reason_and_is_never_written(tmp_path, mockmed): workflow = Workflow(name="one-save", steps=[_save("save_triage", "Triage")]) bundle = _bundle(tmp_path, workflow) report, verified = propose_undos( @@ -705,9 +703,12 @@ def test_a_save_without_a_bound_record_effect_gets_no_candidate(): def test_compare_snapshots_reports_counts_and_field_names_only(): before = [{"id": 1, "patient_id": "p1", "note": "a"}] assert compare_snapshots(before, [dict(before[0])]) is None - assert compare_snapshots( - before, [{"id": 1, "patient_id": "p1", "note": "a", "at": 2}], ignore={"at"} - ) is None + assert ( + compare_snapshots( + before, [{"id": 1, "patient_id": "p1", "note": "a", "at": 2}], ignore={"at"} + ) + is None + ) why = compare_snapshots(before, [{"id": 1, "patient_id": "p1", "note": "b"}]) assert "1 record(s) changed (fields: note)" in why assert "p1" not in why and "'b'" not in why @@ -848,3 +849,39 @@ def test_the_save_request_is_found_when_the_selector_has_a_field_it_lacks(): wrong = _network()[:1] wrong[0]["body"] = json.dumps({"patient_id": "p9", "type": "Triage"}) assert "could not be found" in network_candidates(step, effect, wrong, {})[1] + + +def test_each_step_without_an_undo_says_what_would_close_the_gap(tmp_path): + unbound = _save("save_unbound", "Triage") + unbound.effects = [ + unbound.effects[0].model_copy(update={"needs_operator_confirmation": True}) + ] + form = _save("save_form", "Vitals") + workflow = Workflow(name="two", steps=[unbound, form]) + bundle = _bundle(tmp_path, workflow) + lines = [ + { + "method": "POST", + "url": "http://x/save.php", + "form": [["patient_id", "p1"], ["type", "Vitals"]], + "status": 200, + }, + { + "method": "GET", + "url": "http://x/chart", + "status": 200, + "response_type": "text/html", + "response_text": "Delete", + }, + ] + report, _ = propose_undos( + workflow, + bundle_dir=bundle, + sources=Sources(network_lines=lines), + target=None, + ) + first, second = report.saving_steps + assert "bind the save's record check" in first.to_close_the_gap + assert "browser form post" in second.reason + assert "the app's own delete or cancel path" in second.to_close_the_gap + assert "'Delete'" in second.to_close_the_gap From 929f205aaa516552b20460e2d45e9cb41d379f2e Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 11:24:57 -0400 Subject: [PATCH 04/11] undo propose: fix test-copy snapshot identity; live MockLoan rows Co-Authored-By: Claude Opus 5.5 --- .../results/mockloan-fee-row.json | 61 ++++++++++++++++ .../undo_proposal/results/mockloan.json | 61 ++++++++++++++++ .../undo_proposal/results/summary.json | 72 +++++++++++++++++-- packages/flow/benchmark/undo_proposal/run.py | 33 ++++++--- packages/flow/openadapt_flow/undo_proposal.py | 9 ++- packages/flow/tests/test_undo_proposal.py | 44 ++++++++++++ 6 files changed, 265 insertions(+), 15 deletions(-) create mode 100644 packages/flow/benchmark/undo_proposal/results/mockloan-fee-row.json create mode 100644 packages/flow/benchmark/undo_proposal/results/mockloan.json diff --git a/packages/flow/benchmark/undo_proposal/results/mockloan-fee-row.json b/packages/flow/benchmark/undo_proposal/results/mockloan-fee-row.json new file mode 100644 index 0000000000..57a6137d97 --- /dev/null +++ b/packages/flow/benchmark/undo_proposal/results/mockloan-fee-row.json @@ -0,0 +1,61 @@ +{ + "saving_steps": [ + { + "candidates": [ + { + "basis": "the save's JSON POST /api/disbursement returned a record id; REST convention gives DELETE /api/disbursement/{undo_record_id}", + "candidate_id": "undo-step_010-network-1", + "evidence": { + "after_save": { + "records": 2, + "sha256": "7684265eafe6dde20e630141fe6de95396b8533ad769a34745032c5859956d85" + }, + "after_undo": { + "records": 1, + "sha256": "41cdfc3100b763dd693744a12ccf4b1aae8de5b1a6f18a22f7f89950a836c8fa" + }, + "basis": "the save's JSON POST /api/disbursement returned a record id; REST convention gives DELETE /api/disbursement/{undo_record_id}", + "before": { + "records": 0, + "sha256": "4f53cda18c2baa0c0354bb5f9a3ecbe5ed12ab4d8e11ba873c2f11161202b945" + }, + "candidate_id": "undo-step_010-network-1", + "earlier_saves": 0, + "finished_at": "2026-10-19T15:24:47.244521+00:00", + "ignored_fields": [], + "reason": "after the undo the test copy differs from before the save: 1 record(s) added since before the save remain", + "save": "completed", + "save_model_calls": 0, + "schema": "openadapt.undo-verification/v1", + "source": "network", + "started_at": "2026-10-19T15:24:30.572482+00:00", + "test_copy": "mockloan-fee-row-test-copy", + "undo": "completed", + "undo_digest": "c64b712caff380bd1da7ddd6252eea092c48616da0aae77ba023e5fe24511a13", + "undo_judgement": "every undo step ran and an independent read of the system of record confirmed its declared effects", + "undo_model_calls": 0, + "undoes": "step_010", + "verdict": "failed" + }, + "reason": "after the undo the test copy differs from before the save: 1 record(s) added since before the save remain", + "source": "network", + "status": "failed" + } + ], + "intent": "click 'AuthorizeDisbursement'", + "reason": "network: after the undo the test copy differs from before the save: 1 record(s) added since before the save remain (also: no other recordings were given; the save creates a record; there is no old value to restore)", + "step_id": "step_010", + "to_close_the_gap": "record a person doing the app's own delete or cancel path and pass it with --from-recording; if the only difference is a timestamp or audit field, a person may name it with --ignore-field", + "verified_undo": null + } + ], + "schema": "openadapt.undo-propose-report/v1", + "summary": { + "no_verified_undo": 1, + "saving_steps": 1, + "verified_undo": 0 + }, + "test_copy": "mockloan-fee-row-test-copy", + "workflow_name": "mockloan-fee-row-demo", + "written": false +} \ No newline at end of file diff --git a/packages/flow/benchmark/undo_proposal/results/mockloan.json b/packages/flow/benchmark/undo_proposal/results/mockloan.json new file mode 100644 index 0000000000..1aa2e96845 --- /dev/null +++ b/packages/flow/benchmark/undo_proposal/results/mockloan.json @@ -0,0 +1,61 @@ +{ + "saving_steps": [ + { + "candidates": [ + { + "basis": "the save's JSON POST /api/disbursement returned a record id; REST convention gives DELETE /api/disbursement/{undo_record_id}", + "candidate_id": "undo-step_010-network-1", + "evidence": { + "after_save": { + "records": 1, + "sha256": "ed575da8ce2eb2368659390665637dbb57992a240766f35912012a9791c18dcd" + }, + "after_undo": { + "records": 0, + "sha256": "4f53cda18c2baa0c0354bb5f9a3ecbe5ed12ab4d8e11ba873c2f11161202b945" + }, + "basis": "the save's JSON POST /api/disbursement returned a record id; REST convention gives DELETE /api/disbursement/{undo_record_id}", + "before": { + "records": 0, + "sha256": "4f53cda18c2baa0c0354bb5f9a3ecbe5ed12ab4d8e11ba873c2f11161202b945" + }, + "candidate_id": "undo-step_010-network-1", + "earlier_saves": 0, + "finished_at": "2026-10-19T15:23:22.362175+00:00", + "ignored_fields": [], + "reason": "the undo ran, its record read-back confirmed it, and every record matches the read-back from before the save", + "save": "completed", + "save_model_calls": 0, + "schema": "openadapt.undo-verification/v1", + "source": "network", + "started_at": "2026-10-19T15:23:14.680070+00:00", + "test_copy": "mockloan-test-copy", + "undo": "completed", + "undo_digest": "c64b712caff380bd1da7ddd6252eea092c48616da0aae77ba023e5fe24511a13", + "undo_judgement": "every undo step ran and an independent read of the system of record confirmed its declared effects", + "undo_model_calls": 0, + "undoes": "step_010", + "verdict": "verified" + }, + "reason": "the undo ran, its record read-back confirmed it, and every record matches the read-back from before the save", + "source": "network", + "status": "verified" + } + ], + "intent": "click 'AuthorizeDisbursement'", + "reason": "", + "step_id": "step_010", + "to_close_the_gap": "", + "verified_undo": "undo-step_010-network-1" + } + ], + "schema": "openadapt.undo-propose-report/v1", + "summary": { + "no_verified_undo": 0, + "saving_steps": 1, + "verified_undo": 1 + }, + "test_copy": "mockloan-test-copy", + "workflow_name": "mockloan-demo", + "written": true +} \ No newline at end of file diff --git a/packages/flow/benchmark/undo_proposal/results/summary.json b/packages/flow/benchmark/undo_proposal/results/summary.json index d484d2179c..3519175ed3 100644 --- a/packages/flow/benchmark/undo_proposal/results/summary.json +++ b/packages/flow/benchmark/undo_proposal/results/summary.json @@ -1,10 +1,70 @@ { "schema": "openadapt.undo-propose-measurement/v1", - "engine_commit": "835803ff14f7e0da6ee1b8de3f7ba37157819e6c", - "measured_at": "2026-10-19T11:16:15-0400", + "engine_commit": "b5189495df43a0884b7ce316b7502e3e89f171bc", + "measured_at": "2026-10-19T11:24:47-0400", "host": "macOS-15.7.3-arm64-arm-64bit", - "load_1m": 135.74, + "load_1m": 251.62, "fixtures": [ + { + "fixture": "mockloan", + "what_the_save_does": "Authorize writes one disbursement record", + "mode": "live (real browser, app declared a test copy)", + "exit_code": 0, + "save_step": "step_010", + "compiler_labelled_the_save_a_write": false, + "seconds": 18.5, + "summary": { + "no_verified_undo": 0, + "saving_steps": 1, + "verified_undo": 1 + }, + "steps": [ + { + "step_id": "step_010", + "verified_undo": "undo-step_010-network-1", + "reason": "", + "to_close_the_gap": "", + "candidates": [ + { + "source": "network", + "status": "verified", + "basis": "the save's JSON POST /api/disbursement returned a record id; REST convention gives DELETE /api/disbursement/{undo_record_id}", + "reason": "the undo ran, its record read-back confirmed it, and every record matches the read-back from before the save" + } + ] + } + ] + }, + { + "fixture": "mockloan-fee-row", + "what_the_save_does": "Authorize writes the disbursement and also a fee row on a second surface (MockLoan's collateral mode)", + "mode": "live (real browser, app declared a test copy)", + "exit_code": 0, + "save_step": "step_010", + "compiler_labelled_the_save_a_write": false, + "seconds": 78.3, + "summary": { + "no_verified_undo": 1, + "saving_steps": 1, + "verified_undo": 0 + }, + "steps": [ + { + "step_id": "step_010", + "verified_undo": null, + "reason": "network: after the undo the test copy differs from before the save: 1 record(s) added since before the save remain (also: no other recordings were given; the save creates a record; there is no old value to restore)", + "to_close_the_gap": "record a person doing the app's own delete or cancel path and pass it with --from-recording; if the only difference is a timestamp or audit field, a person may name it with --ignore-field", + "candidates": [ + { + "source": "network", + "status": "failed", + "basis": "the save's JSON POST /api/disbursement returned a record id; REST convention gives DELETE /api/disbursement/{undo_record_id}", + "reason": "after the undo the test copy differs from before the save: 1 record(s) added since before the save remain" + } + ] + } + ] + }, { "fixture": "openemr-new-patient", "what_the_save_does": "OpenEMR fixture: the new-patient form saves one chart (browser form post)", @@ -61,8 +121,8 @@ } ], "totals": { - "saving_steps": 3, - "verified_undo": 0, - "no_verified_undo": 3 + "saving_steps": 5, + "verified_undo": 1, + "no_verified_undo": 4 } } diff --git a/packages/flow/benchmark/undo_proposal/run.py b/packages/flow/benchmark/undo_proposal/run.py index 5dbffaec7f..ffae398e49 100644 --- a/packages/flow/benchmark/undo_proposal/run.py +++ b/packages/flow/benchmark/undo_proposal/run.py @@ -50,6 +50,8 @@ class Fixture: extra_match: tuple[tuple[str, str], ...] = () #: Plain words on what this fixture's save does. note: str = "" + #: Text in the compiled save step's intent (``click 'Save Encounter'``). + save_label: str = "" def _fixtures() -> dict[str, Fixture]: @@ -67,6 +69,7 @@ def _fixtures() -> dict[str, Fixture]: match_fields=("patient_id", "type"), identity_key="patient_id", note="Save Encounter writes one encounter record", + save_label="Save Encounter", ), "mockloan": Fixture( name="mockloan", @@ -78,6 +81,7 @@ def _fixtures() -> dict[str, Fixture]: identity_key="loan_id", extra_match=(("surface", "disbursements"),), note="Authorize writes one disbursement record", + save_label="Authorize", ), "mockloan-fee-row": Fixture( name="mockloan-fee-row", @@ -92,6 +96,7 @@ def _fixtures() -> dict[str, Fixture]: "Authorize writes the disbursement and also a fee row on a " "second surface (MockLoan's collateral mode)" ), + save_label="Authorize", ), } @@ -133,17 +138,28 @@ def _save_body(recording: Path) -> dict[str, Any]: return {} -def _bind_save_effect(bundle: Path, fixture: Fixture, body: dict[str, Any]) -> str: - """Bind the save's record effect as an operator would; return its step id.""" +def _bind_save_effect( + bundle: Path, fixture: Fixture, body: dict[str, Any] +) -> tuple[str, bool]: + """Bind the save's record effect as an operator would. + + Returns the save step's id and whether the compiler had already labelled + it a write. Binding a record check makes the step consequential either + way. + """ from openadapt_flow.ir import Workflow from openadapt_flow.runtime.effects.effect import Effect, EffectKind from openadapt_flow.undo_proposal import saving_steps workflow = Workflow.load(bundle) - saves = saving_steps(workflow) - if len(saves) != 1: - raise SystemExit(f"{fixture.name}: expected one saving step, got {len(saves)}") - index, step = saves[-1] + labelled = [i for i, _s in saving_steps(workflow)] + matches = [ + i for i, step in enumerate(workflow.steps) if fixture.save_label in step.intent + ] + if not matches: + raise SystemExit(f"{fixture.name}: no step names {fixture.save_label!r}") + index = matches[-1] + step = workflow.steps[index] match = {k: str(body[k]) for k in fixture.match_fields if k in body} if len(match) != len(fixture.match_fields): raise SystemExit( @@ -163,7 +179,7 @@ def _bind_save_effect(bundle: Path, fixture: Fixture, body: dict[str, Any]) -> s ] workflow.steps[index] = step workflow.save(bundle) - return step.id + return step.id, index in labelled def _contract(directory: Path, base_url: str, fixture: Fixture, body: dict) -> Path: @@ -243,7 +259,7 @@ def run_fixture(fixture: Fixture, out_dir: Path, work: Path) -> dict[str, Any]: bundle = work / "bundle" compile_recording(recording, bundle, name=f"{fixture.name}-demo") body = _save_body(recording) - step_id = _bind_save_effect(bundle, fixture, body) + step_id, compiler_labelled = _bind_save_effect(bundle, fixture, body) contract = _contract(work / "contract", base, fixture, body) deployment = _deployment(work / "deployment.yaml", fixture, base, contract) report_path = out_dir / f"{fixture.name}.json" @@ -271,6 +287,7 @@ def run_fixture(fixture: Fixture, out_dir: Path, work: Path) -> dict[str, Any]: "mode": "live (real browser, app declared a test copy)", "exit_code": code, "save_step": step_id, + "compiler_labelled_the_save_a_write": compiler_labelled, "seconds": round(time.monotonic() - started, 1), "summary": report["summary"], "steps": _rows(report), diff --git a/packages/flow/openadapt_flow/undo_proposal.py b/packages/flow/openadapt_flow/undo_proposal.py index 98ad381309..425910756d 100644 --- a/packages/flow/openadapt_flow/undo_proposal.py +++ b/packages/flow/openadapt_flow/undo_proposal.py @@ -1357,6 +1357,7 @@ def __init__(self, cfg: Any, *, deployment_path: Path) -> None: contract = Path(deployment_path).parent / contract bundle = RewardBundle.load(contract) self._oracle = build_oracle(bundle.oracle, bundle.directory) + self._identity_keys = tuple(bundle.identity_keys) self._params: dict[str, Any] = {} def reset(self) -> None: @@ -1365,7 +1366,13 @@ def reset(self) -> None: def snapshot(self) -> Optional[list[dict[str, Any]]]: from openadapt_flow.reward.oracles import records_of - return records_of(self._oracle.read({})) + # The read returns every record in the collection; the identity only + # labels the observation (``reward.oracles.VerifierOracle.read``). + identity = { + key: str(self._params.get(key) or "undo-propose-snapshot") + for key in self._identity_keys or ("subject",) + } + return records_of(self._oracle.read(identity)) def read_records(self) -> Optional[list[dict[str, Any]]]: from openadapt_flow.after_stop import records_reader diff --git a/packages/flow/tests/test_undo_proposal.py b/packages/flow/tests/test_undo_proposal.py index ba98690f00..30c493e9a8 100644 --- a/packages/flow/tests/test_undo_proposal.py +++ b/packages/flow/tests/test_undo_proposal.py @@ -885,3 +885,47 @@ def test_each_step_without_an_undo_says_what_would_close_the_gap(tmp_path): assert "browser form post" in second.reason assert "the app's own delete or cancel path" in second.to_close_the_gap assert "'Delete'" in second.to_close_the_gap + + +def test_deployment_test_copy_resets_and_reads_every_record(tmp_path, mockmed): + from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey + + from openadapt_flow.deployment import load_deployment + from openadapt_flow.reward.seed import write_bundle + from openadapt_flow.undo_proposal import DeploymentUndoTestCopy + + write_bundle( + tmp_path / "contract", + contract_id="undo_copy", + oracle={"kind": "rest", "base_url": mockmed}, + channel="api", + key=Ed25519PrivateKey.generate(), + issuer_key_id="self_signed:test", + certify=False, + required_effects=[ + { + "kind": "record_written", + "match": {"patient_id": {"param": "patient_id"}, "type": "Triage"}, + "expected_count": 1, + "count_new_only": True, + } + ], + forbidden_effects=[], + ) + config = tmp_path / "deployment.yaml" + config.write_text( + "name: copy\n" + f"backend:\n kind: web\n url: {mockmed}/\n" + f"effects:\n kind: rest\n base_url: {mockmed}\n" + "test_copy:\n declared: true\n" + f" reset_url: {mockmed}/api/reset\n" + " reward_contract: contract\n" + ) + copy = DeploymentUndoTestCopy(load_deployment(config), deployment_path=config) + requests.post(f"{mockmed}/api/encounter", json={"patient_id": "p1"}, timeout=5) + requests.post(f"{mockmed}/api/encounter", json={"patient_id": "p2"}, timeout=5) + # Every record, not only the subject's: the collateral guard needs them. + assert len(copy.snapshot()) == 2 + assert len(copy.read_records()) == 2 + copy.reset() + assert copy.snapshot() == [] From 7a9b20d405858c49995e6cd1e67a1d39c1808582 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 11:27:16 -0400 Subject: [PATCH 05/11] undo propose benchmark: Markdown table output Co-Authored-By: Claude Opus 5.5 --- packages/flow/benchmark/undo_proposal/run.py | 39 ++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/packages/flow/benchmark/undo_proposal/run.py b/packages/flow/benchmark/undo_proposal/run.py index ffae398e49..2dd6127ddd 100644 --- a/packages/flow/benchmark/undo_proposal/run.py +++ b/packages/flow/benchmark/undo_proposal/run.py @@ -340,6 +340,38 @@ def run_offline( } +def markdown_table(summary: dict[str, Any]) -> str: + """One row per saving step: the fixture, how it was run, the outcome, why, + and what would close the gap.""" + lines = [ + "| Fixture | Run | Saving step | Verified undo | Why not, and what would close the gap |", + "| --- | --- | --- | --- | --- |", + ] + for fixture in summary["fixtures"]: + mode = "live" if str(fixture.get("mode", "")).startswith("live") else "offline" + for step in fixture["steps"]: + if step["verified_undo"]: + source = next( + c for c in step["candidates"] if c["status"] == "verified" + ) + outcome = f"yes ({source['source']})" + why = source["basis"] + else: + outcome = "no" + why = f"{step['reason']}. To close it: {step['to_close_the_gap']}" + why = why.replace("|", "\\|") + lines.append( + f"| {fixture['fixture']} | {mode} | `{step['step_id']}` | {outcome} | {why} |" + ) + totals = summary["totals"] + lines.append("") + lines.append( + f"Total: {totals['saving_steps']} saving steps, {totals['verified_undo']} " + f"with a verified undo, {totals['no_verified_undo']} without." + ) + return "\n".join(lines) + + def main(argv: Optional[list[str]] = None) -> int: parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) parser.add_argument("--out", required=True, type=Path) @@ -355,7 +387,14 @@ def main(argv: Optional[list[str]] = None) -> int: "a test copy (RECORDING may be '-')" ), ) + parser.add_argument( + "--table", action="store_true", help="Print summary.json as a Markdown table" + ) args = parser.parse_args(argv) + if args.table: + summary = json.loads((args.out / "summary.json").read_text(encoding="utf-8")) + print(markdown_table(summary)) + return 0 fixtures = _fixtures() names = args.fixture if args.fixture is not None else list(fixtures) if args.offline and args.fixture is None: From b7e1617c9bd93a515c7f82cbeaa3a52104991d5d Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 11:27:53 -0400 Subject: [PATCH 06/11] undo propose: results tables in docs, regenerate artifact inventory Co-Authored-By: Claude Opus 5.5 --- .../flow/benchmark/undo_proposal/README.md | 11 +++++++- packages/flow/docs/AFTER_A_STOP.md | 26 ++++++++++++++++--- packages/flow/public-artifacts.json | 20 ++++++++++++++ 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/packages/flow/benchmark/undo_proposal/README.md b/packages/flow/benchmark/undo_proposal/README.md index dbe3d57d2c..a9ca390ebf 100644 --- a/packages/flow/benchmark/undo_proposal/README.md +++ b/packages/flow/benchmark/undo_proposal/README.md @@ -26,7 +26,16 @@ Synthetic data only. No model calls. ## Results -RESULTS_TABLE +| App | Run | Saving step | Verified undo | Why not, and what would close the gap | +| --- | --- | --- | --- | --- | +| MockMed | live | Save Encounter | MOCKMED_RESULT | MOCKMED_WHY | +| MockLoan | live | Authorize Disbursement | Yes: `DELETE /api/disbursement/{undo_record_id}` (captured traffic, REST convention) | | +| MockLoan, fee-row mode | live | Authorize Disbursement | No | The save also writes a fee row on a second surface, and the `DELETE` leaves it, so the records after the undo don't match. Closes with a recording of the app's own reversal path. | +| OpenEMR fixture, new patient | offline | `step_039` (a click Flow counts as a possible write) | No | No record check is bound to the step, so there is nothing to invert. Closes with a bound record check. | +| OpenEMR fixture, new patient | offline | `step_040` (the save) | No | The save is a browser form post, so no API undo is built. The only control after the save is Cancel. No recording deletes a chart. Closes with a recording of the app's own delete path. | +| openIMIS fixture, claim | offline | `step_015` (the save) | No | The recording has no network capture and no recording deletes a claim. The live fixture wasn't started. Closes with a recording of the app's own delete or cancel path, or a new recording with `--capture-network`. | + +TOTALS_LINE The full reports are in [`results/`](results/): one JSON file per fixture (each candidate's source, status, basis and reason, and for a live row the diff --git a/packages/flow/docs/AFTER_A_STOP.md b/packages/flow/docs/AFTER_A_STOP.md index 38b2cd0d0e..cd8b3a68ba 100644 --- a/packages/flow/docs/AFTER_A_STOP.md +++ b/packages/flow/docs/AFTER_A_STOP.md @@ -260,11 +260,14 @@ run ended, and the reason) is in the report. A failed candidate is reported with its reason and never written. For example: ```text -2 saving step(s): 1 with a verified undo, 1 without. - save_triage: verified undo undo-save_triage-network-1 - authorize: no verified undo. network: after the undo the test copy differs from before the save: 1 record(s) added since before the save remain +1 saving step(s): 0 with a verified undo, 1 without. + step_010: no verified undo. network: after the undo the test copy differs from before the save: 1 record(s) added since before the save remain (also: no other recordings were given; the save creates a record; there is no old value to restore) + To close the gap: record a person doing the app's own delete or cancel path and pass it with --from-recording; if the only difference is a timestamp or audit field, a person may name it with --ignore-field. ``` +That is MockLoan in a mode where the save also writes a fee row: the +`DELETE` removes the disbursement and leaves the fee. + The evidence file holds no record values. The report names fields, never values. @@ -318,7 +321,22 @@ After a stop, the approved proposal is offered, approved and run exactly as in demonstration in a real browser with network capture, then runs `undo propose` against the app as a test copy. -MEASUREMENT_TABLE +| App | Run | Saving step | Verified undo | Why not, and what would close the gap | +| --- | --- | --- | --- | --- | +| MockMed | live | Save Encounter | MOCKMED_RESULT | MOCKMED_WHY | +| MockLoan | live | Authorize Disbursement | Yes: `DELETE /api/disbursement/{undo_record_id}` (captured traffic, REST convention) | | +| MockLoan, fee-row mode | live | Authorize Disbursement | No | The save also writes a fee row on a second surface, and the `DELETE` leaves it, so the records after the undo don't match. Closes with a recording of the app's own reversal path. | +| OpenEMR fixture, new patient | offline | `step_039` (a click Flow counts as a possible write) | No | No record check is bound to the step, so there is nothing to invert. Closes with a bound record check. | +| OpenEMR fixture, new patient | offline | `step_040` (the save) | No | The save is a browser form post, so no API undo is built. The only control after the save is Cancel. No recording deletes a chart. Closes with a recording of the app's own delete path. | +| openIMIS fixture, claim | offline | `step_015` (the save) | No | The recording has no network capture and no recording deletes a claim. The live fixture wasn't started. Closes with a recording of the app's own delete or cancel path, or a new recording with `--capture-network`. | + +TOTALS_LINE + +Live rows record the demonstration in a real browser with network capture +and run `undo propose` against the app declared a test copy. Offline rows read +an existing bundle and recording and run nothing; they answer whether any +candidate can be built. Every candidate's source, status, basis and reason is +in [`benchmark/undo_proposal/results/`](../benchmark/undo_proposal/results/). ## What this doesn't do diff --git a/packages/flow/public-artifacts.json b/packages/flow/public-artifacts.json index 242a6c485c..b94afe42e2 100644 --- a/packages/flow/public-artifacts.json +++ b/packages/flow/public-artifacts.json @@ -859,6 +859,26 @@ "path": "benchmark/ui_readback/results.json", "sha256": "7f7a1ef83fef7ecab6f3bb990e6cd257a12e0af0b49be83aca6a16bfd25a2060" }, + { + "path": "benchmark/undo_proposal/results/mockloan-fee-row.json", + "sha256": "a42eb2cd91b714c9eccc55ac8ff7fa737cd3e9ce389dfe26ee59aa35e96288e4" + }, + { + "path": "benchmark/undo_proposal/results/mockloan.json", + "sha256": "8e7cd2a4a7dc7e5157c899ba6316d9ed270bfbd94ffc1e609814b095093d1844" + }, + { + "path": "benchmark/undo_proposal/results/openemr-new-patient.json", + "sha256": "1be944a8c13c04bc690a20da6ecf9710abff6363d5c4fecc316830d1d9e455a3" + }, + { + "path": "benchmark/undo_proposal/results/openimis-claim.json", + "sha256": "8252e5af1a706b25c12859c5f39e71f3870f71f3a2bb81d9705ed6ac4f5d4792" + }, + { + "path": "benchmark/undo_proposal/results/summary.json", + "sha256": "5f77d556ed53654c866035c6baf4e49a27ba126404e17924ab2ec9829f4de363" + }, { "path": "benchmark/vision_hardening/corpus.json", "sha256": "c64e45ce996f6b13b2e1a3f234d2931e10b5090163215a1185c639a019820e35" From 43aa5eaf6f778a27ebb843aeff0722f59d8f2241 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 11:28:16 -0400 Subject: [PATCH 07/11] CAPABILITIES: link #1346 Co-Authored-By: Claude Opus 5.5 --- CAPABILITIES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CAPABILITIES.md b/CAPABILITIES.md index 03139c7984..d76e88a742 100644 --- a/CAPABILITIES.md +++ b/CAPABILITIES.md @@ -133,7 +133,7 @@ improvising or retrying when something is off. | Checked later: a save confirmed by a later report or acknowledgment, never counted as done until then | In review ([#1297](https://github.com/OpenAdaptAI/OpenAdapt/pull/1297)) | Synthetic | `SAVED_AWAITING_CHECK` | | Keep the server's own answer to a save as lower-level evidence on the receipt | In review ([#1298](https://github.com/OpenAdaptAI/OpenAdapt/pull/1298)) | Synthetic | receipts | | A ledger of what changed before a stop, a write-ordering lint, and undo steps a person approves | On main ([#1263](https://github.com/OpenAdaptAI/OpenAdapt/pull/1263)) | Synthetic (MockMed) | change ledger | -| Propose an undo for each saving step (another recording, captured traffic, or the old value), keep it only if a test copy shows every record restored, and offer it after a stop once a person approves it | In review ([#PRNUM](https://github.com/OpenAdaptAI/OpenAdapt/pull/PRNUM)) | Synthetic (MockMed, MockLoan, live browser) | `openadapt-flow undo propose`, `undo accept`; [AFTER_A_STOP.md](packages/flow/docs/AFTER_A_STOP.md) | +| Propose an undo for each saving step (another recording, captured traffic, or the old value), keep it only if a test copy shows every record restored, and offer it after a stop once a person approves it | In review ([#1346](https://github.com/OpenAdaptAI/OpenAdapt/pull/1346)) | Synthetic (MockMed, MockLoan, live browser) | `openadapt-flow undo propose`, `undo accept`; [AFTER_A_STOP.md](packages/flow/docs/AFTER_A_STOP.md) | ## Approvals From 2e2e1769f326d4c543369d466acebd0c563cf860 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 13:25:11 -0400 Subject: [PATCH 08/11] undo propose: live MockMed row Co-Authored-By: Claude Opus 5.5 --- .../flow/benchmark/undo_proposal/README.md | 4 +- .../undo_proposal/results/mockmed.json | 61 +++++++++++++++++++ .../undo_proposal/results/summary.json | 40 ++++++++++-- packages/flow/docs/AFTER_A_STOP.md | 4 +- 4 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 packages/flow/benchmark/undo_proposal/results/mockmed.json diff --git a/packages/flow/benchmark/undo_proposal/README.md b/packages/flow/benchmark/undo_proposal/README.md index a9ca390ebf..7a8fa8db5c 100644 --- a/packages/flow/benchmark/undo_proposal/README.md +++ b/packages/flow/benchmark/undo_proposal/README.md @@ -28,14 +28,14 @@ Synthetic data only. No model calls. | App | Run | Saving step | Verified undo | Why not, and what would close the gap | | --- | --- | --- | --- | --- | -| MockMed | live | Save Encounter | MOCKMED_RESULT | MOCKMED_WHY | +| MockMed | live | Save Encounter | Yes: `DELETE /api/encounter/{undo_record_id}` (captured traffic, REST convention) | | | MockLoan | live | Authorize Disbursement | Yes: `DELETE /api/disbursement/{undo_record_id}` (captured traffic, REST convention) | | | MockLoan, fee-row mode | live | Authorize Disbursement | No | The save also writes a fee row on a second surface, and the `DELETE` leaves it, so the records after the undo don't match. Closes with a recording of the app's own reversal path. | | OpenEMR fixture, new patient | offline | `step_039` (a click Flow counts as a possible write) | No | No record check is bound to the step, so there is nothing to invert. Closes with a bound record check. | | OpenEMR fixture, new patient | offline | `step_040` (the save) | No | The save is a browser form post, so no API undo is built. The only control after the save is Cancel. No recording deletes a chart. Closes with a recording of the app's own delete path. | | openIMIS fixture, claim | offline | `step_015` (the save) | No | The recording has no network capture and no recording deletes a claim. The live fixture wasn't started. Closes with a recording of the app's own delete or cancel path, or a new recording with `--capture-network`. | -TOTALS_LINE +Total: 6 saving steps, 2 with a verified undo (both synthetic apps' plain saves), 4 without. On the two real-app fixtures, 0 of 3: each needs a recording of the app's own delete or cancel path, or a bound record check. The full reports are in [`results/`](results/): one JSON file per fixture (each candidate's source, status, basis and reason, and for a live row the diff --git a/packages/flow/benchmark/undo_proposal/results/mockmed.json b/packages/flow/benchmark/undo_proposal/results/mockmed.json new file mode 100644 index 0000000000..2927cfae37 --- /dev/null +++ b/packages/flow/benchmark/undo_proposal/results/mockmed.json @@ -0,0 +1,61 @@ +{ + "saving_steps": [ + { + "candidates": [ + { + "basis": "the save's JSON POST /api/encounter returned a record id; REST convention gives DELETE /api/encounter/{undo_record_id}", + "candidate_id": "undo-step_010-network-1", + "evidence": { + "after_save": { + "records": 1, + "sha256": "eae6bf27bdbcf517433491073d4db11940df07d3b676748dd033fa32bd475a83" + }, + "after_undo": { + "records": 0, + "sha256": "4f53cda18c2baa0c0354bb5f9a3ecbe5ed12ab4d8e11ba873c2f11161202b945" + }, + "basis": "the save's JSON POST /api/encounter returned a record id; REST convention gives DELETE /api/encounter/{undo_record_id}", + "before": { + "records": 0, + "sha256": "4f53cda18c2baa0c0354bb5f9a3ecbe5ed12ab4d8e11ba873c2f11161202b945" + }, + "candidate_id": "undo-step_010-network-1", + "earlier_saves": 0, + "finished_at": "2026-10-19T17:24:59.181507+00:00", + "ignored_fields": [], + "reason": "the undo ran, its record read-back confirmed it, and every record matches the read-back from before the save", + "save": "completed", + "save_model_calls": 0, + "schema": "openadapt.undo-verification/v1", + "source": "network", + "started_at": "2026-10-19T17:24:52.054561+00:00", + "test_copy": "mockmed-test-copy", + "undo": "completed", + "undo_digest": "63ea229c780a1d0135ee28e957bb792ce8263caee254b959260876238dfdfdca", + "undo_judgement": "every undo step ran and an independent read of the system of record confirmed its declared effects", + "undo_model_calls": 0, + "undoes": "step_010", + "verdict": "verified" + }, + "reason": "the undo ran, its record read-back confirmed it, and every record matches the read-back from before the save", + "source": "network", + "status": "verified" + } + ], + "intent": "click 'Save Encounter'", + "reason": "", + "step_id": "step_010", + "to_close_the_gap": "", + "verified_undo": "undo-step_010-network-1" + } + ], + "schema": "openadapt.undo-propose-report/v1", + "summary": { + "no_verified_undo": 0, + "saving_steps": 1, + "verified_undo": 1 + }, + "test_copy": "mockmed-test-copy", + "workflow_name": "mockmed-demo", + "written": true +} \ No newline at end of file diff --git a/packages/flow/benchmark/undo_proposal/results/summary.json b/packages/flow/benchmark/undo_proposal/results/summary.json index 3519175ed3..fae5d0a25b 100644 --- a/packages/flow/benchmark/undo_proposal/results/summary.json +++ b/packages/flow/benchmark/undo_proposal/results/summary.json @@ -1,9 +1,9 @@ { "schema": "openadapt.undo-propose-measurement/v1", - "engine_commit": "b5189495df43a0884b7ce316b7502e3e89f171bc", - "measured_at": "2026-10-19T11:24:47-0400", + "engine_commit": "43aa5eaf6f778a27ebb843aeff0722f59d8f2241", + "measured_at": "2026-10-19T13:24:59-0400", "host": "macOS-15.7.3-arm64-arm-64bit", - "load_1m": 251.62, + "load_1m": 5.11, "fixtures": [ { "fixture": "mockloan", @@ -65,6 +65,36 @@ } ] }, + { + "fixture": "mockmed", + "what_the_save_does": "Save Encounter writes one encounter record", + "mode": "live (real browser, app declared a test copy)", + "exit_code": 0, + "save_step": "step_010", + "compiler_labelled_the_save_a_write": true, + "seconds": 20.4, + "summary": { + "no_verified_undo": 0, + "saving_steps": 1, + "verified_undo": 1 + }, + "steps": [ + { + "step_id": "step_010", + "verified_undo": "undo-step_010-network-1", + "reason": "", + "to_close_the_gap": "", + "candidates": [ + { + "source": "network", + "status": "verified", + "basis": "the save's JSON POST /api/encounter returned a record id; REST convention gives DELETE /api/encounter/{undo_record_id}", + "reason": "the undo ran, its record read-back confirmed it, and every record matches the read-back from before the save" + } + ] + } + ] + }, { "fixture": "openemr-new-patient", "what_the_save_does": "OpenEMR fixture: the new-patient form saves one chart (browser form post)", @@ -121,8 +151,8 @@ } ], "totals": { - "saving_steps": 5, - "verified_undo": 1, + "saving_steps": 6, + "verified_undo": 2, "no_verified_undo": 4 } } diff --git a/packages/flow/docs/AFTER_A_STOP.md b/packages/flow/docs/AFTER_A_STOP.md index cd8b3a68ba..757441cef0 100644 --- a/packages/flow/docs/AFTER_A_STOP.md +++ b/packages/flow/docs/AFTER_A_STOP.md @@ -323,14 +323,14 @@ demonstration in a real browser with network capture, then runs | App | Run | Saving step | Verified undo | Why not, and what would close the gap | | --- | --- | --- | --- | --- | -| MockMed | live | Save Encounter | MOCKMED_RESULT | MOCKMED_WHY | +| MockMed | live | Save Encounter | Yes: `DELETE /api/encounter/{undo_record_id}` (captured traffic, REST convention) | | | MockLoan | live | Authorize Disbursement | Yes: `DELETE /api/disbursement/{undo_record_id}` (captured traffic, REST convention) | | | MockLoan, fee-row mode | live | Authorize Disbursement | No | The save also writes a fee row on a second surface, and the `DELETE` leaves it, so the records after the undo don't match. Closes with a recording of the app's own reversal path. | | OpenEMR fixture, new patient | offline | `step_039` (a click Flow counts as a possible write) | No | No record check is bound to the step, so there is nothing to invert. Closes with a bound record check. | | OpenEMR fixture, new patient | offline | `step_040` (the save) | No | The save is a browser form post, so no API undo is built. The only control after the save is Cancel. No recording deletes a chart. Closes with a recording of the app's own delete path. | | openIMIS fixture, claim | offline | `step_015` (the save) | No | The recording has no network capture and no recording deletes a claim. The live fixture wasn't started. Closes with a recording of the app's own delete or cancel path, or a new recording with `--capture-network`. | -TOTALS_LINE +Total: 6 saving steps, 2 with a verified undo (both synthetic apps' plain saves), 4 without. On the two real-app fixtures, 0 of 3: each needs a recording of the app's own delete or cancel path, or a bound record check. Live rows record the demonstration in a real browser with network capture and run `undo propose` against the app declared a test copy. Offline rows read From bfa7d5517b751973940a8fa5f34ecdb6ee9b35fc Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 13:25:14 -0400 Subject: [PATCH 09/11] Regenerate public artifact inventory Co-Authored-By: Claude Opus 5.5 --- llms-full.txt | 92 ++++++++++++++++++++++++----- packages/flow/public-artifacts.json | 6 +- 2 files changed, 81 insertions(+), 17 deletions(-) diff --git a/llms-full.txt b/llms-full.txt index 623dfd5b6b..5254b3458d 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -375,7 +375,8 @@ shipping in the next release; a pull request number means it is being merged. 1. **Learns from any successful run, a person's or an agent's.** A person records a task, or a computer-use agent does it once and OpenAdapt compiles the verified run into a program (`openadapt_flow.jit`, released; - prompt-to-program `compile-agent`, landing). Agents can drive the recording + prompt-to-program `compile-agent`, landing; the agent can act through code + instead of screenshots, [#1305](https://github.com/OpenAdaptAI/OpenAdapt/pull/1305)). Agents can drive the recording on web, native Windows, RDP and Citrix. 2. **Compiles to the fastest layer the app allows.** Page structure first (DOM, Windows UI Automation, macOS Accessibility), direct API calls where the app's @@ -436,9 +437,10 @@ shipping in the next release; a pull request number means it is being merged. | Runs steps as direct API calls; compiling them from captured network traffic is being merged. | `runtime/actuators/api.py`, [#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269) | | Repairs itself when the app changes: governed repair, agent-demonstrated fixes, fleet patches, and self-repair probed up to the save. | [REPAIR_LIFECYCLE.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/REPAIR_LIFECYCLE.md) | | Compiles a computer-use agent's verified run, with no human demonstration needed. | [JIT.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/JIT.md) | +| The agent that does a task once can act through code, not a screenshot-and-click loop: it reads the page or accessibility tree, acts by role and name, keeps notes with a sandboxed read-only shell, and asks for a screenshot only when it needs one. It is arm 7 of the scoreboard; its figures are quoted from there once its rows are in. | `CODE_AS_ACTION.md` in [#1305](https://github.com/OpenAdaptAI/OpenAdapt/pull/1305) | | Binds each approval to the exact screenshot and form state, and re-reads the form before saving. | [APPROVAL_MODES.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/APPROVAL_MODES.md) | | Fills inputs from a referral PDF, fax image or email, and holds back anything unclear for a person. | `intake`, [INTAKE.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/INTAKE.md) | -| Ships as one monorepo with separately installable packages (flow, agent, types, capture, and privacy and desktop moving in). | [README](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/README.md) | +| Ships as one monorepo with separately installable packages (flow, agent, types, capture, privacy and desktop), each released from the monorepo. | [README](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/README.md) | | Makes no model calls on healthy runs. Model training (openadapt-ml) is separate research. | [README](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/README.md) | ## The full map @@ -474,6 +476,7 @@ improvising or retrying when something is off. | Compile one or more demonstrations into a deterministic program | Released | Real app (OpenEMR referral, 62 timed runs) | `openadapt-flow record`, `compile`; [packages/flow](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/README.md) | | Right-record check before any write (structure, then OCR, then pixels) | Released | Real app and synthetic | [RESOLUTION_LADDER.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/RESOLUTION_LADDER.md), `runtime/identity.py` | | Read the saved record back from the system of record (API, database, FHIR, separate session) | Released | Real app (OpenEMR 12 of 12 fields) | [VERIFICATION.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/VERIFICATION.md), `runtime/effects/` | +| Read values out of an app as a workflow step (READ), with a records format, a signed review queue and export to CSV or a database | On main ([#1229](https://github.com/OpenAdaptAI/OpenAdapt/pull/1229), [#1215](https://github.com/OpenAdaptAI/OpenAdapt/pull/1215), [#1218](https://github.com/OpenAdaptAI/OpenAdapt/pull/1218)–[#1224](https://github.com/OpenAdaptAI/OpenAdapt/pull/1224)) | Synthetic (mock desktop apps) | `openadapt_flow/interchange/` | | Stop before the effect instead of improvising (`HALTED_BEFORE_EFFECT`) | Released | Real app and synthetic | `runtime/replayer.py` | | Never retry an uncertain save (`RECONCILIATION_REQUIRED`) | Released | Synthetic (fault server) | [VERIFICATION.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/VERIFICATION.md) | | One check-level scale, 0 to 5, from "unchecked" to "counterparty confirmed" | Released | Unit tests | `openadapt_flow/check_level.py` | @@ -486,6 +489,7 @@ improvising or retrying when something is off. | Checked later: a save confirmed by a later report or acknowledgment, never counted as done until then | In review ([#1297](https://github.com/OpenAdaptAI/OpenAdapt/pull/1297)) | Synthetic | `SAVED_AWAITING_CHECK` | | Keep the server's own answer to a save as lower-level evidence on the receipt | In review ([#1298](https://github.com/OpenAdaptAI/OpenAdapt/pull/1298)) | Synthetic | receipts | | A ledger of what changed before a stop, a write-ordering lint, and undo steps a person approves | On main ([#1263](https://github.com/OpenAdaptAI/OpenAdapt/pull/1263)) | Synthetic (MockMed) | change ledger | +| Propose an undo for each saving step (another recording, captured traffic, or the old value), keep it only if a test copy shows every record restored, and offer it after a stop once a person approves it | In review ([#1346](https://github.com/OpenAdaptAI/OpenAdapt/pull/1346)) | Synthetic (MockMed, MockLoan, live browser) | `openadapt-flow undo propose`, `undo accept`; [AFTER_A_STOP.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/AFTER_A_STOP.md) | ## Approvals @@ -508,7 +512,7 @@ improvising or retrying when something is off. | 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) | | Use spoken narration while recording to propose the rule behind a branch | In review ([#1291](https://github.com/OpenAdaptAI/OpenAdapt/pull/1291)) | Synthetic (generated audio) | `openadapt-capture[audio,privacy]` | -| Settle a disagreement between recordings by trying both paths on a test copy of the app and reading the record back | In review ([#1276](https://github.com/OpenAdaptAI/OpenAdapt/pull/1276)) | Synthetic (MockMed) | `induce --probe-on` | +| Settle a disagreement between recordings by trying both paths on a test copy of the app and reading the record back | On main ([#1276](https://github.com/OpenAdaptAI/OpenAdapt/pull/1276), [#1289](https://github.com/OpenAdaptAI/OpenAdapt/pull/1289)) | Synthetic (MockMed; desktop and remote test copies) | `induce --probe-on` | | Route each item to the right verified variant of a program | On main ([#1264](https://github.com/OpenAdaptAI/OpenAdapt/pull/1264)) | Synthetic | `variants`, `route` | ## Surfaces @@ -519,7 +523,8 @@ improvising or retrying when something is off. | Native Windows (UI Automation) | Released, early | Synthetic (WinForms, 3 of 3 trials) | [SURFACES.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/SURFACES.md) | | Native macOS (Accessibility) | Released, early | Synthetic (TextEdit, 3 of 3 trials) | [SURFACES.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/SURFACES.md) | | RDP (pixels; Aardwolf or headless FreeRDP with a session pool) | Released, early | Real Windows 11 host, 3 of 3, plus a FreeRDP batch | [SURFACES.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/SURFACES.md), `backends/xfreerdp.py` | -| Citrix | Exploratory | A no-DOM stand-in only; no real ICA/HDX session yet | [LIMITS.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/LIMITS.md) | +| Citrix and other virtual desktops, with OpenAdapt installed inside the session (`in_session`): the same structure-first path as a local desktop | Released, early | Same evidence as the Windows path; no live Citrix session yet | [SURFACES.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/SURFACES.md) | +| Citrix from outside the session, pixels only (`remote_display`), the fallback when nothing can be installed inside | Exploratory | A no-DOM stand-in only; no real ICA/HDX session yet | [LIMITS.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/LIMITS.md) | | Chain programs across surfaces, passing confirmed values | Released | Synthetic | `openadapt-flow compose`, `process`; [PROCESS_CONTRACT.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/PROCESS_CONTRACT.md) | | One program that switches surfaces, tabs and popups mid-run | On main ([#1197](https://github.com/OpenAdaptAI/OpenAdapt/pull/1197)) | Real Chromium; Windows switch on a mock | [#1197](https://github.com/OpenAdaptAI/OpenAdapt/pull/1197) | | Halt when a workflow qualified with page structure runs on a pixels-only surface | Released | Synthetic | `openadapt-flow qualify` | @@ -533,7 +538,10 @@ 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 | In review ([#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 | In review ([#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269)) | Synthetic | [#1269](https://github.com/OpenAdaptAI/OpenAdapt/pull/1269) | -| Run a worklist on several workers at once, one signed-in session each, with a stop on any worker stopping all | In review ([#1296](https://github.com/OpenAdaptAI/OpenAdapt/pull/1296)) | Synthetic | `--workers` | +| Run a worklist on several workers at once, one signed-in session each, each item claimed once, with a stop on any worker stopping all | In review ([#1296](https://github.com/OpenAdaptAI/OpenAdapt/pull/1296)) | Synthetic patients on a local OpenEMR test copy | `--workers`, [THROUGHPUT.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/THROUGHPUT.md) | +| Read a save back while the next item runs (pipelined read-back, REST, FHIR, GraphQL or SQL record checks) | In review ([#1296](https://github.com/OpenAdaptAI/OpenAdapt/pull/1296)) | Synthetic | `--readback-window`, [THROUGHPUT.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/THROUGHPUT.md) | +| A faster speed profile that keeps every check (one insert per plain text field, shorter page-quiet wait) | In review ([#1296](https://github.com/OpenAdaptAI/OpenAdapt/pull/1296)) | Synthetic | `runtime.speed: fast`, [THROUGHPUT.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/THROUGHPUT.md) | +| Record where each step's time goes in every run report | In review ([#1296](https://github.com/OpenAdaptAI/OpenAdapt/pull/1296)) | Real app and synthetic | `timings` in `report.json` | ## When the app changes @@ -544,7 +552,7 @@ improvising or retrying when something is off. | Font and layout drift halts with a named reason | Released | Synthetic | `runtime/layout_drift.py` | | Self-repair: an agent proposes a fix for one halted click, Flow probes it up to the save without saving, a person approves | In review ([#1265](https://github.com/OpenAdaptAI/OpenAdapt/pull/1265)) | Synthetic (MockMed redesign) | `run --self-repair`, [#1265](https://github.com/OpenAdaptAI/OpenAdapt/pull/1265) | | Self-repair across several new steps (a new dialog, a control moved into a menu) | In progress | Synthetic (MockMed drift modes) | bridge branch, stacked on #1265 | -| Keep pixel-path runs working across display scaling and font smoothing changes | In review ([#1286](https://github.com/OpenAdaptAI/OpenAdapt/pull/1286)) | Synthetic (RDP and Citrix-style renderings) | `vision/match.py` | +| Keep pixel-path runs working across display scaling and font smoothing changes | On main ([#1286](https://github.com/OpenAdaptAI/OpenAdapt/pull/1286)) | Synthetic (in-process RDP and canvas renderings: 17 of 24 healthy runs across 100–175%, up from 3; 0 wrong saves) | [benchmark/pixel_scale](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/benchmark/pixel_scale/README.md) | | Pre-approved repairs: a signed policy lists low-risk changes that apply without a person, a test-record run before each queue, and one approval for every item halted the same way | In review ([#1294](https://github.com/OpenAdaptAI/OpenAdapt/pull/1294)) | Synthetic | standard changes | | Fleet drift patches: a verified fix with no patient data, shared across sites under policy | Released, local reference hub | Simulator | `openadapt-flow fleet`, [FLEET_DRIFT_PATCHES.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/FLEET_DRIFT_PATCHES.md) | @@ -553,9 +561,11 @@ improvising or retrying when something is off. | Capability | Status | Evidence | Where | | --- | --- | --- | --- | | Compile a computer-use agent's successful run into a program (JIT); later runs make no model calls | Released | Synthetic (a recorded Claude run on MockMed) | `openadapt_flow.jit`, [JIT.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/JIT.md) | +| Code as action: the run-1 agent acts through one terminal tool (structure first, actions by role and name, a sandboxed read-only shell, screenshots on request); every action is a structured step checked against the compiled program | In review ([#1305](https://github.com/OpenAdaptAI/OpenAdapt/pull/1305)) | Real app (local OpenEMR) and synthetic | `ClaudeAgent(vision="code")`, `CODE_AS_ACTION.md` ([#1305](https://github.com/OpenAdaptAI/OpenAdapt/pull/1305)) | | Admit a draft by a person, an admitter key, or an agent under a person-signed policy, including self-admission when the policy allows it | Released | Unit tests | [JIT_ADMISSION.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/JIT_ADMISSION.md) | | MCP server: your agent decides what to enter, OpenAdapt makes the entry and checks it | Released | Synthetic sandbox | `openadapt-agent`, [packages/agent](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/agent/README.md) | -| Compile an agent's first verified run from a prompt (`compile-agent`), and a code-as-action authoring agent | In review ([#1277](https://github.com/OpenAdaptAI/OpenAdapt/pull/1277), [#1305](https://github.com/OpenAdaptAI/OpenAdapt/pull/1305)) | Synthetic | `compile-agent` | +| Compile an agent's first verified run from a prompt (`compile-agent`), and a code-as-action authoring agent | In review ([#1277](https://github.com/OpenAdaptAI/OpenAdapt/pull/1277), [#1305](https://github.com/OpenAdaptAI/OpenAdapt/pull/1305)); the agent harness behind it runs on every backend, on main ([#1283](https://github.com/OpenAdaptAI/OpenAdapt/pull/1283)) | Synthetic | `compile-agent` | +| Score harness optimizers (Harness², Meta-Harness) on GUI tasks with the record read-back as the reward, with full trace export | On main ([#1299](https://github.com/OpenAdaptAI/OpenAdapt/pull/1299), [#1300](https://github.com/OpenAdaptAI/OpenAdapt/pull/1300), [#1301](https://github.com/OpenAdaptAI/OpenAdapt/pull/1301)) | Synthetic (scripted proposer) | `integrations/harness2/` | | One scoreboard comparing the agent alone with OpenAdapt across tasks (correctness by read-back, cost, time, items per hour) | In progress ([#1303](https://github.com/OpenAdaptAI/OpenAdapt/pull/1303)) | Synthetic and local OpenEMR and openIMIS test copies | `benchmark/scoreboard/` | ## Recording, privacy and proof @@ -565,6 +575,7 @@ improvising or retrying when something is off. | Record mouse, keyboard, screen video and accessibility trees on one clock (Windows, macOS, Linux X11) | Released (capture 1.3.0) | Real OS | [packages/capture](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/capture/README.md); Wayland is not supported | | Find and redact personal and health data in text, screenshots and structure | Released (privacy 1.1.3) | Unit tests | [packages/privacy](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/privacy/README.md), [PRIVACY.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/PRIVACY.md) | | Scrub by default before anything leaves the machine | On main ([#1262](https://github.com/OpenAdaptAI/OpenAdapt/pull/1262)) | Unit tests | `openadapt-privacy` | +| Each package installs on its own within a size budget, checked in CI (privacy without desktop, capture without plotting) | On main ([#1270](https://github.com/OpenAdaptAI/OpenAdapt/pull/1270), [#1271](https://github.com/OpenAdaptAI/OpenAdapt/pull/1271), [#1279](https://github.com/OpenAdaptAI/OpenAdapt/pull/1279)) | CI | `.github/workflows/packages.yml` | | Signed receipts (Seals), countersigned and timestamped, offline by default | Released | Unit tests, real TSA tokens | `openadapt-flow seal`, `countersign`, `local-seal` | | Strict no-outbound mode and an unattended on-prem service | Released | Synthetic | `openadapt-flow no-outbound`, `service`; [ON_PREM.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/ON_PREM.md) | @@ -574,10 +585,10 @@ Each `openadapt-flow` command, grouped by the capability it serves. A test fails when a command is added without being listed here. - **Record and build a program:** `record`, `demo-record`, `compile`, `induce`, `disambiguate`, `for-each`, `compose`, `process`, `intake`, `visualize`, `pull`, `variants`, `route`. -- **Check and admit:** `lint`, `scaffold-verifier`, `check`, `bind-record-checks`, `certify`, `qualify`, `propose-qualification`, `known-ids`, `jit`, `admit`, `approver-grant`. +- **Check and admit:** `lint`, `scaffold-verifier`, `check`, `api`, `bind-record-checks`, `certify`, `qualify`, `propose-qualification`, `known-ids`, `jit`, `admit`, `approver-grant`. - **Run and answer:** `run`, `replay`, `resume`, `approve`, `item-approval`, `triage`, `business-decisions`, `explain`, `console`. - **Judge any executor's write:** `verify-run`, `serve-reward` (scores agent training episodes by reading the system of record). -- **Repair and learn from halts:** `teach`, `teach-branch`, `repair`, `fleet`, `undo` (the undo steps a person approves after a stop). +- **Repair and learn from halts:** `teach`, `teach-branch`, `repair`, `fleet`, `undo` (the undo steps a person approves after a stop; `undo propose` finds and verifies them on a test copy, `undo accept` is a person's one-time approval of a proposal). - **Give agents the program:** `emit-skill`, `emit-mcp`, `serve-execute`. - **Proof:** `seal`, `local-seal`, `countersign`, `attach-seal`. - **Run on premises:** `service`, `no-outbound`. @@ -962,6 +973,31 @@ class named with `--allow-halt`. A handoff copies an effect-bound parameter from a confirmed receipt. Missing evidence HALTs. Subflows and worklists stay inside each child, on that child's surface. +## Parallel worklists + +`--workers N` runs a worklist's items on N signed-in sessions at once +([`THROUGHPUT.md`](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/THROUGHPUT.md)). Every item keeps every check. The run +refuses N above the deployment's `worklist.max_workers`, fewer accounts than +workers, two accounts for the same user, two workers sharing one session, a +program with steps outside its loop, and rows not declared +`items_independent`. It can't tell on its own whether two rows touch the same +record; that declaration is yours. A halt on any worker stops new items on +all of them, and an item claimed before a crash is never run again +automatically: it is marked `uncertain` for a person to check. The pipelined +read-back window applies only to an item's last save and only to read-backs +that don't use the screen; durable runs, which include the Standard and +Regulated profiles, always read back inline. Parallel RDP runs use the +FreeRDP session pool and have unit tests only. Throughput figures come from +one machine and a local OpenEMR test copy, not from a production EMR. + +## Short parameter values + +The right-record check binds a typed value into its expected text only when +the value has at least 4 characters. An item whose bound value is shorter, +such as the first name "Bo", halts at the identity check instead of saving. +This is deliberate: a short value matches too much unrelated text to identify +a record. + ## Save triage `openadapt-flow triage` orders verified saves for review under @@ -1296,12 +1332,16 @@ grade is `screen_readback`, never `independent_readback`. **Without a read that is independent of the acting session, the run cannot be `VERIFIED`.** That read is either a system-of-record read (REST, FHIR, SQL, -file; tier 1) or the application's record page read in a separate signed-in -session (`ui_readback` for web, `fresh_session` for desktop and RDP, -tested on mocks only; tier 2). The second goes through the same -application, so it is not a system-of-record read and has its own limits -([`EFFECT_KIT.md`](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/EFFECT_KIT.md#separate-session-ui-read-back-ui_readback)). -A pixel-only Citrix or RDP session with neither is that case. Reopening the +file; tier 1) or the application's own screens read in a separate signed-in +session (tier 2): a record page in a separate browser (`ui_readback`), a +relaunched desktop app or a second RDP session (`fresh_session`, tested on +mocks only), or a look-up in a separate desktop, RDP or Citrix session +(`desktop_readback`, tier 2 with its own RDP logon). None of them goes around +the application, so none is a system-of-record read, and each has its own +limits ([`EFFECT_KIT.md`](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/EFFECT_KIT.md#separate-session-ui-read-back-ui_readback), +[`desktop_readback`](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/EFFECT_KIT.md#separate-session-desktop-read-back-desktop_readback)). +If a person can look the record up, the check can too. A pixel-only Citrix or +RDP deployment that declares no such check is the remaining case. Reopening the record in the acting session (tier 3) is reported as its own check level, `reopened` ("Reopened in the same session"), and is not `VERIFIED`: a confirmed reopen ends `COMPLETED_UNVERIFIED`, and the agent result says @@ -1417,12 +1457,32 @@ What it does provide ([`AFTER_A_STOP.md`](https://github.com/OpenAdaptAI/OpenAda to a person, runs only after that person approves it, runs at most once, and counts as done only when an independent read of the system of record confirms its own declared effects. +- `undo propose` finds candidate undo steps for each saving step (another + recording, captured traffic, or the field's old value) and tries each one + on a declared test copy. It writes a candidate into the bundle, as a + proposal, only when every record afterwards matches the read-back from + before the save. A person approves a proposal once (`undo accept`), and + each run still needs approval after the stop. The limits of that: - A step with no declared effect is `unknown` in the ledger. Flow can't say whether it changed anything. -- An undo exists only where someone declared it. Flow doesn't infer one. +- An undo exists only where someone declared it or approved a verified + proposal. Flow never uses an undo no person approved. +- A proposal is verified once, on a test copy, with the bundle's parameters. + It is evidence that the inverse path works on that copy, not a proof for + every record in production. +- A proposal must restore every record exactly. An app that cancels by + changing a status, or writes an audit row when a record is deleted, fails + verification unless a person names those fields with `--ignore-field`. +- Flow builds API undo steps only from JSON requests. A browser form post, or + a Delete control on the saved record's page, needs a recording of a person + doing the undo. +- A proposal that finds its record (`locate`) or restores old values + (`restore`) needs values the replayer keeps just before the save. They exist + only for runs whose bundle already held the proposal, and only when the + save's record check read the system of record before the save. - An undo can fail or be refused. Then a person must repair the record by hand. - No undo is offered for a workflow with a loop: the undo can't be bound to diff --git a/packages/flow/public-artifacts.json b/packages/flow/public-artifacts.json index b94afe42e2..819c13ab73 100644 --- a/packages/flow/public-artifacts.json +++ b/packages/flow/public-artifacts.json @@ -867,6 +867,10 @@ "path": "benchmark/undo_proposal/results/mockloan.json", "sha256": "8e7cd2a4a7dc7e5157c899ba6316d9ed270bfbd94ffc1e609814b095093d1844" }, + { + "path": "benchmark/undo_proposal/results/mockmed.json", + "sha256": "44f22f82c97cf2ff137513820e6de02260ca96445ba465af7681e97054a8a5df" + }, { "path": "benchmark/undo_proposal/results/openemr-new-patient.json", "sha256": "1be944a8c13c04bc690a20da6ecf9710abff6363d5c4fecc316830d1d9e455a3" @@ -877,7 +881,7 @@ }, { "path": "benchmark/undo_proposal/results/summary.json", - "sha256": "5f77d556ed53654c866035c6baf4e49a27ba126404e17924ab2ec9829f4de363" + "sha256": "e59fbe763f1aff0be37f64071eaf62a726460dfed732086825481705047e08aa" }, { "path": "benchmark/vision_hardening/corpus.json", From 26ef5db36f6d7d2ef694238c0a678be02a918123 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 15:02:00 -0400 Subject: [PATCH 10/11] Replayer undo hook: accept no effects (mypy) Co-Authored-By: Claude Opus 5.5 --- packages/flow/openadapt_flow/runtime/replayer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index 9841c9afe8..2eafe5586c 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -9135,7 +9135,7 @@ def _keep_undo_prior_state( workflow: Workflow, step: Step, before: Any, - effects: list["Effect"], + effects: Optional[list["Effect"]], params: Mapping[str, Any], ) -> None: """Keep what a locating or restoring undo of ``step`` needs. From 1ae6440f22e5b94a29663a94ffa032b5c264d365 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 15:06:15 -0400 Subject: [PATCH 11/11] undo_proposal: narrow selector value type (mypy) Co-Authored-By: Claude Opus 5.5 --- packages/flow/openadapt_flow/undo_proposal.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/flow/openadapt_flow/undo_proposal.py b/packages/flow/openadapt_flow/undo_proposal.py index 425910756d..ec10b25774 100644 --- a/packages/flow/openadapt_flow/undo_proposal.py +++ b/packages/flow/openadapt_flow/undo_proposal.py @@ -347,7 +347,7 @@ def _selector_values(effect: Effect, params: Mapping[str, Any]) -> dict[str, str out: dict[str, str] = {} for key, expr in effect.match.items(): value = expr.resolve(params) - if value not in (None, ""): + if value is not None and value != "": out[key] = value return out