abrichr · Nov 3, 2026 · Nov 3, 2026
@@ -171,6 +171,17 @@ imports) and, for agent arms, one ledger per item with every model call's
 tokens and cost. Screenshots and videos are not committed. Raw data is
 committed before any number is quoted.
 
+## Note on amendment numbers
+
+Amendments 11 to 14 were written as Amendments 10 to 13 on their branch. They
+were renumbered when the branch merged after Amendment 10 (lifecycle
+economics). Old Amendment N is now Amendment N+1. Files committed before the
+merge keep the old numbers:
+`raw/openemr_referral/code_agent/2026-10-25-rb/meta.json`,
+`raw/openemr_referral/code_agent/2026-10-25-rb-admit/meta.json` and
+`raw/mockmed/code_agent/2026-10-25-rb/HAND_REVIEW.md`. Their "Amendment 11" is
+Amendment 12 here.
+
 ## Amendment 1 (2026-10-18, after a code review, before any figure was quoted)
 
 Arms 1 and 3 on MockMed and arm 1 on openIMIS were running when these changes
@@ -381,11 +392,6 @@ that has a REPORT.md.
 
 ## Amendment 11 (2026-10-25): arm 7 with a read-back on every save, and the program it compiles
 
-Amendments 11 to 14 were written as Amendments 10 to 13 on their branch and
-renumbered when the branch merged after Amendment 10 (lifecycle economics).
-Run notes committed under `raw/` before the merge use the old numbers: their
-"Amendment 11" is Amendment 12 here.
-
 Written while arm 7 was on item 25, before any result was summed. Code as
 action became the default first-run agent,
 and run 1 now reads every save back from the system of record before the

@@ -946,7 +946,9 @@ class BrowserTeachBridge:
 
     Duck-types :class:`~openadapt_flow.agent.teach.TeachBridge`: the MCP layer
     calls ``handles``, ``list_tool_specs``, ``hold_needed`` and ``dispatch``.
-    No action here needs a person's approval, because none can change data.
+    No action here needs a person's approval. Without a write contract
+    nothing can change data. With one, the wire check lets only the
+    contract's writes leave the browser.
     """
 
     @property
@@ -1014,7 +1016,7 @@ def handles(self, name: str) -> bool:
     def hold_needed(
         self, arguments: Mapping[str, Any], tool: str = "teach_act"
     ) -> None:
-        """The MCP layer passes the tool name (``teach_act`` or ``teach_run``)."""
+        """Always None: this bridge has no person holds. ``tool`` is ignored."""
 
         return None
 

@@ -288,7 +288,8 @@ def _server_instructions(
     elif teach is not None:
         text += (
             " To add a workflow, teach it: call teach_start with the task, then "
-            "teach_observe and teach_act until it is done, then teach_finish. "
+            "teach_observe and teach_run (web apps) or teach_act until it is "
+            "done, then teach_finish. "
             "Run 1 makes a real entry in the app; use test records. A person "
             "confirms the record check (teach_confirm_checks shows them a form) "
             "and teach_admit admits it only under the operator's signed policy. "

@@ -128,7 +128,7 @@ def handles(self, name: str) -> bool:
     def hold_needed(
         self, arguments: Mapping[str, Any], tool: str = "teach_act"
     ) -> None:
-        """The MCP layer passes the tool name (``teach_act`` or ``teach_run``)."""
+        """Always None: this bridge has no person holds. ``tool`` is ignored."""
 
         return None
 

@@ -0,0 +1,119 @@
+"""Every teach bridge takes the MCP layer's ``hold_needed(arguments, tool=...)``.
+
+``build_server`` duck-types its ``teach`` bridge. When the MCP layer started
+passing ``tool=``, one bridge lacked the keyword, and only an e2e test found
+the TypeError. This builds each real bridge into the real server, calls its
+teach tools through an in-memory MCP client, and checks that ``hold_needed``
+was called with ``tool=`` and that no call failed inside the server.
+No app, browser or model is used.
+"""
+
+from __future__ import annotations
+
+import inspect
+import json
+from pathlib import Path
+from typing import Any
+
+import pytest
+import yaml
+
+pytest.importorskip("mcp")
+
+import anyio  # noqa: E402
+from mcp import ClientSession  # noqa: E402
+from mcp.shared.memory import create_client_server_memory_streams  # noqa: E402
+
+from openadapt_flow.agent.browser_teach import BrowserTeachBridge  # noqa: E402
+from openadapt_flow.agent.mcp import build_server  # noqa: E402
+from openadapt_flow.agent.teach import TeachBridge, TeachConfig  # noqa: E402
+from openadapt_flow.agent.verify_tools import VerifyBridge  # noqa: E402
+from openadapt_flow.guard_demo import verify_yaml  # noqa: E402
+
+BASE = "http://127.0.0.1:9"
+# The MCP layer's answer when a call raised something other than BridgeError,
+# such as a TypeError from a hold_needed without ``tool``.
+FAILED = "Local tool execution failed safely"
+# Schema-valid arguments; each call fails cleanly because no run is going.
+ARGS = {
+    "teach_act": {"action": "click", "ref": "e1"},
+    "teach_run": {"script": "tree"},
+}
+
+
+def _teach(tmp_path: Path) -> TeachBridge:
+    verify = tmp_path / "verify.yaml"
+    verify.write_text(
+        "verifier:\n  kind: rest\n"
+        f"  base_url: {BASE}\n  records_path: /api/db\n  records_key: records\n"
+    )
+    return TeachBridge(
+        TeachConfig(verify=verify, cache_dir=tmp_path / "teach", url=f"{BASE}/")
+    )
+
+
+def _browser(tmp_path: Path) -> BrowserTeachBridge:
+    return BrowserTeachBridge(tmp_path / "tools", tmp_path / "files")
+
+
+def _verify(tmp_path: Path) -> VerifyBridge:
+    contracts = tmp_path / "contracts"
+    contracts.mkdir()
+    (contracts / "referral.yaml").write_text(yaml.safe_dump(verify_yaml(BASE)))
+    return VerifyBridge(contracts, tmp_path / "state")
+
+
+BRIDGES = {"teach": _teach, "browser_teach": _browser, "verify": _verify}
+
+
+async def _calls(server: Any, names: list[str]) -> dict[str, dict]:
+    out: dict[str, dict] = {}
+    async with create_client_server_memory_streams() as (client, transport):
+        async with anyio.create_task_group() as tasks:
+            tasks.start_soon(
+                server.run, *transport, server.create_initialization_options()
+            )
+            async with ClientSession(*client) as session:
+                await session.initialize()
+                listed = {tool.name for tool in (await session.list_tools()).tools}
+                for name in names:
+                    if name not in listed:
+                        continue
+                    result = await session.call_tool(name, ARGS[name])
+                    out[name] = json.loads(result.content[0].text)
+            tasks.cancel_scope.cancel()
+    return out
+
+
+@pytest.mark.parametrize("kind", sorted(BRIDGES))
+def test_hold_needed_takes_the_tool_name(
+    kind: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    bridge = BRIDGES[kind](tmp_path)
+    # The signature the MCP layer calls, for both tool names.
+    for tool in ARGS:
+        inspect.signature(bridge.hold_needed).bind({}, tool=tool)
+
+    # Record the real method's calls; it still runs as written.
+    seen: list[dict[str, Any]] = []
+    real = bridge.hold_needed
+
+    def recording(arguments: Any, **kwargs: Any) -> Any:
+        seen.append(kwargs)
+        return real(arguments, **kwargs)
+
+    monkeypatch.setattr(bridge, "hold_needed", recording)
+    server = build_server(teach=bridge)
+    results = anyio.run(_calls, server, list(ARGS))
+
+    for name, payload in results.items():
+        assert FAILED not in str(payload.get("error")), (name, payload)
+    held = [name for name in ARGS if name in results and bridge.handles(name)]
+    assert seen == [{"tool": name} for name in held]
+    if kind == "teach":
+        # The web teach bridge offers both, and the MCP layer asks it to hold both.
+        assert held == ["teach_act", "teach_run"]
+    elif kind == "browser_teach":
+        assert held == ["teach_act"] and "teach_run" not in results
+    else:
+        assert results == {}
@@ -0,0 +1,158 @@
+"""Run 1's read-back on a FHIR store whose resources change on every read.
+
+``RecordReadBack`` compares whole records. Some FHIR servers set a
+resource's ``meta.lastUpdated`` (or ``meta.versionId``) when it is read, not
+when it is written. This pins what the read-back does then, on the real
+``load_observer`` path over HTTP: a field the operator's ``field_paths`` does
+not map is never seen; a mapped one stops run 1 as ``wrong_record`` although
+only a read changed; ``records_drop_fields`` removes it. The whole-database
+snapshot (``db_snapshot``) is not a read-back source: ``load_observer`` has no
+kind for it, so it is checked only by the contract frame.
+"""
+
+from __future__ import annotations
+
+import itertools
+import json
+import threading
+from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
+from pathlib import Path
+from typing import Any, Iterator
+
+import pytest
+import yaml
+
+from openadapt_flow.compile_agent.harness import record_readback
+from openadapt_flow.record_sor import RecordSorError, load_observer
+
+NOTE = "Synthetic follow-up in two weeks"
+
+
+class _Store:
+    def __init__(self) -> None:
+        self.resources = [
+            {"id": "1", "subject": {"reference": "Patient/8"}, "valueString": "old"}
+        ]
+        self.reads = itertools.count()
+
+    def bundle(self) -> dict[str, Any]:
+        n = next(self.reads)
+        entries = [
+            {
+                "resource": {
+                    "resourceType": "Observation",
+                    "status": "final",
+                    # Changes on every read, as on servers that stamp it then.
+                    "meta": {
+                        "lastUpdated": f"2026-11-03T12:00:{n:02d}Z",
+                        "versionId": str(n),
+                    },
+                    **r,
+                }
+            }
+            for r in self.resources
+        ]
+        return {
+            "resourceType": "Bundle",
+            "type": "searchset",
+            "total": len(entries),
+            "entry": entries,
+        }
+
+
+@pytest.fixture
+def fhir() -> Iterator[tuple[str, _Store]]:
+    store = _Store()
+
+    class Handler(BaseHTTPRequestHandler):
+        def do_GET(self) -> None:  # noqa: N802 - http.server API
+            body = json.dumps(store.bundle()).encode()
+            self.send_response(200)
+            self.send_header("Content-Type", "application/fhir+json")
+            self.send_header("Content-Length", str(len(body)))
+            self.end_headers()
+            self.wfile.write(body)
+
+        def log_message(self, *args: Any) -> None:
+            pass
+
+    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_port}/fhir", store
+    finally:
+        server.shutdown()
+        server.server_close()
+
+
+def _run1_save(tmp_path: Path, base: str, store: _Store, **extra: Any) -> str:
+    """One save in run 1, read back as the pipeline does; the outcome."""
+
+    del store.resources[1:]
+    verify = tmp_path / "verify.yaml"
+    verify.write_text(
+        yaml.safe_dump(
+            {
+                "verifier": {
+                    "kind": "fhir",
+                    "base_url": base,
+                    "resource_type": "Observation",
+                    "timeout_s": 2.0,
+                    **extra,
+                }
+            }
+        )
+    )
+    observer, _ = load_observer(verify)
+    baseline = observer.system_of_record
+    readback = record_readback(
+        lambda: observer.system_of_record,
+        {"patient": "Patient/9"},
+        lambda: baseline,
+        values={"note": NOTE},
+    )
+    readback.start()
+    readback.mark()
+    store.resources.append(
+        {"id": "2", "subject": {"reference": "Patient/9"}, "valueString": NOTE}
+    )
+    return readback().outcome
+
+
+def test_an_unmapped_volatile_field_is_never_seen(fhir, tmp_path: Path) -> None:
+    base, store = fhir
+    assert _run1_save(tmp_path, base, store) == "landed"
+
+
+@pytest.mark.parametrize("path", ["meta.lastUpdated", "meta.versionId"])
+def test_a_mapped_volatile_field_stops_run_1(fhir, tmp_path: Path, path: str) -> None:
+    # A false stop: only a read changed the starting record. It fails closed.
+    base, store = fhir
+    paths = {
+        "id": "id",
+        "patient": "subject.reference",
+        "note": "valueString",
+        "updated": path,
+    }
+    assert _run1_save(tmp_path, base, store, field_paths=paths) == "wrong_record"
+    assert (
+        _run1_save(
+            tmp_path,
+            base,
+            store,
+            field_paths=paths,
+            records_drop_fields=["updated"],
+        )
+        == "landed"
+    )
+
+
+@pytest.mark.parametrize("kind", ["db_snapshot", "database"])
+def test_the_database_snapshot_is_not_a_readback_source(
+    tmp_path: Path, kind: str
+) -> None:
+    verify = tmp_path / "verify.yaml"
+    verify.write_text(yaml.safe_dump({"verifier": {"kind": kind}}))
+    with pytest.raises(RecordSorError):
+        load_observer(verify)