From 1163d8dce226915d74f18e9bfe999a42e4802db3 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 14:23:58 -0400 Subject: [PATCH 1/3] fix(flow): bind an API step to the complete definition of the steps it replaces Review of #1269: folded_checks_digest hashed only the folded steps' on-screen checks, so changing what a replaced step does (its action, value, parameter, target or identity, a guard, the save itself) left the admitted API step running in its place. The digest now covers every replaced step's complete definition, the save included, minus only the save's own API binding. Any change keeps the API step on the GUI with the reason until it is proposed and admitted again. Bindings proposed before this need proposing again (they fail closed). Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/API_TIER.md | 8 ++- .../openadapt_flow/compiler/api_proposal.py | 2 +- packages/flow/openadapt_flow/ir.py | 9 ++- .../runtime/actuators/web_form.py | 12 +++- .../flow/openadapt_flow/runtime/replayer.py | 7 +- .../test_api_step_replaced_steps_digest.py | 70 +++++++++++++++++++ packages/flow/tests/test_api_tier_web_form.py | 4 +- 7 files changed, 98 insertions(+), 14 deletions(-) create mode 100644 packages/flow/tests/test_api_step_replaced_steps_digest.py diff --git a/packages/flow/docs/API_TIER.md b/packages/flow/docs/API_TIER.md index 38242de35..915831072 100644 --- a/packages/flow/docs/API_TIER.md +++ b/packages/flow/docs/API_TIER.md @@ -38,9 +38,11 @@ openadapt-flow run bundle/ --approval-mode not_gated ... first of those steps carries `api_fallback` with the reason, and the GUI steps run as recorded. -If you change a check on one of the covered steps after compiling (for -example, you add a required on-screen text), the API step stops running -because it would skip that check. Run `openadapt-flow api propose bundle/ +If you change any of the covered steps after compiling (an action, a typed +value or parameter, a target or its identity evidence, a guard, an on-screen +check, or the save's record check), the API step stops running: its +admission is bound to a digest of the complete definition of every step it +replaces, the save included. Run `openadapt-flow api propose bundle/ --recording rec/` to propose it again, then admit it. ## What the compiler checks diff --git a/packages/flow/openadapt_flow/compiler/api_proposal.py b/packages/flow/openadapt_flow/compiler/api_proposal.py index 97a89ce08..260d3166f 100644 --- a/packages/flow/openadapt_flow/compiler/api_proposal.py +++ b/packages/flow/openadapt_flow/compiler/api_proposal.py @@ -612,7 +612,7 @@ def carried(line: dict[str, Any]) -> int: replaces_steps=[s.id for s in folded], other_writes_seen=others, screen_only_checks=screen_only, - folded_checks_digest=folded_checks_digest(folded[:-1]), + folded_checks_digest=folded_checks_digest(folded), ) binding = ApiBinding( kind="web_form", diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index ddae6a1ca..d5621334f 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -612,9 +612,12 @@ class WebFormSpec(BaseModel): #: On-screen checks of folded steps that caused no request (screen-only, #: not repeated), listed for the person admitting the step. screen_only_checks: list[str] = Field(default_factory=list) - #: Digest of the folded steps' on-screen checks when this was proposed. - #: If a check is added or changed later, the API step does not run (it - #: would skip that check) until it is proposed again. + #: Digest of the complete definition of every replaced step, the save + #: included (minus its API binding), when this was proposed + #: (``folded_checks_digest`` in runtime.actuators.web_form). Any change to + #: those steps keeps the API step from running until it is proposed and + #: admitted again. Bindings proposed before it covered more than the + #: on-screen checks need proposing again. folded_checks_digest: str = Field(pattern=r"^sha256:[0-9a-f]{64}$") diff --git a/packages/flow/openadapt_flow/runtime/actuators/web_form.py b/packages/flow/openadapt_flow/runtime/actuators/web_form.py index 27915c01a..9fd991210 100644 --- a/packages/flow/openadapt_flow/runtime/actuators/web_form.py +++ b/packages/flow/openadapt_flow/runtime/actuators/web_form.py @@ -345,11 +345,19 @@ def option_for_label(control: dict[str, Any], label: str) -> Optional[str]: def folded_checks_digest(steps: Any) -> str: - """SHA-256 over the on-screen checks (``expect``) of the folded steps.""" + """SHA-256 over the complete definition of the steps an API step replaces. + + Every replaced step, the save included: its action and values, its + parameters, its target and identity evidence, its guards and waits, its + on-screen checks, its record check and its risk. Only the save's own API + binding is left out (it carries this digest). Any change to what the GUI + steps do or check changes the digest, and the API step doesn't run until + it is proposed and admitted again. + """ import hashlib - payload = [[s.id, [p.model_dump(mode="json") for p in s.expect]] for s in steps] + payload = [s.model_dump(mode="json", exclude={"api_binding"}) for s in steps] canonical = json.dumps(payload, sort_keys=True, separators=(",", ":")) return "sha256:" + hashlib.sha256(canonical.encode("utf-8")).hexdigest() diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index 7f26df4fc..3d68fd123 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -2436,12 +2436,13 @@ def _web_form_span_at( elif ids != list(binding.web_form.replaces_steps): reason = "the program's steps no longer match the API step" elif ( - folded_checks_digest(workflow.steps[step_index:end_index]) + folded_checks_digest(workflow.steps[step_index : end_index + 1]) != binding.web_form.folded_checks_digest ): reason = ( - "an on-screen check of the GUI steps changed after the API " - "step was proposed, and the API step would skip it " + "the steps the API step replaces changed after it was " + "proposed (an action, value, target, check, guard or record " + "check), so the API step no longer stands for them " "(openadapt-flow api propose)" ) elif self._actuator_for(binding) is None: diff --git a/packages/flow/tests/test_api_step_replaced_steps_digest.py b/packages/flow/tests/test_api_step_replaced_steps_digest.py new file mode 100644 index 000000000..b80ac338e --- /dev/null +++ b/packages/flow/tests/test_api_step_replaced_steps_digest.py @@ -0,0 +1,70 @@ +"""An API step stands for the exact steps it was admitted for. + +The reviewer's case for #1269: folded_checks_digest hashed only the folded +steps' on-screen checks, so changing what a replaced step does (its action, +its value, its parameter, its target or identity, a guard, the save's record +check) left the admitted API step running in its place. +""" + +from __future__ import annotations + +import pytest + +from openadapt_flow.ir import ActionKind, Anchor, Guard, Predicate, PredicateKind +from openadapt_flow.runtime.replayer import Replayer +from tests.test_api_tier_web_form import _span_workflow +from tests.test_replayer import FakeBackend, FakeVision + + +def _edit(wf, index, **update): + steps = list(wf.steps) + steps[index] = steps[index].model_copy(update=update) + return wf.model_copy(update={"steps": steps}) + + +def _why(wf): + replayer = Replayer(FakeBackend(), vision=FakeVision(), web_form_actuator=object()) + _end, _save, why = replayer._web_form_span_at(wf, 0, durable=False) + return why + + +def test_the_admitted_steps_run_as_the_api_step(): + assert _why(_span_workflow()) is None + + +@pytest.mark.parametrize( + "index, update", + [ + (0, {"action": ActionKind.CLICK, "key": None}), + (0, {"key": "Shift+Tab"}), + (0, {"text": "{lname}", "action": ActionKind.TYPE, "key": None}), + (0, {"param": "lname"}), + ( + 0, + { + "anchor": Anchor( + template="templates/x.png", + region=(0, 0, 10, 10), + click_point=(5, 5), + context_text="Another record", + ) + }, + ), + ( + 0, + { + "guard": Guard( + predicate=Predicate( + kind=PredicateKind.PARAM_EQUALS, param="lname", value="x" + ), + on_unmet="halt", + ) + }, + ), + (1, {"key": "Ctrl+Enter"}), + (1, {"risk": "irreversible"}), + ], +) +def test_any_change_to_a_replaced_step_keeps_it_on_the_gui(index, update): + why = _why(_edit(_span_workflow(), index, **update)) + assert why is not None and "replaces changed" in why diff --git a/packages/flow/tests/test_api_tier_web_form.py b/packages/flow/tests/test_api_tier_web_form.py index 4e99f9099..75396777e 100644 --- a/packages/flow/tests/test_api_tier_web_form.py +++ b/packages/flow/tests/test_api_tier_web_form.py @@ -561,7 +561,7 @@ def _span_workflow(*, admitted=True) -> Workflow: WebFormPrecheck(path="/dup.php", require_text=["No matches were found."]) ], replaces_steps=["step_000", "step_001"], - folded_checks_digest=folded_checks_digest(steps[:1]), + folded_checks_digest=folded_checks_digest(steps), ) binding = ApiBinding( kind="web_form", url_template="/save.php", effects=[effect], web_form=spec @@ -666,7 +666,7 @@ def test_a_rejected_save_halts_without_gui_retry(tmp_path): } ), {}, - "on-screen check", + "replaces changed", ), ], ) From ac9a17d50b1320e636d6144b6c0bcb1c28086824 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 14:24:28 -0400 Subject: [PATCH 2/3] docs(flow): scope the API step to a single HTML form submission Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/API_TIER.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/flow/docs/API_TIER.md b/packages/flow/docs/API_TIER.md index 915831072..fe991d5c9 100644 --- a/packages/flow/docs/API_TIER.md +++ b/packages/flow/docs/API_TIER.md @@ -1,9 +1,14 @@ # API steps from a recording's network traffic -A web form save that OpenAdapt can repeat as one HTTP request runs as one -request instead of as the clicks and keystrokes that filled the form. The -right-record check before the save and the record check after it stay the -same. If anything the API step depends on has changed, the recorded GUI +**Scope: a single HTML form submission.** An API step repeats one +`application/x-www-form-urlencoded` POST of one HTML form, read again from +the live page, in place of the clicks and keystrokes that filled it. It is +not a general API client: JSON or script-built requests, more than one +write, multi-page server state, and forms whose script validates or rewrites +values all stay on the GUI. + +The right-record check before the save and the record check after it stay +the same. If anything the API step depends on has changed, the recorded GUI steps run instead. Its speed is measured on the local OpenEMR referral as arm `program_api` of From 9c9e8d59cef524dc760478a5ba8e5631818eba00 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 18:45:49 -0400 Subject: [PATCH 3/3] fix(flow): bind the authorization's API-step check to the full replaced-step digest After merging main (#1361): the run authorization's admitted_api_step compared folded_checks_digest over the steps before the save, while #1372 now digests every replaced step including the save. It uses the same range as the replayer and the proposal. Tests that edit the save re-propose its digest first, as openadapt-flow api propose does. Co-Authored-By: Claude Opus 5.5 --- .../openadapt_flow/runtime/authorization.py | 4 +-- packages/flow/tests/test_api_step_durable.py | 26 +++++++++++++++++-- packages/flow/tests/test_api_step_governed.py | 2 +- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/packages/flow/openadapt_flow/runtime/authorization.py b/packages/flow/openadapt_flow/runtime/authorization.py index 5748c7b77..2cbd94623 100644 --- a/packages/flow/openadapt_flow/runtime/authorization.py +++ b/packages/flow/openadapt_flow/runtime/authorization.py @@ -343,7 +343,7 @@ class AdmittedApiStep(BaseModel): """One API step (``web_form`` binding) admitted into a governed run. Binds the exact binding a person admitted (``admission_digest``), the - on-screen checks of the GUI steps it stands for (``folded_checks_digest``) + complete definition of the GUI steps it stands for (``folded_checks_digest``) and its record check (``effect_contract_hashes``). The runtime runs an API step under a governed authorization only when all three still match. """ @@ -545,7 +545,7 @@ def admitted_api_step(self, step: Any, workflow: Any) -> Optional[str]: row.admission_digest != binding.admission_digest() or row.folded_checks_digest != binding.web_form.folded_checks_digest or row.folded_checks_digest - != folded_checks_digest(workflow.steps[start:end]) + != folded_checks_digest(workflow.steps[start : end + 1]) or list(row.effect_contract_hashes) != [effect.contract_hash() for effect in step.effects] ): diff --git a/packages/flow/tests/test_api_step_durable.py b/packages/flow/tests/test_api_step_durable.py index 82b80900d..322c1c090 100644 --- a/packages/flow/tests/test_api_step_durable.py +++ b/packages/flow/tests/test_api_step_durable.py @@ -79,6 +79,28 @@ def actuate(self, binding, params): ) +def _reproposed(wf: Workflow) -> Workflow: + """The span program after an edit to its steps, its API step proposed again. + + The API step's admission binds the complete definition of the steps it + replaces, so an edited save needs the binding's digest recomputed, as + ``openadapt-flow api propose`` does, before it is admitted again. + """ + + from openadapt_flow.runtime.actuators.web_form import folded_checks_digest + + save = wf.steps[-1] + spec = save.api_binding.web_form.model_copy( + update={"folded_checks_digest": folded_checks_digest(wf.steps)} + ) + binding = save.api_binding.model_copy(update={"web_form": spec}) + return wf.model_copy( + update={ + "steps": [*wf.steps[:-1], save.model_copy(update={"api_binding": binding})] + } + ) + + def _absolute_check_workflow() -> Workflow: """The span program with a record check that needs no before-snapshot. @@ -91,7 +113,7 @@ def _absolute_check_workflow() -> Workflow: effect = save.effects[0].model_copy(update={"forbid_collateral_loss": False}) binding = save.api_binding.model_copy(update={"effects": [effect]}) save = save.model_copy(update={"effects": [effect], "api_binding": binding}) - wf = wf.model_copy(update={"steps": [*wf.steps[:-1], save]}) + wf = _reproposed(wf.model_copy(update={"steps": [*wf.steps[:-1], save]})) wf, _ = admit_api_step(wf, admitted_by="reviewer") return wf @@ -330,7 +352,7 @@ def test_an_identity_checked_save_with_a_right_record_contract_runs_durably(tmp_ update={"steps": [*wf.steps[:-1], save], "params": {**wf.params, "note": "api"}} ) wf, _ = admit_api_step( - wf, admitted_by="reviewer", identity={"subject_name": "note"} + _reproposed(wf), admitted_by="reviewer", identity={"subject_name": "note"} ) fx = Fixture(tmp_path, wf) try: diff --git a/packages/flow/tests/test_api_step_governed.py b/packages/flow/tests/test_api_step_governed.py index d26917e7b..2aeb259eb 100644 --- a/packages/flow/tests/test_api_step_governed.py +++ b/packages/flow/tests/test_api_step_governed.py @@ -96,7 +96,7 @@ def _workflow(*, admitted: bool = True, identity: bool = True) -> Workflow: ], controls=["csrf", "lname"], replaces_steps=[typing.id, submit.id], - folded_checks_digest=folded_checks_digest([typing]), + folded_checks_digest=folded_checks_digest([typing, submit]), ) submit = submit.model_copy( update={